-
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
Simplify for
loop desugar
#90352
Simplify for
loop desugar
#90352
Conversation
Some changes occurred in src/tools/clippy. cc @rust-lang/clippy |
r? @oli-obk (rust-highfive has picked a reviewer for you, use r? to override) |
@bors try @rust-timer queue |
⌛ Trying commit f91e0b1f918aa3b7f9ad897a742730fe50f5eee4 with merge 52f2d9b64faebfa33ae0fd45fcbf21bdc4154fa1... |
This comment has been minimized.
This comment has been minimized.
f91e0b1
to
b44eed9
Compare
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit b44eed913416e05c2a5f889c4a10d9ab3421e035 with merge 261bc72edbd828cafa27c3450ef92a59b5e7bacb... |
@camsteffen |
@petrochenkov It looks like the key points of history are #42265 and #42634. I think these are workarounds for typeck/borrowck issues that no longer exist, especially with NLL? |
This comment has been minimized.
This comment has been minimized.
☀️ Try build successful - checks-actions |
Queued 261bc72edbd828cafa27c3450ef92a59b5e7bacb with parent dd757b9, future comparison URL. |
Finished benchmarking commit (261bc72edbd828cafa27c3450ef92a59b5e7bacb): comparison url. Summary: This change led to very large relevant mixed results 🤷 in compiler performance.
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. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led 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 |
@@ -183,7 +183,7 @@ pub fn add_loop_label_to_break() { | |||
#[cfg(not(any(cfail1,cfail4)))] | |||
#[rustc_clean(cfg="cfail2", except="hir_owner_nodes")] | |||
#[rustc_clean(cfg="cfail3")] | |||
#[rustc_clean(cfg="cfail5", except="hir_owner_nodes")] | |||
#[rustc_clean(cfg="cfail5", except="hir_owner_nodes, optimized_mir")] |
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.
Why is optimized_mir
dirty?
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.
I don't know. Do you know how to see what changed?
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.
Perhaps it's an optimization of
match next {
None => break,
Some(..) => break,
}
rust/library/std/src/keyword_docs.rs Line 536 in 85c0558
|
c3c982d
to
a60977b
Compare
51ee323
to
21a53d1
Compare
@bors r=oli-obk |
📌 Commit 21a53d1b1ba896d06534207402885fb93ffd1da9 has been approved by |
⌛ Testing commit 21a53d1b1ba896d06534207402885fb93ffd1da9 with merge 22453953cf17d8d767864fd79f96aeae798d6542... |
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
21a53d1
to
66da8fa
Compare
@bors r=oli-obk |
📌 Commit 66da8fa has been approved by |
☀️ Test successful - checks-actions |
Finished benchmarking commit (cebd2dd): comparison url. Summary: This change led to very large relevant mixed results 🤷 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression |
@rustbot label: +perf-regression-triaged |
Basically two intermediate bindings are inlined. I could have left one intermediate binding in place as this would simplify some diagnostic logic, but I think the difference in that regard would be negligible, so it is better to have a minimal HIR.
For checking that the pattern is irrefutable, I added a special case when the
match
is found to be non-exhaustive.The reordering of the arms is purely stylistic. I don't think there are any perf implications.