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

add TEnumIndex and make matcher generate that instead of Type.enumIndex call #6364

Merged
merged 1 commit into from
Jun 14, 2017

Conversation

nadako
Copy link
Member

@nadako nadako commented Jun 14, 2017

This adds TEnumIndex of texpr node and makes pattern-matcher generate it instead of Type.enumIndex call, as well as adds handling of that node to target generators. Adding this introduced some optimizations that eliminated a couple switches on some targets, also surfacing #6352 on C# (disabled the test for now).

I didn't remove older enum index hacks and Type.enumIndex for now, and there's no way to generate TEnumIndex from haxe code right now, so Type.enumIndex implementations are duplicated. I tried to add matching on EnumValueTools.getIndex() and using that for Type.enumIndex but that loses the enum expression type making it EnumValue instead, so this is something we have to think about.

Also, I have to check out the diffs again, but creating this PR nevertheless so Travis can show if everything is okay.

@nadako nadako requested a review from Simn June 14, 2017 11:49
@Simn
Copy link
Member

Simn commented Jun 14, 2017

It's missing here:

let e1 = match ctx.com.platform,e.eexpr with

But I'm fine with merging this as soon as the CI is green so we can take it from there.

@nadako
Copy link
Member Author

nadako commented Jun 14, 2017

No it's not :)

| TEnumIndex e1 ->

@nadako
Copy link
Member Author

nadako commented Jun 14, 2017

Huh, Lua is failing with some weird error Type not found : Issue6276

@Simn
Copy link
Member

Simn commented Jun 14, 2017

Ah I see. Looking forward to cleaning up that mess.

I wonder why it checks the type for TEnum in the current implementation.

@nadako nadako merged commit 94809e3 into development Jun 14, 2017
@nadako nadako deleted the tenumindex branch June 14, 2017 13:09
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.

None yet

2 participants