Class build loop detection breaks everything.

[back2dos]

I get this error a lot: acaa3ec#diff-4a35a028da1fe67ee6a21fbd5305d08cR3127

Minimal example (save as WindowManager.hx and compile with haxe -lib coconut.vdom WindowManager -js whatever.js):

package ;

import coconut.ui.*;
import coconut.data.*;
using tink.CoreApi;

class DefaultWindowManager extends View<{}> {
  function render() '<div />';
}

class Window extends View<{}> {
  function render() '<div />';
}

There is nothing cyclic going on anywhere, so I wonder if the detection logic itself is flawed/based on assumptions that I break. My guess is that the issue could be caused by the fact that there's two build macros, one being @:autoBuild on coconut.ui.View and one being a global one (per --macro addGlobalMetadata) from tink_syntaxhub.

FWIW this code (well actually, the whole project, which is far more complex) certainly did not prevent the compiler from terminating, so I have to wonder what's wrong here.

[ncannasse]

To get an idea about what's happening you can remove the (*/* and */*) comments in typecore.ml to enable the debug section, then compile with -D cdebug to get a trace of all compiler process

[ncannasse]

If you don't want to change the compiler, try sending a minimal reproducible example without any library involved

[nadako]

This seems to be a case of follow-when-building, I reduced the example to this:

Main.hx

@:build(Macro.build())
class BuiltBase {}

class Base extends BuiltBase {}

class A extends Base {}

class B extends Base {}

VDom.hx

class VDom {
	static function f() new HtmlFragment();
}

HtmlFragment .hx

class HtmlFragment {
	public function new() {}
}

Macro.hx

#if macro
import haxe.macro.Context;

class Macro {
	static function build() {
		switch Context.getType("VDom") {
			case TInst(_.get() => cl, _):
				for (f in cl.statics.get())
					Context.follow(f.type);
			case _:
		}
		return [];
	}
}
#end

[Simn]

Sanitized cdebug for that:

  out 0 add build-module ()
  out 0 add force (StdTypes)
  out 0 add build-class (StdTypes)
  out 0 add build-class (String)
  out 0 add build-class (Array)
  out 0 add build-class (haxe.EnumTools)
  out 0 add build-class (Std)
  out 0 add build-class (Main)
  out 0 flush final(final)
  out 1 	run build-module ()
  out 1 	run build-module ()
  out 1 	run build-class (Main.B)
  out 2 		run build-class (Main.Base)
  out 3 			run build-class (Main.BuiltBase)
  out 4 				add build-class (VDom)
  out 4 				run build-class (VDom)
  out 5 					init_class_done VDom
  out 5 					add type-field (VDom:f)
  out 4 				run type_fun (VDom:f) ??PENDING[build-class (VDom);build-class (Main);build-class (Main);build-class (Main);build-class (Std);build-class (haxe.EnumTools);build-class (haxe.EnumTools);build-class (Array);build-class (String);build-class (StdTypes)]
  out 5 					add build-class (HtmlFragment)
  out 5 					flush build-class(load_module)
  out 6 						run build-class (HtmlFragment)
  out 7 							init_class_done HtmlFragment
  out 7 							add type-field (HtmlFragment:new)
  out 6 						run build-class (Main.A)
  out 7 							add late build-class (Main)
  out 6 						run build-class (Std)
  out 6 						run build-class (haxe.EnumTools.EnumValueTools)
  out 6 						run build-class (haxe.EnumTools)
  out 6 						run build-class (Array)
  out 6 						run build-class (String)
  out 6 						run build-class (StdTypes.ArrayAccess)
  out 6 						run build-module (Main)
  out 7 							add late build-class (Main)
  out 6 						run build-module (Main)
  out 7 							FATAL Error.Error(_, _)

[ncannasse]

I'm not sure what do to with this.

Trying to type things while in a build macro is unsound.

Depending on the things being currently on the typing stack (in our case build Main.Base) we might enter an infinite loop which is what happens here.

This is because typing requires all classes to be built. Typing without it if we want to turn the whole thing into a lazy process would require to perform a cl_build() everytime we access a TInst class value in the whole compiler code...

Now it's true that before we might have let things error silently so if you were lucky you didn't hit into a problem, but that does not mean it was correctly working.

@back2dos do you perform / trigger typing in your example ? What's the basis for it ?

[ncannasse]

The problem comes from subclasses with build macro. We require before entering typing that all classes are built, but we can't built the subclasses because we are currently building the superclass.

An approach would be to ignore such cases, but then that would require to store theses "skipped builds" somewhere and put them back into the task list after we have finished building the superclass so they don't get skipped (which would cause problems later)

[back2dos]

Stupid question: why exactly does the typing of Main.B involve the typing of Main.A?

Because as it is, there is nothing cyclic in the code. It's not like the build macro of B triggers building of A which then goes to access the fields of B or something. Otherwise put: shouldn't out 6 run build-class (Main.A) be out 1 run build-class (Main.A)?

[ncannasse]

It's not about cycle. Before entering expression typing we need to make sure that all pending classes structure are correctly populated, so we need to build all of them, because we will not check/trigger such build everytime we access the class structure/fields

[Simn]

What if we had something like this:

and tclass_structure = {
	mutable cl_fields : (string, tclass_field) PMap.t;
	mutable cl_statics : (string, tclass_field) PMap.t;
	mutable cl_ordered_statics : tclass_field list;
	mutable cl_ordered_fields : tclass_field list;
}

