HaxeFoundation / haxe

Haxe - The Cross-Platform Toolkit
https://haxe.org
6.03k stars 648 forks source link

[macro] apply @:native for extern enum #11636

Closed kLabz closed 2 months ago

kLabz commented 2 months ago

Closes #11631

See also 944d4fdba9cce40d0e00c3a2495d0180e6e1f3c5 / #11481

Simn commented 2 months ago

Making this conditional seems strange if the real filters don't do the same. What's the rationale here?

kLabz commented 2 months ago

I'm actually having doubts too.

Rationale is that eval target does apply @:native, which is used for mbedtls extern enums.

I want the same to happen with macro context, but it's a bit more involved than I first thought because macro context does have both externs for current targets and eval externs...

Edit: Given that such externs would only work for std anyway (or maybe plugins? not sure that's still used tbh), we could use some trick there to identify "eval externs", but would be nice to do so in a not too hacky way

kLabz commented 2 months ago

Well.. technically "target externs" should not be loaded in macro context, but I can still see some problematic edge cases.

I don't see any with only doing this for extern enum, though.

Simn commented 2 months ago

So what exactly breaks if we just apply this to everything?

kLabz commented 2 months ago

11481 does

Simn commented 2 months ago

Ah, I completely forgot about that. In that case I think this change is fine, though it makes me wonder if a variant of #11481 with enum abstracts could exist.

kLabz commented 2 months ago

Turning into a draft while adding a test because it's not working properly for some reason atm :thinking:

Edit: right, it's enum abstract, not enum...