Skip to content

typeck: catch continues pointing to blocks #141317

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

Merged
merged 1 commit into from
May 21, 2025

Conversation

dianne
Copy link
Contributor

@dianne dianne commented May 21, 2025

This taints the typeck results with errors if a continue is found not pointing to a loop.

A few things were going wrong here. First, since this wasn't caught in typeck, we'd end up building the THIR and then running liveness lints on ill-formed HIR. Since liveness assumes all continues point to loops, it wasn't setting a live node for the continue's destination. There was a fallback for if it couldn't retrieve that live node, but it was faulty; it would create a new live node to represent an erroneous state after the analysis's RWU table had already been built. This would ICE if the new live node was used in operations, such as merging results from the arms of a match. I've removed this error-recovery since it was buggy, and we should really catch bad labels before liveness.

I've also replaced an outdated comment about when liveness lints are run. At this point, I think the call to check_liveness could be moved elsewhere, but if it can be run when the typeck results are tainted by errors, it'll need some slight refactoring so it can bail out in that case. In lieu of that, I've added an assert.

Fixes #113379
Fixes #121623

This taints the typeck results with errors if a `continue` is found not
pointing to a loop, which fixes an ICE.

A few things were going wrong here. First, since this wasn't caught in
typeck, we'd end up building the THIR and then running liveness lints on
ill-formed HIR. Since liveness assumes all `continue`s point to loops,
it wasn't setting a live node for the `continue`'s destination. However,
the fallback for this was faulty; it would create a new live node to
represent the erroneous state after the analysis's RWU table had already
been built. This would ICE if the new live node was used in operations,
such as merging results from the arms of a match. I've removed this
error-recovery since it was buggy, and we should really catch bad labels
before liveness.

I've also replaced an outdated comment about when liveness lints are
run. At this point, I think the call to `check_liveness` could be moved
elsewhere, but if it can be run when the typeck results are tainted by
errors, it'll need some slight refactoring so it can bail out in that
case. In lieu of that, I've added an assertion.
@rustbot
Copy link
Collaborator

rustbot commented May 21, 2025

r? @Nadrieril

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 21, 2025
@rustbot
Copy link
Collaborator

rustbot commented May 21, 2025

This PR changes a file inside tests/crashes. If a crash was fixed, please move into the corresponding ui subdir and add 'Fixes #' to the PR description to autoclose the issue upon merge.

@dianne
Copy link
Contributor Author

dianne commented May 21, 2025

Not tagging the guard patterns tracking issue, but I'll be basing my PR for liveness analysis for guard patterns on this. The way I'm checking liveness for guard patterns in fn params ended up surfacing this bug in an another test (tests/ui/label/label_break_value_continue.rs).

@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented May 21, 2025

📌 Commit ed01a20 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 21, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request May 21, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#137759 (Add `std::os::unix::process::CommandExt::chroot` to safely chroot a child process)
 - rust-lang#140994 (replace `cc_detect::cc2ar` with `cc::try_get_archiver`)
 - rust-lang#141213 (Suggest use "{}", self.x instead of {self.x} when resolve x as field of `self`)
 - rust-lang#141283 (Allow `x perf` to find rustc.exe on Windows)
 - rust-lang#141284 (Allow trailing comma after argument in query definition)
 - rust-lang#141317 (typeck: catch `continue`s pointing to blocks)
 - rust-lang#141318 (Avoid creating an empty identifer in `Symbol::to_ident_string`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit aaa684f into rust-lang:master May 21, 2025
6 checks passed
@rustbot rustbot added this to the 1.89.0 milestone May 21, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 21, 2025
Rollup merge of rust-lang#141317 - dianne:continue-liveness-ice-fix, r=compiler-errors

typeck: catch `continue`s pointing to blocks

This taints the typeck results with errors if a `continue` is found not pointing to a loop.

A few things were going wrong here. First, since this wasn't caught in typeck, we'd end up building the THIR and then running liveness lints on ill-formed HIR. Since liveness assumes all `continue`s point to loops, it wasn't setting a live node for the `continue`'s destination. There was a fallback for if it couldn't retrieve that live node, but it was faulty; it would create a new live node to represent an erroneous state after the analysis's RWU table had already been built. This would ICE if the new live node was used in operations, such as merging results from the arms of a match. I've removed this error-recovery since it was buggy, and we should really catch bad labels before liveness.

I've also replaced an outdated comment about when liveness lints are run. At this point, I think the call to `check_liveness` could be moved elsewhere, but if it can be run when the typeck results are tainted by errors, it'll need some slight refactoring so it can bail out in that case. In lieu of that, I've added an assert.

Fixes rust-lang#113379
Fixes rust-lang#121623
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
5 participants