rustc_parse: Remove Parser::normalized(_prev)_token by petrochenkov · Pull Request #69801 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation34 Commits6 Checks0 Files changed
Conversation
This file contains hidden or 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
[ Show hidden characters]({{ revealButtonHref }})
⌛ Trying commit f572d400edc7f52ce6597f359b291dc1322de3c7 with merge ac2bf9054006fee0f39772b8daad9845a9fa570f...
☀️ Try build successful - checks-azure
Build commit: ac2bf9054006fee0f39772b8daad9845a9fa570f (ac2bf9054006fee0f39772b8daad9845a9fa570f)
Awaiting bors try build completion
@rust-timer build ac2bf9054006fee0f39772b8daad9845a9fa570f
Finished benchmarking try commit ac2bf9054006fee0f39772b8daad9845a9fa570f, comparison URL.
Haven't reviewed the PR yet, but those numbers seem like a slight regression overall, although with some improvements?
I expected the perf diff to be within the noise.
Let's check whether it's reproducible.
@bors try @rust-timer queue
Awaiting bors try build completion
bors added a commit that referenced this pull request
☀️ Try build successful - checks-azure
Build commit: 5f66c2a (5f66c2afe2e46c53471539710934a5c1ab1d4ce5)
Yeah, the perf diff is noise.
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.
| @@ -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 { |
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.
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.
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. :)
📌 Commit 7a30bb1 has been approved by Centril
🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened
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
bors added a commit that referenced this pull request
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
Centril added a commit to Centril/rust that referenced this pull request
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
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
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.