From ed01a205144f53741534611a74d40487f611cdea Mon Sep 17 00:00:00 2001 From: dianne Date: Tue, 20 May 2025 17:27:38 -0700 Subject: [PATCH] typeck: catch `continue`s pointing to blocks 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. --- compiler/rustc_hir_typeck/src/expr.rs | 34 +++++++++++---- compiler/rustc_mir_build/src/builder/mod.rs | 3 +- compiler/rustc_passes/src/liveness.rs | 10 +++-- tests/crashes/113379.rs | 7 --- tests/crashes/121623.rs | 8 ---- .../continue-pointing-to-block-ice-113379.rs | 12 ++++++ ...ntinue-pointing-to-block-ice-113379.stderr | 43 +++++++++++++++++++ .../continue-pointing-to-block-ice-121623.rs | 11 +++++ ...ntinue-pointing-to-block-ice-121623.stderr | 14 ++++++ 9 files changed, 113 insertions(+), 29 deletions(-) delete mode 100644 tests/crashes/113379.rs delete mode 100644 tests/crashes/121623.rs create mode 100644 tests/ui/label/continue-pointing-to-block-ice-113379.rs create mode 100644 tests/ui/label/continue-pointing-to-block-ice-113379.stderr create mode 100644 tests/ui/label/continue-pointing-to-block-ice-121623.rs create mode 100644 tests/ui/label/continue-pointing-to-block-ice-121623.stderr diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 2c28ffd1fe3df..a1a33885b944c 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -532,14 +532,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ExprKind::Break(destination, ref expr_opt) => { self.check_expr_break(destination, expr_opt.as_deref(), expr) } - ExprKind::Continue(destination) => { - if destination.target_id.is_ok() { - tcx.types.never - } else { - // There was an error; make type-check fail. - Ty::new_misc_error(tcx) - } - } + ExprKind::Continue(destination) => self.check_expr_continue(destination, expr), ExprKind::Ret(ref expr_opt) => self.check_expr_return(expr_opt.as_deref(), expr), ExprKind::Become(call) => self.check_expr_become(call, expr), ExprKind::Let(let_expr) => self.check_expr_let(let_expr, expr.hir_id), @@ -989,6 +982,31 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + fn check_expr_continue( + &self, + destination: hir::Destination, + expr: &'tcx hir::Expr<'tcx>, + ) -> Ty<'tcx> { + if let Ok(target_id) = destination.target_id { + if let hir::Node::Expr(hir::Expr { kind: ExprKind::Loop(..), .. }) = + self.tcx.hir_node(target_id) + { + self.tcx.types.never + } else { + // Liveness linting assumes `continue`s all point to loops. We'll report an error + // in `check_mod_loops`, but make sure we don't run liveness (#113379, #121623). + let guar = self.dcx().span_delayed_bug( + expr.span, + "found `continue` not pointing to loop, but no error reported", + ); + Ty::new_error(self.tcx, guar) + } + } else { + // There was an error; make type-check fail. + Ty::new_misc_error(self.tcx) + } + } + fn check_expr_return( &self, expr_opt: Option<&'tcx hir::Expr<'tcx>>, diff --git a/compiler/rustc_mir_build/src/builder/mod.rs b/compiler/rustc_mir_build/src/builder/mod.rs index 9cf051a8760be..8400709740f05 100644 --- a/compiler/rustc_mir_build/src/builder/mod.rs +++ b/compiler/rustc_mir_build/src/builder/mod.rs @@ -66,8 +66,7 @@ pub fn build_mir<'tcx>(tcx: TyCtxt<'tcx>, def: LocalDefId) -> Body<'tcx> { } }; - // this must run before MIR dump, because - // "not all control paths return a value" is reported here. + // Checking liveness after building the THIR ensures there were no typeck errors. // // maybe move the check to a MIR pass? tcx.ensure_ok().check_liveness(def); diff --git a/compiler/rustc_passes/src/liveness.rs b/compiler/rustc_passes/src/liveness.rs index 4e9b7fd44d4b8..763d9fda80494 100644 --- a/compiler/rustc_passes/src/liveness.rs +++ b/compiler/rustc_passes/src/liveness.rs @@ -122,7 +122,6 @@ enum LiveNodeKind { VarDefNode(Span, HirId), ClosureNode, ExitNode, - ErrNode, } fn live_node_kind_to_string(lnk: LiveNodeKind, tcx: TyCtxt<'_>) -> String { @@ -133,7 +132,6 @@ fn live_node_kind_to_string(lnk: LiveNodeKind, tcx: TyCtxt<'_>) -> String { VarDefNode(s, _) => format!("Var def node [{}]", sm.span_to_diagnostic_string(s)), ClosureNode => "Closure node".to_owned(), ExitNode => "Exit node".to_owned(), - ErrNode => "Error node".to_owned(), } } @@ -492,6 +490,9 @@ struct Liveness<'a, 'tcx> { impl<'a, 'tcx> Liveness<'a, 'tcx> { fn new(ir: &'a mut IrMaps<'tcx>, body_owner: LocalDefId) -> Liveness<'a, 'tcx> { let typeck_results = ir.tcx.typeck(body_owner); + // Liveness linting runs after building the THIR. We make several assumptions based on + // typeck succeeding, e.g. that breaks and continues are well-formed. + assert!(typeck_results.tainted_by_errors.is_none()); // FIXME(#132279): we're in a body here. let typing_env = ty::TypingEnv::non_body_analysis(ir.tcx, body_owner); let closure_min_captures = typeck_results.closure_min_captures.get(&body_owner); @@ -976,8 +977,9 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { // Now that we know the label we're going to, // look it up in the continue loop nodes table self.cont_ln.get(&sc).cloned().unwrap_or_else(|| { - self.ir.tcx.dcx().span_delayed_bug(expr.span, "continue to unknown label"); - self.ir.add_live_node(ErrNode) + // Liveness linting happens after building the THIR. Bad labels should already + // have been caught. + span_bug!(expr.span, "continue to unknown label"); }) } diff --git a/tests/crashes/113379.rs b/tests/crashes/113379.rs deleted file mode 100644 index 7163cbc39342d..0000000000000 --- a/tests/crashes/113379.rs +++ /dev/null @@ -1,7 +0,0 @@ -//@ known-bug: #113379 - -async fn f999() -> Vec { - 'b: { - continue 'b; - } -} diff --git a/tests/crashes/121623.rs b/tests/crashes/121623.rs deleted file mode 100644 index 3c01a7f452ca7..0000000000000 --- a/tests/crashes/121623.rs +++ /dev/null @@ -1,8 +0,0 @@ -//@ known-bug: #121623 -fn main() { - match () { - _ => 'b: { - continue 'b; - } - } -} diff --git a/tests/ui/label/continue-pointing-to-block-ice-113379.rs b/tests/ui/label/continue-pointing-to-block-ice-113379.rs new file mode 100644 index 0000000000000..8a6a9cc840986 --- /dev/null +++ b/tests/ui/label/continue-pointing-to-block-ice-113379.rs @@ -0,0 +1,12 @@ +//! Regression test for ICE #113379. Liveness linting assumes that `continue`s all point to loops. +//! This tests that if a `continue` points to a block, we don't run liveness lints. + +async fn f999() -> Vec { + //~^ ERROR `async fn` is not permitted in Rust 2015 + 'b: { + //~^ ERROR mismatched types + continue 'b; + //~^ ERROR `continue` pointing to a labeled block + } +} +//~^ ERROR `main` function not found diff --git a/tests/ui/label/continue-pointing-to-block-ice-113379.stderr b/tests/ui/label/continue-pointing-to-block-ice-113379.stderr new file mode 100644 index 0000000000000..ada6305ec999b --- /dev/null +++ b/tests/ui/label/continue-pointing-to-block-ice-113379.stderr @@ -0,0 +1,43 @@ +error[E0670]: `async fn` is not permitted in Rust 2015 + --> $DIR/continue-pointing-to-block-ice-113379.rs:4:1 + | +LL | async fn f999() -> Vec { + | ^^^^^ to use `async fn`, switch to Rust 2018 or later + | + = help: pass `--edition 2024` to `rustc` + = note: for more on editions, read https://doc.rust-lang.org/edition-guide + +error[E0601]: `main` function not found in crate `continue_pointing_to_block_ice_113379` + --> $DIR/continue-pointing-to-block-ice-113379.rs:11:2 + | +LL | } + | ^ consider adding a `main` function to `$DIR/continue-pointing-to-block-ice-113379.rs` + +error[E0696]: `continue` pointing to a labeled block + --> $DIR/continue-pointing-to-block-ice-113379.rs:8:9 + | +LL | / 'b: { +LL | | +LL | | continue 'b; + | | ^^^^^^^^^^^ labeled blocks cannot be `continue`'d +LL | | +LL | | } + | |_____- labeled block the `continue` points to + +error[E0308]: mismatched types + --> $DIR/continue-pointing-to-block-ice-113379.rs:6:5 + | +LL | / 'b: { +LL | | +LL | | continue 'b; +LL | | +LL | | } + | |_____^ expected `Vec`, found `()` + | + = note: expected struct `Vec` + found unit type `()` + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0308, E0601, E0670, E0696. +For more information about an error, try `rustc --explain E0308`. diff --git a/tests/ui/label/continue-pointing-to-block-ice-121623.rs b/tests/ui/label/continue-pointing-to-block-ice-121623.rs new file mode 100644 index 0000000000000..7047f7a309aaf --- /dev/null +++ b/tests/ui/label/continue-pointing-to-block-ice-121623.rs @@ -0,0 +1,11 @@ +//! Regression test for ICE #121623. Liveness linting assumes that `continue`s all point to loops. +//! This tests that if a `continue` points to a block, we don't run liveness lints. + +fn main() { + match () { + _ => 'b: { + continue 'b; + //~^ ERROR `continue` pointing to a labeled block + } + } +} diff --git a/tests/ui/label/continue-pointing-to-block-ice-121623.stderr b/tests/ui/label/continue-pointing-to-block-ice-121623.stderr new file mode 100644 index 0000000000000..a484fb629d181 --- /dev/null +++ b/tests/ui/label/continue-pointing-to-block-ice-121623.stderr @@ -0,0 +1,14 @@ +error[E0696]: `continue` pointing to a labeled block + --> $DIR/continue-pointing-to-block-ice-121623.rs:7:13 + | +LL | _ => 'b: { + | ______________- +LL | | continue 'b; + | | ^^^^^^^^^^^ labeled blocks cannot be `continue`'d +LL | | +LL | | } + | |_________- labeled block the `continue` points to + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0696`.