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

MacroApi cleanup for Haxe 5 #11433

Merged
merged 10 commits into from
Nov 27, 2024
Merged

MacroApi cleanup for Haxe 5 #11433

merged 10 commits into from
Nov 27, 2024

Conversation

Simn
Copy link
Member

@Simn Simn commented Dec 18, 2023

Still some TODOs.

Closes #11431

@skial skial mentioned this pull request Dec 24, 2023
1 task
@back2dos
Copy link
Member

back2dos commented Feb 3, 2024

Since adding optional fields always incurs a marginal risk for breaking user code, might I use this opportunity to request also adding the positions of other names currently not exposed?

  1. The position of the type name in BaseType and TypeDefinition.
  2. The position of the field name in Field, ObjectField, EnumField and ClassField.

I see that some of these are already encoded in an unspecified name_pos (i.e. in ObjectField and EnumField as far as I can tell), but it would be nice to have proper access and generally be able to define / refer to finer grained positions for the sake of not coloring everything red.

FWIW I think in the types from haxe.macro.Type it could even be non-optional, because to the best of my knowledge constructing types by hand is not really a thing.

@Simn
Copy link
Member Author

Simn commented Feb 3, 2024

I'll have to check all this again, I thought this name_pos business was mostly for enum instances, where we can't access the value from Haxe anyway. But yes I'd like to clean all this up and provide all the information to macros.

# Conflicts:
#	src/typing/macroContext.ml
kLabz added a commit that referenced this pull request May 15, 2024
Simn pushed a commit that referenced this pull request May 15, 2024
* [macro] Don't choke on namePos for reification pattern matching

* [tests] Add test for 11670

* Do it like in #11433
kLabz added a commit that referenced this pull request Jun 28, 2024
* [macro] Don't choke on namePos for reification pattern matching

* [tests] Add test for 11670

* Do it like in #11433
@kLabz kLabz added this to the Release 5.0 milestone Jun 28, 2024
kLabz added a commit that referenced this pull request Jul 18, 2024
* [macro] Don't choke on namePos for reification pattern matching

* [tests] Add test for 11670

* Do it like in #11433
# Conflicts:
#	src/typing/matcher/exprToPattern.ml
@Simn Simn marked this pull request as ready for review November 25, 2024 06:55
@Simn
Copy link
Member Author

Simn commented Nov 25, 2024

@yuxiaomao Could you check this one too? Thanks!

@yuxiaomao
Copy link
Contributor

It didn't break anything as far as I see, however in one of the projects I'm getting a warning many times (60+) at the same location in hxbit (https://github.com/HeapsIO/hxbit/blob/010d65702548fb2547cc7f6113910fcef4f68731/hxbit/Macros.hx#L561 at hxbit.Serializable.SerializableEnum<$et>):

`@:extern` is deprecated in favor of `extern`

Which is not the case with current development branch (with development, there is no warning at all). I'm not familiar with macro nor hxbit, and with -D dump I can't find any occurrences of @:extern, so I'm confused :<
In the code of hxbit/Macros.hx there is some meta : [{name:":extern",pos:pos}], so I think it's expected and this PR just makes it visible.

@Simn
Copy link
Member Author

Simn commented Nov 25, 2024

This could come from something that used to have null_pos now having a proper position. Although it's not immediately clear to me why all positions would point to the same reification location. Still this is likely something to fix in the macro, as you say.

@Simn Simn merged commit 35536f3 into development Nov 27, 2024
99 checks passed
@Simn Simn deleted the macroApi_cleanup_2023 branch November 27, 2024 07:10
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.

Regression wrt complex type positions
4 participants