-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
ast_lowering: Introduce lower_span
for catching all spans entering HIR
#88208
Conversation
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit a611199277e1ee5f0af204d9d4132ee6c65ce811 with merge 6f99ad894b945a464742e58a6d3cbae7b7a382a6... |
☀️ Try build successful - checks-actions |
Queued 6f99ad894b945a464742e58a6d3cbae7b7a382a6 with parent db002a0, future comparison URL. |
Finished benchmarking try commit (6f99ad894b945a464742e58a6d3cbae7b7a382a6): comparison url. Summary: This benchmark run did not return any relevant changes. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. @bors rollup=never |
📌 Commit a611199277e1ee5f0af204d9d4132ee6c65ce811 has been approved by |
It seems like it would be very easy to forget to call |
Not setting the span's |
Normally, we would only have a spurious red DepNode and query recomputation. |
I am not really the most neutral reviewer as I essentially wrote the same code. OTOH, this PR is harmless, up to making LLVM chew more code. @Aaron1011 do you have any concern about merging this? |
This comment has been minimized.
This comment has been minimized.
r? @Aaron1011 |
This comment has been minimized.
This comment has been minimized.
r=me with the merge conflicts fixed |
@bors r=Aaron1011 |
📌 Commit 59013cd has been approved by |
☀️ Test successful - checks-actions |
This PR cherry-picks the
fn lower_span
change from #84373.I also introduced
fn lower_ident
for lowering spans in identifiers, and audited places where HIR structures with spans or identifiers are constructed and added a few missinglower_span
s/lower_ident
s.Having a hook for spans entering HIR can be useful for things other than #84373, e.g. #35148.
I also want to check whether this change causes perf regressions due to some accidental inlining issues.
r? @cjgillot