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 statics initilization #5746

Closed
ncannasse opened this issue Oct 13, 2016 · 8 comments

Comments

Projects
None yet
3 participants
@ncannasse
Copy link
Member

commented Oct 13, 2016

Following conversation in #5676

In macro context (with compilation server), static variables:
a) can be kept in the state they were in the previous compilation (reusing macro context)
b) can be reset the their default value during compilation (discarding + creating a new macro context)

Now in order to improve it, we could fix (a) by generating all statics initialization code in a separate function that would get called when reusing the macro context, so you would get a freshly initialized module. That should render useless the onMacroContextReused

For (b) the only solution would be to detect before reusing the macro context (so on the first called macro) if one of the modules of the macro context has been modified.

This would ensure the same statics behavior has you would get without macro context reuse, without the extra time required to initialize the context.

@Simn Simn modified the milestone: 4.0 Jan 9, 2017

@Simn Simn removed this from the Release 4.0 milestone Apr 17, 2018

@Simn Simn added this to the Design milestone Apr 18, 2018

@ncannasse

This comment has been minimized.

Copy link
Member Author

commented Apr 18, 2018

There's no much design decision here.
Here's a complete list of thing to do:

  • have a way in Eval to reset static variables to their default value or expression, like if you would initialize the module again
  • call this everytime we "reuse" the macro context (instead of onMacroContextReuse)
  • add an api "haxe.macro.Storage" similar to browser localstorage, but only in-memory for compilation server. It persists across recompilation. We can have a "global" storage for all compilation contexts and a "local" storage for the current one (set of defines)
  • deprecate previous "context reuse" etc. callbacks (they are no longer by the compiler called anyway)

@back2dos comments?

@Simn Simn assigned Simn and unassigned ncannasse Apr 18, 2018

Simn added a commit to Simn/haxe that referenced this issue Apr 18, 2018

@Simn

This comment has been minimized.

Copy link
Member

commented Apr 18, 2018

Adding haxe.macro.Storage sounds complicated. How about supporting @:persistent static var x = 1; instead? I could support this easily by simply filtering these statics out so they are not reinitialized on context reuse, which would give the previous behavior.

@ncannasse

This comment has been minimized.

Copy link
Member Author

commented Apr 19, 2018

@Simn

This comment has been minimized.

Copy link
Member

commented Apr 19, 2018

Why does the initialization order matter here?

I would prefer a per-static metadata to a dynamic string map.

@ncannasse

This comment has been minimized.

Copy link
Member Author

commented May 5, 2018

I prefer an explicit API than a per-static medata but I guess we can have both. Moving to Release 4.0 as this is a breaking change.

@ncannasse ncannasse modified the milestones: Design, Release 4.0 May 5, 2018

@back2dos

This comment has been minimized.

Copy link
Member

commented May 5, 2018

Hmm, I'm not sure what to make of this. If you run with the server, side effects are generally persisted.

For example in tink_macro, there's this thing called BuildCache for @:genericBuild which allows making sure that MyGenericBuild<SomeType> always returns the same result. In essence it takes a Type->TypeDefinition function that is only run if there's no entry for the particular type. In that case, it does Context.defineType with the result. Which is quite the side effect - and we're not rolling that back on the next build (luckily). So I wonder what the rationale is behind doing it for static vars.

So I guess my question is: Why isn't the macro context reused all the time? In my experience the need to reset global state on recompilation is the exception and not the norm, because most global state changes reflect code that has been generated and persisted, so in the vast majority of cases you don't want to lose the global state corresponding to the code. Using Context.onGenerate to do house keeping is pretty comfortable (you can purge cache entries for types that are long gone and what not).

@Simn Simn modified the milestone: Release 4.0 May 8, 2018

@Simn

This comment has been minimized.

Copy link
Member

commented Sep 5, 2018

Still not sure how to proceed here.

@Simn

This comment has been minimized.

Copy link
Member

commented Oct 15, 2018

I talked with @frabbit about this and we agreed on going forward with the reset while allowing to tag statics to be kept.

@Simn Simn closed this in #7554 Nov 30, 2018

Simn added a commit that referenced this issue Nov 30, 2018

Reset static inits in macros when the context is reused (#7554)
* [eval] apply static inits on context reuse (see #5746)

* test modification of macro class too

* fix assertion position

* cause everytime we touch, I get this fstatSync

* allow @:persistent to avoid resetting statics

* remove reuse-callbacks

AlexHaxe added a commit to AlexHaxe/haxe-formatter that referenced this issue Dec 1, 2018

AlexHaxe added a commit to HaxeCheckstyle/haxe-formatter that referenced this issue Dec 1, 2018

expanded whitespace options for parens and braces, fixes #251 (#282)
* expanded whitespace options for parens and braces, fixes #251
* removed CompileTime.getAllClasses since it uses Context.onMacroContextReused (see HaxeFoundation/haxe#5746)

andyli added a commit to andyli/compiletime that referenced this issue Dec 2, 2018

@filt3rek filt3rek referenced this issue Jan 7, 2019

Closed

Haxe 4 updates #33

@sonygod sonygod referenced this issue Mar 1, 2019

Closed

support haxe4? #30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.