haxe icon indicating copy to clipboard operation
haxe copied to clipboard

Macro defined modules vs direct loading of types using it

Open kLabz opened this issue 1 year ago • 10 comments

While this looks like a very edge case / weird usage, this actually is easy to trigger in real use case because of diagnostics triggering similar behavior to compile2.hxml.

Not sure yet how to handle that, just pushing the repro for now..

kLabz avatar Jul 31 '24 07:07 kLabz

@Simn any idea how this could be handled?

kLabz avatar Jul 31 '24 07:07 kLabz

Could you describe the actual problem? I can't really make it out from looking at that test.

Simn avatar Jul 31 '24 07:07 Simn

Flushing between each actx.classes load seems to do the job, but will have to watch out for new issues..

Edit: only test failure detected so far:

Done running 664 tests with 1 failures
SUMMARY:
projects/Issue11004/build.hxml
-Bar.hx:27: @:storedTypedExpr 3 computed this_ident as: foo.bar
+Bar.hx:27: @:storedTypedExpr 1 computed this_ident as: foo.bar

Might not be a problem, but would need to double check there

kLabz avatar Jul 31 '24 07:07 kLabz

"Normal" typing order:

  • Main
  • foo.Foo
  • => build macro, with defineType
  • Baz
  • => FooData is built

But when adding Baz to actx.classes, we get:

  • Main
  • foo.Foo
  • no flush so no build macro run at this point
  • Baz
  • => FooData is not built yet

kLabz avatar Jul 31 '24 07:07 kLabz

I can't immediately tell what the difference is between referencing Baz on the command line vs. referencing it from Main itself, e.g. via a typedef X = Baz at the bottom.

Simn avatar Jul 31 '24 08:07 Simn

Difference is each of those types referenced in command line / display files / main are loaded without flushing there: https://github.com/HaxeFoundation/haxe/blob/b537e996f67222d004fd9fb26f229920a140ef75/src/compiler/compiler.ml#L311

So build macros don't happen early enough

kLabz avatar Jul 31 '24 08:07 kLabz

vs. referencing it from Main itself, e.g. via a typedef X = Baz at the bottom.

This does trigger the issue too indeed

kLabz avatar Jul 31 '24 08:07 kLabz

Flushing what exactly, and where does that normally occur?

Simn avatar Jul 31 '24 08:07 Simn

Flushing what exactly, and where does that normally occur?

Doing a flush_pass tctx.g PBuildClass in between those:

https://github.com/HaxeFoundation/haxe/blob/b537e996f67222d004fd9fb26f229920a140ef75/src/compiler/compiler.ml#L311

kLabz avatar Jul 31 '24 08:07 kLabz

I found another way to trigger this issue in a shiro project, basically with haxe build.hxml ParentClass ChildClass (or Json RPC diagnostics with ParentClass.hx and ChildClass.hx open), resulting in:

Loop in class building prevent compiler termination (ParentClass)

I don't have a minimal repro yet to add as test; ideas would be appreciated :sweat_smile: (edit: flush does fix that too)

kLabz avatar Aug 07 '24 13:08 kLabz

@Simn can we merge this? It can happen easily with multi file diagnostics :/

kLabz avatar Dec 02 '24 16:12 kLabz

It's unlikely to be a good fix because it doesn't address the typedef X = Baz case. We can merge it to get the test, but let's keep an issue open for the more general situation. Maybe even a PR that has a failing test case.

Simn avatar Dec 03 '24 05:12 Simn

Right. I don't know why I thought the typedef situation was expected

kLabz avatar Dec 03 '24 07:12 kLabz

~~This also somehow solves an issue I have with compiling some shiro projects (while others on windows don't have the issue), but I'm also getting some X is redefined from X errors which could be related to this PR... so uh might not be a good idea to merge yet.~~

Edit:

  • Those errors are actually domkit vs #11338 issues
  • X is redefined from X is not because of this PR.. I get it with development (before #11338) too

kLabz avatar Dec 03 '24 08:12 kLabz