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

Phase for making identifiers globally unique #178

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

jenrik
Copy link
Member

@jenrik jenrik commented May 19, 2019

Uniqueify replaces identifiers for all named nodes with a unique labels. This can in some cases remove the need to keep track scopes.

Depends on #177

Generalized scoping handling into BaseScopedASTVisitor in order to make
ScopedASTVisitor and SymbolChecker consistent with each other.
Uniqueify replaces identifiers for all named nodes with a completely
unique label. This can in some cases remove the need to keep track of
scopes.
@NicEastvillage
Copy link
Contributor

NicEastvillage commented May 22, 2019

Can you give a small explaination of why this is needed?

You haven't added the phase to the compilation function(s). Is that intended?

And CI fails.

@jenrik
Copy link
Member Author

jenrik commented May 22, 2019

I need this as a prerequisite for the webassembly target. WebAssembly has very little in terms of scoping, for instance it has scopes for functions, but not for any of the control structures. This forces me to move variables of for loops up into the function scope, and having unique names would solve the annoying case where a variable in for loops initialization part shadows another variable in the function/state.

@jenrik
Copy link
Member Author

jenrik commented May 22, 2019

I should also note that I have run this phase through the tests, and it passes all except the few tests that check for specific variable names which it fails for obvious reasons.

@NicEastvillage
Copy link
Contributor

Sounds reasonable, but I wont approve before CircleCI passes. And I also don't want to submit this for our exam, so I wont approve it before that has been done.

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.

2 participants