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

Hygiene fns and cleanup #6938

Merged
merged 19 commits into from
Jun 5, 2013
Merged

Conversation

jbclements
Copy link
Contributor

I'm close to flipping the switch on hygiene for let-bound identifiers. This commit adds a bunch of support functions for that change... but also a huge amount of cleanup in syntax.rc. The most interesting of these are

  • the use of TLS for the interners everywhere. We had already breached the "no-global-state" dam by using TLS for encoding, and it saves a lot of code just to use it everywhere.
    Also, there were many places where two or more interners were passed in attached to different structures, and the danger of having those diverge seemed greater that the danger of having a single one get big and heavy. If the interner size proves to be a problem, it should be quite simple to add a "parameterize"-like dynamic binding form--because we don't have interesting continuation operations (or tail calling, mostly) this should just be a case of a mutation followed by another later mutation. Again, this is only if the interner gets too big.
  • I renamed the "repr" field of the identifier to "name". I can see the case for "repr" when there's only one field in the structure, but that's no longer the case; there's now a name and a context (both are uints).
  • the interner now just maps between strings and uints, rather than between idents and uints. The former state made perfect sense when identifiers didn't have syntax contexts, but that's no longer the case.

I've run this patch against a fairly recent incoming, and it appears to pass all tests. Let's see if it can be merged....

bors added a commit that referenced this pull request Jun 5, 2013
…ements

I'm close to flipping the switch on hygiene for let-bound identifiers.  This commit adds a bunch of support functions for that change... but also a huge amount of cleanup in syntax.rc. The most interesting of these are

- the use of TLS for the interners everywhere.  We had already breached the "no-global-state" dam by using TLS for encoding, and it saves a lot of code just to use it everywhere.
Also, there were many places where two or more interners were passed in attached to different structures, and the danger of having those diverge seemed greater that the danger of having a single one get big and heavy. If the interner size proves to be a problem, it should be quite simple to add a "parameterize"-like dynamic binding form--because we don't have interesting continuation operations (or tail calling, mostly) this should just be a case of a mutation followed by another later mutation. Again, this is only if the interner gets too big.

- I renamed the "repr" field of the identifier to "name". I can see the case for "repr" when there's only one field in the structure, but that's no longer the case; there's now a name and a context (both are uints).

- the interner now just maps between strings and uints, rather than between idents and uints. The former state made perfect sense when identifiers didn't have syntax contexts, but that's no longer the case.

I've run this patch against a fairly recent incoming, and it appears to pass all tests. Let's see if it can be merged....
@bors bors closed this Jun 5, 2013
@bors bors merged commit 91b6526 into rust-lang:incoming Jun 5, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 8, 2021
Refactor types

r? `@flip1995`
This is the last PR to close rust-lang#6724 🎉
Also, this fixes rust-lang#6936.

changelog: `vec_box`: Fix FN in `const` or `static`
changelog: `linkedlist`: Fix FN in `const` or `static`
changelog: `option_option`: Fix FN in `const` or `static`
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