Ensure mir_drops_elaborated_and_const_checked when requiring codegen. #110107
Conversation
|
r? @b-naber (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
cjgillot
force-pushed
the
const-prop-lint-junk
branch
from
c7c516e to
34b763d
Compare
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
|
⌛ Trying commit 34b763de3f046b64ed2fdb3562fbf7ca4be79349 with merge d58481d6971e2fcdda6e99ceab5b37c90c87536d... |
|
☀️ Try build successful - checks-actions |
1 similar comment
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (d58481d6971e2fcdda6e99ceab5b37c90c87536d): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDBenchmarking 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 a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis 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.
CyclesResultsThis 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.
|
rustbot
added
perf-regression
|
Looks like we were already not running const prop lints on dead code? Or is it more complicated than that? If you want to preserve the "not running on dead code" behaviour while avoiding creating object files, you can probably just add another abort-if-errors after the monomorphization collector. |
|
That's just it: we didn't run the lint on dead functions, and we should. I think this PR is consistent with the goal stated in #108730, and can be adapted later once that PR is unblocked. |
|
r? @oli-obk |
|
@bors r+ |
|
📌 Commit 34b763de3f046b64ed2fdb3562fbf7ca4be79349 has been approved by It is now in the queue for this repository. |
bors
added
S-waiting-on-bors
|
⌛ Testing commit 34b763de3f046b64ed2fdb3562fbf7ca4be79349 with merge baa520c5a041ac4e29f403f375f9b075a1a07ef5... |
This comment has been minimized.
This comment has been minimized.
|
💔 Test failed - checks-actions |
cjgillot
force-pushed
the
const-prop-lint-junk
branch
from
cb601b6 to
4a51a3e
Compare
|
☔ The latest upstream changes (presumably #96840) made this pull request unmergeable. Please resolve the merge conflicts. |
cjgillot
force-pushed
the
const-prop-lint-junk
branch
from
4a51a3e to
7e214bf
Compare
|
@bors r+ |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (fa4cc63): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis 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.
CyclesResultsThis 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.
|
Mark-Simulacrum
added
the
perf-regression-triaged
|
#110107 (comment) seems like sufficient explanation for the regression, though parts of it may also be noise. |
Remove a test that wasn't carrying its weight See rust-lang#110107 (comment) for discussion. TLDR: it keeps breaking out of unrelated reasons and the real thing to check (early alloc ids can be tracked without ICEing miri) is hard to check otherwise.
Unions cannot have unsized fields, and as such, layout computation for unions asserts that each union field is sized (as this would normally have halted compilation earlier). However, if a generator ends up with an unsized local - a circumstance in which an error will always have been emitted earlier, for example, if attempting to dereference a `&str` - then the generator transform will produce a union with an unsized field. Since rust-lang#110107, later passes will be run, such as constant propagation, and can attempt layout computation on the generator, which will result in layout computation of `str` in the context of it being a field of a union - and so the aforementioned assertion would cause an ICE. It didn't seem appropriate to try and detect this case in the MIR body and skip this specific pass; tainting the MIR body or delaying a bug from the generator transform (or elsewhere) wouldn't prevent this either (as neither would prevent the later pass from running); and tainting when the deref of `&str` is reported, if that's possible, would unnecessarily prevent potential other errors from being reported later in compilation, and is very tailored to this specific case of getting a unsized type in a generator. Given that this circumstance can only happen when an error should have already been reported, the correct fix appears to be just changing the assert to a delayed bug. This will still assert if there is some circumstance where this occurs and no error has been reported, but it won't crash the compiler in this instance. Signed-off-by: David Wood <david@davidtw.co>
abi: unsized field in union - assert to delay bug Fixes rust-lang#113279. > Unions cannot have unsized fields, and as such, layout computation for unions asserts that each union field is sized (as this would normally have halted compilation earlier). > > However, if a generator ends up with an unsized local - a circumstance in which an error will always have been emitted earlier, for example, if attempting to dereference a `&str` - then the generator transform will produce a union with an unsized field. > > Since rust-lang#110107, later passes will be run, such as constant propagation, and can attempt layout computation on the generator, which will result in layout computation of `str` in the context of it being a field of a union - and so the aforementioned assertion would cause an ICE. > > It didn't seem appropriate to try and detect this case in the MIR body and skip this specific pass; tainting the MIR body or delaying a bug from the generator transform (or elsewhere) wouldn't prevent this either (as neither would prevent the later pass from running); and tainting when the deref of `&str` is reported, if that's possible, would unnecessarily prevent potential other errors from being reported later in compilation, and is very tailored to this specific case of getting a unsized type in a generator. > > Given that this circumstance can only happen when an error should have already been reported, the correct fix appears to be just changing the assert to a delayed bug. This will still assert if there is some circumstance where this occurs and no error has been reported, but it won't crash the compiler in this instance. While debugging this, I noticed a translation ICE in a delayed bug, so I fixed that too: > During borrowck, the `MultiSpan` from a buffered diagnostic is cloned and used to emit a delayed bug indicating a diagnostic was buffered - when the buffered diagnostic is translated, then the cloned `MultiSpan` may contain labels which can only render with the diagnostic's arguments, but the delayed bug being emitted won't have those arguments. Adds a function which clones `MultiSpan` without also cloning the contained labels, and use this function when creating the buffered diagnostic delayed bug.
mir_drops_elaborated_and_const_checked may emit errors while codegen has started, and the compiler would exit leaving object code files around.
Found by @cuviper in #109731