-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Always run tail_expr_drop_order
lint in promoted MIR query
#134493
Conversation
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
@@ -2,7 +2,6 @@ | |||
// This lint is to capture potential change in program semantics | |||
// due to implementation of RFC 3606 <https://github.com/rust-lang/rfcs/pull/3606> | |||
//@ edition: 2021 | |||
//@ build-fail |
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.
A build-fail
became a check-fail
, which is evidence enough of this being a sufficient fix.
@@ -2,5 +2,7 @@ | |||
//~| ERROR E0452 | |||
//~| ERROR E0452 | |||
//~| ERROR E0452 | |||
//~| ERROR E0452 |
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.
Duplicated attr errors is due to a call to lints_that_dont_need_to_run
in check-builds. This is deduplicated to the user, so it doesn't matter.
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
tail_expr_drop_order
lint on promoted MIRtail_expr_drop_order
lint in promoted MIR query
bors doesn’t seem to like a couple of errs PRs smh @bors ping |
😪 I'm awake I'm awake |
@bors try |
Reopening to try and fix bors |
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
@bors try |
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
Homu probably got wind of our attempts to get rid of him and is protesting. |
@bors try |
@bors try @rust-timer queue |
Finished benchmarking commit (b58df10): comparison URL. Overall result: ❌ regressions - 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 2.6%, secondary -3.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.
CyclesResults (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.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 767.394s -> 767.368s (-0.00%) |
I don't believe this is significant enough to warrant a fix. |
@bors r=oli-obk |
…oli-obk Always run `tail_expr_drop_order` lint in promoted MIR query Rather than running the lint at the beginning of `mir_drops_elaborated_and_const_checked`, run it at the end of `mir_promoted`. This should ensure that the lint gets picked up when running `cargo fix`, which runs with `--emit metadata` and therefore doesn't actually query `mir_drops_elaborated_and_const_checked`. We could probably push this down into `mir_built` too? but I don't really see a good reason to do so. rust-lang#132861 (comment) cc `@ehuss`
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
8ccdd12
to
b893221
Compare
@bors r=oli-obk |
☀️ Test successful - checks-actions |
Finished benchmarking commit (65fe42a): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary -2.3%, secondary -1.8%)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 -2.5%)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 sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 763.421s -> 763.437s (0.00%) |
Rather than running the lint at the beginning of
mir_drops_elaborated_and_const_checked
, run it at the end ofmir_promoted
. This should ensure that the lint gets picked up when runningcargo fix
, which runs with--emit metadata
and therefore doesn't actually querymir_drops_elaborated_and_const_checked
.We could probably push this down into
mir_built
too? but I don't really see a good reason to do so.#132861 (comment)
cc @ehuss