Skip to content

Simplify PatIdent to contain an Ident rather than a Path #15327

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from

Conversation

jbclements
Copy link
Contributor

Rationale: for what appear to be historical reasons only, the PatIdent contains
a Path rather than an Ident. This means that there are many places in the code
where an ident is artificially promoted to a path, and---much more problematically---
a bunch of elements from a path are simply thrown away, which seems like an invitation
to some really nasty bugs.

This commit replaces the Path in a PatIdent with a Path1, which just contains an ident
and a span.

Bonus! this patch should make the compiler use slightly less memory and be slightly faster, and
represents a net removal of about 100 lines of code from the repo.

@alexcrichton
Copy link
Member

We also have a SpannedIdent typedef in syntax::ast, perhaps these two could be merged?

@jbclements
Copy link
Contributor Author

@alexcrichton nooooo! Sigh. What I meant to say was: yes, that's obviously the right solution. Lemme go fix that.... :) .

Rationale: for what appear to be historical reasons only, the PatIdent contains
a Path rather than an Ident.  This means that there are many places in the code
where an ident is artificially promoted to a path, and---much more problematically---
a bunch of elements from a path are simply thrown away, which seems like an invitation
to some really nasty bugs.

This commit replaces the Path in a PatIdent with a SpannedIdent, which just contains an ident
and a span.
bors added a commit that referenced this pull request Jul 3, 2014
Closes #15276 (Guide: if)
Closes #15280 (std::os - Add join_paths, make setenv non-utf8 capable)
Closes #15314 (Guide: functions)
Closes #15327 (Simplify PatIdent to contain an Ident rather than a Path)
Closes #15340 (Guide: add mutable binding section)
Closes #15342 (Fix ICE with nested macro_rules!-style macros)
Closes #15350 (Remove duplicated slash in install script path)
Closes #15351 (correct a few spelling mistakes in the tutorial)
Closes #15352 (librustc: Have the kind checker check sub-bounds in trait casts.)
Closes #15359 (Fix spelling errors.)
Closes #15361 (Rename set_broadast() to set_broadcast().)
Closes #15366 (Simplify creating a parser from a token tree)
Closes #15367 (Add examples for StrVector methods)
Closes #15372 (Vec::grow should use reserve_additional, Vec::reserve should check against capacity)
Closes #15373 (Fix minor issues in the documentation of libtime.)
@bors bors closed this in #15377 Jul 3, 2014
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants