Replace Idents with Names outside of libsyntax + Idents/Names cleanup (Part 2) #28642
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Make sure Name, SyntaxContext and Ident are passed by value Make sure Idents don't serve as keys (or parts of keys) in maps, Ident comparison is not well defined
|
Could you file issues for the remaining work you identified please? (It sounds really useful and I wouldn't want it to be forgotten). |
|
@bors: r+ |
|
📌 Commit f284cbc has been approved by |
This was referenced Sep 25, 2015
|
@nrc |
bors
added a commit
that referenced
this pull request
Sep 26, 2015
This PR removes random remaining `Ident`s outside of libsyntax and performs general cleanup In particular, interfaces of `Name` and `Ident` are tidied up, `Name`s and `Ident`s being small `Copy` aggregates are always passed to functions by value, and `Ident`s are never used as keys in maps, because `Ident` comparisons are tricky. Although this PR closes #6993 there's still work related to it: - `Name` can be made `NonZero` to compress numerous `Option<Name>`s and `Option<Ident>`s but it requires const unsafe functions. - Implementation of `PartialEq` on `Ident` should be eliminated and replaced with explicit hygienic, non-hygienic or member-wise comparisons. - Finally, large parts of AST can potentially be converted to `Name`s in the same way as HIR to clearly separate identifiers used in hygienic and non-hygienic contexts. r? @nrc
|
Why was |
|
@Manishearth |
|
Yeah, but if it's doing something that should be explicit that's fine by me. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes random remaining
Idents outside of libsyntax and performs general cleanupIn particular, interfaces of
NameandIdentare tidied up,Names andIdents being smallCopyaggregates are always passed to functions by value, andIdents are never used as keys in maps, becauseIdentcomparisons are tricky.Although this PR closes #6993 there's still work related to it:
Namecan be madeNonZeroto compress numerousOption<Name>s andOption<Ident>s but it requires const unsafe functions.PartialEqonIdentshould be eliminated and replaced with explicit hygienic, non-hygienic or member-wise comparisons.Names in the same way as HIR to clearly separate identifiers used in hygienic and non-hygienic contexts.r? @nrc