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

rustc_parse: Remove Parser::normalized(_prev)_token #69801

Merged
merged 6 commits into from
Mar 9, 2020

Conversation

petrochenkov
Copy link
Contributor

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

The final part of #69579 #69384 #69376 #69211 #69034 #69006.
r? @Centril

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 7, 2020
@petrochenkov
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented Mar 7, 2020

⌛ Trying commit f572d400edc7f52ce6597f359b291dc1322de3c7 with merge ac2bf9054006fee0f39772b8daad9845a9fa570f...

@petrochenkov petrochenkov added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 7, 2020
@bors
Copy link
Contributor

bors commented Mar 7, 2020

☀️ Try build successful - checks-azure
Build commit: ac2bf9054006fee0f39772b8daad9845a9fa570f (ac2bf9054006fee0f39772b8daad9845a9fa570f)

@petrochenkov
Copy link
Contributor Author

@rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@petrochenkov
Copy link
Contributor Author

@rust-timer build ac2bf9054006fee0f39772b8daad9845a9fa570f

@rust-timer
Copy link
Collaborator

Queued ac2bf9054006fee0f39772b8daad9845a9fa570f with parent a039217, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit ac2bf9054006fee0f39772b8daad9845a9fa570f, comparison URL.

@Centril
Copy link
Contributor

Centril commented Mar 8, 2020

Haven't reviewed the PR yet, but those numbers seem like a slight regression overall, although with some improvements?

@petrochenkov
Copy link
Contributor Author

I expected the perf diff to be within the noise.
Let's check whether it's reproducible.

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Mar 8, 2020

⌛ Trying commit bfc7502 with merge 5f66c2a...

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
@bors
Copy link
Contributor

bors commented Mar 8, 2020

☀️ Try build successful - checks-azure
Build commit: 5f66c2a (5f66c2afe2e46c53471539710934a5c1ab1d4ce5)

@rust-timer
Copy link
Collaborator

Queued 5f66c2a with parent f943349, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 5f66c2a, comparison URL.

@petrochenkov
Copy link
Contributor Author

Yeah, the perf diff is noise.

@petrochenkov petrochenkov removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 8, 2020
Copy link
Contributor

@Centril Centril left a comment

Choose a reason for hiding this comment

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

r=me with these comments considered.

src/librustc_expand/mbe/macro_parser.rs Outdated Show resolved Hide resolved
src/librustc_parse/parser/expr.rs Outdated Show resolved Hide resolved
src/librustc_parse/parser/expr.rs Outdated Show resolved Hide resolved
src/librustc_parse/parser/item.rs Outdated Show resolved Hide resolved
src/librustc_ast/token.rs Outdated Show resolved Hide resolved
src/librustc_ast/token.rs Outdated Show resolved Hide resolved
@@ -714,6 +726,24 @@ pub enum Nonterminal {
#[cfg(target_arch = "x86_64")]
rustc_data_structures::static_assert_size!(Nonterminal, 40);

impl Nonterminal {
fn span(&self) -> Span {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should add trait HasSpan { ... } for this. I've felt the need for it in reviewing / writing other PRs as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe.
It's not too helpful for this PR though because all the uses of spans are concrete.
I'm not sure where exactly it would be useful, actually.

Copy link
Contributor

Choose a reason for hiding this comment

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

Mainly where you have slices of objects with spans, and you want to have "first to last"; it seems to occur sometimes. I agree though that for the purposes of this PR it doesn't add much, so let's defer this to some other PR. :)

src/librustc_parse/parser/mod.rs Show resolved Hide resolved
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 9, 2020
@petrochenkov
Copy link
Contributor Author

Updated.

@Centril
Copy link
Contributor

Centril commented Mar 9, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Mar 9, 2020

📌 Commit 7a30bb1 has been approved by Centril

@bors
Copy link
Contributor

bors commented Mar 9, 2020

🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 9, 2020
bors added a commit that referenced this pull request Mar 9, 2020
Rollup of 6 pull requests

Successful merges:

 - #69201 (Permit attributes on 'if' expressions)
 - #69685 (unix: Don't override existing SIGSEGV/BUS handlers)
 - #69762 (Ensure that validity only raises validity errors)
 - #69779 (librustc_codegen_llvm: Use slices in preference to 0-terminated strings)
 - #69801 (rustc_parse: Remove `Parser::normalized(_prev)_token`)
 - #69842 (Add more regression tests)

Failed merges:

r? @ghost
@bors bors merged commit 2677d59 into rust-lang:master Mar 9, 2020
Centril added a commit to Centril/rust that referenced this pull request Mar 11, 2020
parse: Tweak the function parameter edition check

Follow-up to rust-lang#69801.

Edition of a code fragment is inferred from "the place where the code is written".
For individual tokens like edition-specific keywords it may be the span of the token itself ("uninterpolated" span), but for larger code fragments it's probably not, in the test example the trait method is obviously written in "2015 edition code".

r? @Centril
Centril added a commit to Centril/rust that referenced this pull request Mar 11, 2020
parse: Tweak the function parameter edition check

Follow-up to rust-lang#69801.

Edition of a code fragment is inferred from "the place where the code is written".
For individual tokens like edition-specific keywords it may be the span of the token itself ("uninterpolated" span), but for larger code fragments it's probably not, in the test example the trait method is obviously written in "2015 edition code".

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.

5 participants