Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[macro] Error when (re)defining types after failed build #11456

Open
player-03 opened this issue Dec 31, 2023 · 8 comments
Open

[macro] Error when (re)defining types after failed build #11456

player-03 opened this issue Dec 31, 2023 · 8 comments

Comments

@player-03
Copy link
Contributor

I have created a large project using Echoes, and often after a failed build, I get an error about a missing function body, pointing to this line:

var def:TypeDefinition = macro class $storageTypeName extends echoes.ComponentStorage<$componentComplexType> {
	public static final instance:$storageType = new $storageTypePath();
	
	//ComponentStorageBuilder.hx:44: lines 44-46 : Missing function body
	private function new() {
		super($v{ componentTypeName });
	}
};

storageCache.set(storageTypeName, def);
if(!registered) {
	registered = true;
	Context.onTypeNotFound(storageCache.get);
}

Additional clues:

  • I haven't been able to reproduce this in a small project. (So no minimal sample, sorry.)
  • In my large project, I can only reproduce it if I'm running enough other programs in the background. If I close my browser and media player and restart the Haxe server, I can no longer get it to happen. (Possibly related to threads and timing?)
  • If I remove the Context.onTypeNotFound() call and instead use Context.defineType(def), the error messages tell me which type(s) are being redefined, but I'm not sure it's useful information because it causes errors in situations where onTypeNotFound() would lead to a successful build. I believe this is why I switched to onTypeNotFound() in the first place.
    • Using Context.onGenerate() to exclude duplicates doesn't help; the error seems to happen before then.
  • If I use a Printer to print def.fields[1], I always find that the function body is there, regardless of whether the build ends up succeeding or failing. So I believe the function body is being provided to the compiler, but gets deleted/lost later.
  • I've tested in Haxe 4.3.1 and 4.3.3, mainly targeting HL.

I've thought of a few features that could help work around this, or at least debug it. I can compile from source to help test these, but I don't know enough to implement them.

  • A Context.isTypeDefined() function, so I can check if a type was preserved from the last build before calling defineType(). (getType() isn't suitable because of all the side effects.)
  • A @:noCache metadata that prevents the compile server from trying to cache the type.
  • An overwrite argument in Context.defineType() and/or Context.defineModule().
  • A way to check the status of the previous build, and manually clear the compile cache if it failed. (Though shouldn't this happen already? It might be a timing issue where the main thread cleared the cache after an error, but then another thread added one final incomplete class.)
@player-03
Copy link
Contributor Author

Ok, I just tried a simpler test case, and it worked fine.

public static macro function makeThing(name:ExprOf<String>):Expr {
	static var defined:Bool = false;
	if(!defined) {
		defined = true;
		
		//Once per build, define the type.
		Context.defineType(macro class Thing {
			public var name:String;
			public var value:Int = 0;
			
			public inline function new(name:String) {
				this.name = name;
			}
		});
	}
	
	//Multiple times per build, return the code to make a new instance.
	return macro new Thing($name);
}

So it seems this is allowed, but it doesn't work in a large or macro-heavy codebase. I'll see if I can find out anything more about it.

@kLabz
Copy link
Contributor

kLabz commented Jan 1, 2024

You may want that defined var to be @:persistent?

@player-03
Copy link
Contributor Author

In this case, the point is to call defineType() once per build, rather than once per cache.

But now that you mention it, I should be doing both. I don't necessarily need Context.isTypeDefined() if I store my own list.

@player-03
Copy link
Contributor Author

Update: I tried it, and @:persistent can actually produce a "Type not found" error.

  • If I build twice (or more) in a row with no changes, it works fine.
  • If I build once, change Main.hx (even just whitespace), and rebuild, it gives the "Type not found" error.
  • If I change Macro.hx before a build, it works fine, whether or not the previous build had an error.

This implies that Haxe clears generated types even when reusing the macro context. So in theory, you should call defineType() once per build.

@kLabz
Copy link
Contributor

kLabz commented Jan 2, 2024

Hmm yes it's a bit more involved than that.

I've updated my PR #11159 and it does seem stable on nightlies at least, what I thought was random failures were some kind of stdout race condition x_x Might be interesting to run those tests against 4.3.x too.

@player-03
Copy link
Contributor Author

Update: CompilationServer.invalidateFiles() fixes the issue!

New question: is there a way to detect that the previous build failed? I'd rather call invalidateFiles() only when needed, rather than before every single build.

@kralle333
Copy link

I still get this issue using json2object, I added the invalidateFiles function to the file that has the macro and it seems to fix the issue, but doesn't seem like the right way to go about it.

Any updates on this issue?

@kLabz
Copy link
Contributor

kLabz commented Jun 19, 2024

No because it's a user/lib error without minimal repro so there isn't much we can do on compiler side

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants