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

Remove some API from haxe.macro.Compiler #11540

Merged
merged 3 commits into from
Feb 3, 2024
Merged

Conversation

Simn
Copy link
Member

@Simn Simn commented Feb 3, 2024

Reasons follow:

removeField and setFieldType

I don't know what this was used for, but I'd like to think that we've moved beyond doing string-based patching operations like that.

addMetadata

This has always had some sort of issues that I don't remember right now, but it was replaced by addGlobalMetadata for a reason. In fact, I've changed it here to use that, in a way that makes sense to me. The only difference should be that isStatic is now ignored.

patchTypes

To my knowledge, this is ancient legacy from making flash externs. I don't think anyone even remembers the format this uses.

includeFile

According to my investigations and #6252, the position argument never worked properly. The lua generator only checked for "top", which means that "closure" would silently be ignored. Genjs does check both, so there's a chance that somebody was actually using the latter somehow and we're breaking this now. That's a risk I'm willing to take. I can't see how "inline" ever did anything because it only makes sense from a macro call, and that's the part that never worked in the first place.

Edit: One thing that maybe worked was using "inline" directly instead of IncludePosition.Inline... So I guess maybe this was used after all. I'll still try to remove it because I'd like to discourage putting js.Syntax.plainCode things in code.

Edit: Never mind, too controversial.


Will leave this open for a bit and see if somebody complains too loudly.

@Simn
Copy link
Member Author

Simn commented Feb 3, 2024

Ok I lied, when I said I'd leave this open I was mostly concerned about the includeFile changes. I definitely want to remove the patchTypes parts, so I'll go ahead and merge this!

@Simn Simn merged commit 4260da3 into development Feb 3, 2024
122 checks passed
@Simn Simn deleted the trim_haxe_macro_compiler branch February 3, 2024 08:48
@skial skial mentioned this pull request Feb 6, 2024
1 task
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

Successfully merging this pull request may close these issues.

1 participant