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] Detect some invalid usages of the haxe.macro.Compiler API #7871

Closed
Simn opened this issue Feb 26, 2019 · 7 comments · Fixed by #11323
Closed

[macro] Detect some invalid usages of the haxe.macro.Compiler API #7871

Simn opened this issue Feb 26, 2019 · 7 comments · Fixed by #11323
Labels
discussion platform-macro Everything related to Haxe macros
Milestone

Comments

@Simn
Copy link
Member

Simn commented Feb 26, 2019

At the moment, the haxe.macro.Compiler API has various functions that modify compilation parameters. However, they can be freely used from any random macro, which I don't think was the original intent.

For instance, using Compiler.addClassPath in the middle of a compilation could lead to very weird situations where some parts of a package are resolved in one class path and other parts are resolved in another class path. This coupled with the undefined build order is almost certainly not what anyone wants.

In similar fashion, using Compiler.define at some random point in time causes a hot mess in the compilation server because suddenly the define hash changed.

I think we should restrict these operations to initialization macros. However, I don't know how to transition this properly.

Another problem

is that the haxe.macro.Compiler API itself can cause typing. For instance, include uses Context.getModule. This is pretty bad because we never reach a proper "configuration done" state due to this.

Ideally, we would have a properly defined order:

  1. Run initialization macros. They are not allowed to cause any immediate typing. If you want to type something, register a callback.
  2. Run everything else, but don't access the parts of haxe.macro.Compiler which modifies compilation parameters.

Any thoughts?

@Simn Simn added discussion platform-macro Everything related to Haxe macros labels Feb 26, 2019
@Simn Simn added this to the Release 4.1 milestone Feb 26, 2019
@Simn
Copy link
Member Author

Simn commented Mar 3, 2019

If we can address the "Another problem", we could further improve the compilation server by not initializing the outside context before running initialization macros. At the moment, we add a bunch of modules immediately when creating the typer: StdTypes, String, Array, haxe.EnumTools. If an initialization macro adds a define, these types will forever remain unused on the compilation server.

Ideally, we would do this:

  1. Create typer context but don't type anything into it
  2. Run initialization macros
  3. Populate typer context with the standard types mentioned above

The one place I can think of where we need types is when typing constant macro arguments. However, we should be able to simply do this in the macro context instead of the outside context.

In vshaxe, we have 4 before_init_macros contexts which just sit there and waste memory.

@Simn
Copy link
Member Author

Simn commented Mar 3, 2019

How about we add a value to the macro define so it's macro = "init" | "build" | "expr". Then we can add custom checks like if (Context.definedValue("macro") != "build") error("getBuildFields can only be called inside build-macros").

@RealyUniqueName
Copy link
Member

Being able to type arbitrary things at init macro is nice if one wants to configure something based on the code in project.
Maybe the problem is that macro context of init macros is reused for other kinds of macros. E.g. ctx.g.modules is shared among different kinds of macros, but compilation config changes during init macros. That makes types to be loaded again with a different cache signature. But since they are already added to ctx.g.modules the "X redefined from X" error occurs.
I think that could happen even in init macros before compilation proceeds to "normal" steps: 1. Type1 is loaded for init macros; 2. Compiler.define('whatever') is called; 3. Type2 is loaded for init macros and it depends on Type1, so Type1 is loaded again because cache signature is changed with the new define.

So how about another approach:

  1. Have a completely separate context for init macros
  2. Delay any config changes until init macros are finished

Compilation flow would look like this:
Init macros (accumulate config changes) -> apply config changes -> compilation

@Simn
Copy link
Member Author

Simn commented Jul 21, 2019

Note that the problem isn't the macro context, it's the compilation context.

@RealyUniqueName
Copy link
Member

We have so many contexts in the code, I just mix them together :)
What I meant is typer context, common context and other contexts should be separated for init macros and other kinds of macros.

@RealyUniqueName
Copy link
Member

And yes, methods changing compilation configuration should only be available in init macros.

@Pidroh
Copy link

Pidroh commented Mar 15, 2022

Any time previsions for this one? The 'redefined' problem is a bit of a pain in the ass :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion platform-macro Everything related to Haxe macros
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants