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

parser: Keep current and previous tokens precisely #69006

Merged
merged 1 commit into from
Feb 10, 2020

Conversation

petrochenkov
Copy link
Contributor

...including their unnormalized forms.
Add more documentation for them.

Hopefully, this will help to eliminate footguns like #68728 (comment).

I'll try to address the FIXMEs in separate PRs during the next week.

r? @Centril

including their unnormalized forms.
Add more documentation for them.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 9, 2020
prev_token_kind: PrevTokenKind,
/// Equivalent to `unnormalized_prev_token().span`.
/// FIXME: Remove in favor of `(unnormalized_)prev_token().span`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used so frequently that we might want self.prev_span() (unless those occurrences are meant to be replaced with self.prev_token.span).

@Centril
Copy link
Contributor

Centril commented Feb 10, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Feb 10, 2020

📌 Commit cd7a428 has been approved by Centril

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 10, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 10, 2020
parser: Keep current and previous tokens precisely

...including their unnormalized forms.
Add more documentation for them.

Hopefully, this will help to eliminate footguns like rust-lang#68728 (comment).

I'll try to address the FIXMEs in separate PRs during the next week.

r? @Centril
bors added a commit that referenced this pull request Feb 10, 2020
Rollup of 6 pull requests

Successful merges:

 - #68694 (Reduce the number of `RefCell`s in `InferCtxt`.)
 - #68966 (Improve performance of coherence checks)
 - #68976 (Make `num::NonZeroX::new` an unstable `const fn`)
 - #68992 (Correctly parse `mut a @ b`)
 - #69005 (Small graphviz improvements for the new dataflow framework)
 - #69006 (parser: Keep current and previous tokens precisely)

Failed merges:

r? @ghost
@bors bors merged commit cd7a428 into rust-lang:master Feb 10, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 12, 2020
parser: Remove `Parser::prev_token_kind`

Follow-up to rust-lang#69006.
r? @Centril
Centril added a commit to Centril/rust that referenced this pull request Feb 18, 2020
parser: Simplify treatment of macro variables in `Parser::bump`

Follow-up to rust-lang#69006.

Token normalization for `$ident` and `$lifetime` is merged directly into `bump`.
Special "unknown macro variable" diagnostic for unexpected `$`s is removed as preventing legal code from compiling (as a result `bump` also doesn't call itself recursively anymore and can't make `prev_token` inconsistent).

r? @Centril
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 23, 2020
parser: Cleanup `Parser::bump_with` and its uses

Follow-up to rust-lang#69006.
r? @Centril
Centril added a commit to Centril/rust that referenced this pull request Feb 28, 2020
parser: `token` -> `normalized_token`, `nonnormalized_token` -> `token`

So, after rust-lang#69006, its follow-ups and an attempt to remove `Parser::prev_span` I came to the conclusion that the unnormalized token and its span is what you want in most cases, so it should be default.

Normalization only makes difference in few cases where we are checking against `token::Ident` or `token::Lifetime` specifically.
This PR uses `normalized_token` for those cases.

Using normalization explicitly means that people writing code should remember about `NtIdent` and `NtLifetime` in general. (That is alleviated by the fact that `token.ident()` and `fn parse_ident_*` are already written.)
Remembering about `NtIdent`, was, however, already the case, kind of, because the implicit normalization was performed only for the current/previous token, but not for things like `look_ahead`.
As a result, most of token classification methods in `token.rs` already take `NtIdent` into account (this PR fixes a few pre-existing minor mistakes though).

The next step is removing `normalized(_prev)_token` entirely and replacing it with `token.ident()` (mostly) and `token.normalize()` (occasionally).
I want to make it a separate PR for that and run it though perf.
`normalized_token` filled on every bump has both a potential to avoid repeated normalization, and to do unnecessary work in advance (it probably doesn't matter anyway, the normalization is very cheap).

r? @Centril
bors added a commit that referenced this pull request Mar 8, 2020
rustc_parse: Remove `Parser::normalized(_prev)_token`

Perform the "normalization" (renamed to "uninterpolation") on the fly when necessary.

The final part of #69579 #69384 #69376 #69211 #69034 #69006.
r? @Centril
Centril added a commit to Centril/rust that referenced this pull request Mar 9, 2020
rustc_parse: Remove `Parser::normalized(_prev)_token`

Perform the "normalization" (renamed to "uninterpolation") on the fly when necessary.

The final part of rust-lang#69579 rust-lang#69384 rust-lang#69376 rust-lang#69211 rust-lang#69034 rust-lang#69006.
r? @Centril
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants