-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Move ast::Item::ident
into ast::ItemKind
#138740
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
Conversation
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
…try> Move `ast::Item::ident` into `ast::ItemKind` r? `@ghost`
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (960f748): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary -0.7%, secondary -1.1%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeResults (primary -0.2%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Bootstrap: 773.468s -> 775.556s (0.27%) |
Performance is basically neutral. If anything, the improvements slightly outweigh the regressions, but all changes are very small. @rustbot label: +perf-regression-triaged |
87677af
to
c977a9d
Compare
Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
@fmease: thanks for reviewing, and apologies for the size of the final commit. I can move the four preliminary commits into a separate PR if that would be helpful. |
☔ The latest upstream changes (presumably #138923) made this pull request unmergeable. Please resolve the merge conflicts. |
c977a9d
to
8ba9610
Compare
I rebased. |
☔ The latest upstream changes (presumably #138933) made this pull request unmergeable. Please resolve the merge conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me after rebase and important nitpicks addressed. I like it very much, thanks!
I also posted a bunch of stylistic suggestions some of which probably aren't that important in retrospect. Feel free to ignore them (or address them) at your discretion. I don't want to require unnecessary work
This PR is super bitrotty. |
`global_allocator_spans` and `alloc_error_handler_span` are identical except for `name`.
Instead of putting the item inside it, just pass the ident and visibility (the only things needed) alongside it where necessary. This helps with the next commit, which will move the ident's location. Specifically, it gets rid of the `match visitor_kind` in `rewrite_type_alias`.
8ba9610
to
ec10833
Compare
Some changes occurred in compiler/rustc_builtin_macros/src/autodiff.rs cc @ZuseZ4 |
Thanks for the detailed review. I made all the changes you suggested except where I mentioned otherwise. Should be good to go. @bors r=fmease |
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 97ea17b (parent) -> ed20157 (this PR) Test differencesShow 66 test diffsAdditionally, 66 doctest diffs were found. These are ignored, as they are noisy. Job group index Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
Finished benchmarking commit (ed20157): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary -0.3%, secondary 2.2%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (secondary -3.2%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeResults (primary -0.2%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Bootstrap: 773.194s -> 773.019s (-0.02%) |
- fix errors caused by the move of `ast::Item::ident` (see rust-lang#138740) - move the logic of getting `sig`, `vis`, and `ident` from two seperate `match` statements into one (less repetition especially with the nested `match`)
…mease Move `ast::Item::ident` into `ast::ItemKind` The follow-up to rust-lang#138384, which did the same thing for `hir::ItemKind`. r? `@fmease`
I accidentally added this in rust-lang#138740.
- fix errors caused by the move of `ast::Item::ident` (see rust-lang#138740) - move the logic of getting `sig`, `vis`, and `ident` from two seperate `match` statements into one (less repetition especially with the nested `match`)
Ident was moved to the struct Fn in rust-lang/rust#138740
This PR update the sample code in the development guide because `ident` was moved to the struct `Fn` in rust-lang/rust#138740 . changelog: none
The follow-up to #138384, which did the same thing for
hir::ItemKind
.r? @fmease