and tclass = {
	mutable cl_structure : unit -> tclass_structure;
}

And then the cl_structure function calls cl_build()?

[Simn]

I guess cl_constructor would have to go in there too...

[ncannasse]

That is not an easy change.

…

On Tue, Sep 26, 2017 at 10:05 AM, Simon Krajewski ***@***.***> wrote: I guess cl_constructor would have to go in there too... — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#6567 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-bwDelRtc5jYPal26ICPZzSwSMFrn8ks5smLBjgaJpZM4PRVUO> .

[Simn]

You're not lying...

[back2dos]

Generally, having the build of one class depend on the structure of another is very useful. It's what syntactic delegation works in tink_lang. And it's how one would have to implement something like haxe.remoting.Proxy per macro.

But: in all of these use cases, one doesn't need "the full information". Would it be any easier if we said that getting the fields doesn't require to start expression typing? I think one really only needs to know that the field is a TFun and the number, names and optionality of the arguments. All the argument types and the return type could just be TLazy and the compiler can do something about that in a later pass. Just an uninformed guess ;)

[back2dos]

FWIW I found my way around this. But I'm personally still a bit worried/puzzled by "Trying to type things while in a build macro is unsound." although I haven't fully grasped the implications.

I think there are good use cases for the build macro of one class being able to operate based on the structure of another class and it would be great to have some information on when that's possible and when it isn't.

[Simn]

It's a limitation due to our architecture. But acknowledging that and addressing it are two separate things here.

[lublak]

@Simn is there any solution for it?
I'm confused about it.

[back2dos]

When in a build macro, try to avoid triggering expression typing. For example, you might be able to generate macro calls in the returned fields, that will get run later, i.e. during the expression typing.

[lublak]

@back2dos thank you for the tip.
I complete rework my code that maps expr against imports.
So i can cache the expr and use it later in my code without trigger typedexpr.
It works fine with my workaround.

[Simn]

Does anyone know what the status is here? We fixed some related things.

[back2dos]

Hmm, dunno. The status is that the commit I linked introduced a regression. I suppose there are good reasons, but it's not exactly transparent why this restriction got added or even really what the restriction is. I've found ways not to run into this, but it feels like flying blind. So yeah, if this works as intended, some sort of explanation would be great. Thanks ;)

[RealyUniqueName]

Does anyone know what the status is here? We fixed some related things.

This is still reproducible

[RealyUniqueName]

The problem is that the compiler tries to flush all the types, while it should only flush types pulled by Context.getType("VDom"):

haxe/src/typing/typeloadModule.ml

Lines 975 to 977 in c4d5230

	add_dependency ctx.m.curmod m2;
	if ctx.pass = PTypeField then flush_pass ctx PBuildClass "load_module";
	m2

[ncannasse]

We can't do partial flushes atm :-/

…

On Wed, Jul 3, 2019 at 9:26 AM Aleksandr Kuzmenko ***@***.***> wrote: The problem is that the compiler tries to flush all the types, while it should only flush types pulled by Context.getType("VDom"): https://github.com/HaxeFoundation/haxe/blob/c4d523088e1767c608132ba5a2590c7a663e5328/src/typing/typeloadModule.ml#L975-L977 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#6567?email_source=notifications&email_token=AAHZXQALJOKB6UU5UAKI2XLP5RIC5A5CNFSM4D2FKUHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZDRJLI#issuecomment-507974829>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHZXQBDS7B4D5NKZ7YUSLDP5RIC5ANCNFSM4D2FKUHA> .

[nodiex-cloud]

This is occuring subequent to a parse with the usage of tink_await. I am trying to isolate the issue, but it is very confusing. Effectively in a class hierarchy (C extends B extends A), there is @async (SyntaxHub Built and an independent autoBuild) in A. It appears that there is a Parse on C, where A has had it's @async built, but there has been no Build or processing of B.

It says effectively:

C.hx (lines from the Type signature to the end): Loop in class building prevent compiler termination (B)

The confusing part is that this is occurring in a Map class hierarchy (effectively advanced Map classes of increasing functionality). I have an almost identical class hierarchy for Array, where this is not occurring.

I thought I would try to isolate in my code first instead of building an example from scratch, but it's very unclear why this is happening on Map but not Array, I think it may have to do entirely with ordering. The Array.B type equivalent is parsed and built almost immedietly (maybe because the letter A(rray) comes a lot earlier than M(ap)), and the map types are hit recursively for parsing from an edge from a different type instead of through their direct position in the packages directory structure.

[RealyUniqueName]

Map is based on @:multiType which itself brings a lot of special cases in the compiler code. Try using StringMap<...> instead of Map<String,...>, IntMap<...> instead of Map<Int,...> etc.

[nodiex-cloud]

Good to know, however my base Map does use StringMap.

This was not resolved for my issue even with significant debugging but not examining compiler code, My issue is super weird, if I take the content of B and put it in C, it compiles. B doesn't have any tink_await calls or any building at all except for SyntaxHub and my autoBuild whih doesn't do anything.

[back2dos]

Personally, I think I found my ways to not running into this anymore. Is there a chance someone who understands how and when this does and doesn't work adding an explanation to the manual under macro-limitations or macro-type-building? Otherwise, please close.

[Simn]

Reopening this because I'm currently working on the typer pass system, and I'd at least try to address this. Nadakos example is very nicely isolated and should allow some investigation.

[Sword352]

Are there any updates about this issue?