Loop match proper errors

compiler/rustc_mir_build/messages.ftl

@@ -478,3 +478,23 @@ mir_build_unused_unsafe_enclosing_block_label = because it's nested under this `
 mir_build_variant_defined_here = not covered
 
 mir_build_wrap_suggestion = consider wrapping the function body in an unsafe block
+
+mir_build_loop_match_invalid_update =
+    invalid update of the `loop_match` state
+    .label = the assignment must update this variable
+
+mir_build_loop_match_invalid_match =
+    invalid match on `loop_match` state
+    .note = only matches on local variables are valid
+
+mir_build_loop_match_bad_statements =
+    statements are not allowed in this position within a `loop_match`
+
+mir_build_loop_match_bad_rhs =
+    this expression must be a single `match` wrapped in a labelled block
+
+mir_build_loop_match_missing_assignment =
+    expected a single assignment expression
+
+mir_build_const_continue_missing_value =
+    a `const_continue` must break to a label with a value

compiler/rustc_mir_build/src/builder/expr/into.rs

@@ -11,8 +11,7 @@ use rustc_middle::thir::*;
 use rustc_middle::ty::CanonicalUserTypeAnnotation;
 use rustc_middle::ty::util::Discr;
 use rustc_pattern_analysis::constructor::Constructor;
-use rustc_pattern_analysis::rustc::DeconstructedPat;
-use rustc_pattern_analysis::rustc::RustcPatCtxt;
+use rustc_pattern_analysis::rustc::{DeconstructedPat, RustcPatCtxt};
 use rustc_span::source_map::Spanned;
 use tracing::{debug, instrument};
 
@@ -300,7 +299,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     let discr_ty = match state_ty {
                         ty if ty.is_enum() => ty.discriminant_ty(this.tcx),
                         ty if ty.is_integral() => ty,
-                        _ => todo!(),
+                        other => todo!("{other:?}"),
                     };
 
                     let rvalue = match state_ty {

compiler/rustc_mir_build/src/errors.rs

@@ -1161,3 +1161,48 @@ impl Subdiagnostic for Rust2024IncompatiblePatSugg {
         }
     }
 }
+
+#[derive(Diagnostic)]
+#[diag(mir_build_loop_match_invalid_update)]
+pub(crate) struct LoopMatchInvalidUpdate {
+    #[primary_span]
+    pub lhs: Span,
+    #[label]
+    pub scrutinee: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(mir_build_loop_match_invalid_match)]
+#[note]
+pub(crate) struct LoopMatchInvalidMatch {
+    #[primary_span]
+    pub span: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(mir_build_loop_match_bad_statements)]
+pub(crate) struct LoopMatchBadStatements {
+    #[primary_span]
+    pub span: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(mir_build_loop_match_bad_rhs)]
+pub(crate) struct LoopMatchBadRhs {
+    #[primary_span]
+    pub span: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(mir_build_loop_match_missing_assignment)]
+pub(crate) struct LoopMatchMissingAssignment {
+    #[primary_span]
+    pub span: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(mir_build_const_continue_missing_value)]
+pub(crate) struct ConstContinueMissingValue {
+    #[primary_span]
+    pub span: Span,
+}

compiler/rustc_mir_build/src/thir/cx/expr.rs

@@ -21,6 +21,7 @@ use rustc_middle::{bug, span_bug};
 use rustc_span::{Span, sym};
 use tracing::{debug, info, instrument, trace};
 
+use crate::errors::*;
 use crate::thir::cx::ThirBuildCx;
 
 impl<'tcx> ThirBuildCx<'tcx> {
@@ -798,15 +799,20 @@ impl<'tcx> ThirBuildCx<'tcx> {
                     .any(|attr| attr.has_name(sym::const_continue));
                 if is_const_continue {
                     match dest.target_id {
-                        Ok(target_id) => ExprKind::ConstContinue {
-                            label: region::Scope {
-                                local_id: target_id.local_id,
-                                data: region::ScopeData::Node,
-                            },
-                            value: self.mirror_expr(
-                                value.expect("missing value for #[const_continue] break"),
-                            ),
-                        },
+                        Ok(target_id) => {
+                            let Some(value) = value else {
+                                let span = expr.span;
+                                self.tcx.dcx().emit_fatal(ConstContinueMissingValue { span })
+                            };
+
+                            ExprKind::ConstContinue {
+                                label: region::Scope {
+                                    local_id: target_id.local_id,
+                                    data: region::ScopeData::Node,
+                                },
+                                value: self.mirror_expr(value),
+                            }
+                        }
                         Err(err) => bug!("invalid loop id for break: {}", err),
                     }
                 } else {
@@ -863,27 +869,57 @@ impl<'tcx> ThirBuildCx<'tcx> {
                     .iter()
                     .any(|attr| attr.has_name(sym::loop_match));
                 if is_loop_match {
-                    assert!(body.stmts.is_empty());
-                    let hir::ExprKind::Assign(state, block_expr, _) = body.expr.unwrap().kind
-                    else {
-                        panic!();
+                    let dcx = self.tcx.dcx();
+
+                    if let ([first, ..], [.., last]) = (body.stmts, body.stmts) {
+                        dcx.emit_fatal(LoopMatchBadStatements { span: first.span.to(last.span) })
+                    }
+
+                    let Some(loop_body_expr) = body.expr else {
+                        dcx.emit_fatal(LoopMatchMissingAssignment { span: body.span })
                     };
-                    let hir::ExprKind::Block(block_body, _) = block_expr.kind else {
-                        panic!();
+
+                    let hir::ExprKind::Assign(state, rhs_expr, _) = loop_body_expr.kind else {
+                        dcx.emit_fatal(LoopMatchMissingAssignment { span: loop_body_expr.span })
                     };
-                    assert!(block_body.stmts.is_empty());
-                    let hir::ExprKind::Match(discr, arms, match_source) =
-                        block_body.expr.unwrap().kind
+
+                    let hir::ExprKind::Block(block_body, _) = rhs_expr.kind else {
+                        dcx.emit_fatal(LoopMatchBadRhs { span: rhs_expr.span })
+                    };
+
+                    if let Some(first) = block_body.stmts.first() {
+                        let span = first.span.to(block_body.stmts.last().unwrap().span);
+                        dcx.emit_fatal(LoopMatchBadStatements { span })
+                    }
+
+                    let Some(block_body_expr) = block_body.expr else {
+                        dcx.emit_fatal(LoopMatchBadRhs { span: block_body.span })
+                    };
+
+                    let hir::ExprKind::Match(scrutinee, arms, match_source) = block_body_expr.kind
                     else {
-                        panic!();
+                        dcx.emit_fatal(LoopMatchBadRhs { span: block_body_expr.span })
+                    };
+
+                    fn local(expr: &rustc_hir::Expr<'_>) -> Option<hir::HirId> {
+                        if let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) = expr.kind {
+                            if let Res::Local(hir_id) = path.res {
+                                return Some(hir_id);
+                            }
+                        }
+
+                        None
+                    }
+
+                    let Some(scrutinee_hir_id) = local(scrutinee) else {
+                        dcx.emit_fatal(LoopMatchInvalidMatch { span: scrutinee.span })
                     };
-                    match (state.kind, discr.kind) {
-                        (
-                            hir::ExprKind::Path(hir::QPath::Resolved(_, state)),
-                            hir::ExprKind::Path(hir::QPath::Resolved(_, discr)),
-                        ) if state.segments.iter().map(|seg| seg.ident).collect::<Vec<_>>()
-                            == discr.segments.iter().map(|seg| seg.ident).collect::<Vec<_>>() => {}
-                        _ => panic!(),
+
+                    if local(state) != Some(scrutinee_hir_id) {
+                        dcx.emit_fatal(LoopMatchInvalidUpdate {
+                            scrutinee: scrutinee.span,
+                            lhs: state.span,
+                        })
                     }
 
                     ExprKind::LoopMatch {

loop_match_todo.md

@@ -1,9 +1,9 @@
 # TODO
 
-* [ ] errors instead of ICE on incorrect usage
+* [x] errors instead of ICE on incorrect usage
 * [ ] deny drop for `#[const_continue]`
-* [ ] integer patterns
-* [ ] `_` and `Foo | Bar` patterns
+* [x] integer patterns
+* [x] `_` and `Foo | Bar` patterns
 * [ ] handle in the `let mut` checker (likely needs handling drop trees for StorageDead)
 * [ ] `lint_level`?
 * [ ] test if nested `#[loop_match]` with `#[const_continue]` operating on outer loop works

tests/ui/loop-match/invalid.rs

@@ -0,0 +1,142 @@
+#![feature(loop_match)]
+#![crate_type = "lib"]
+
+enum State {
+    A,
+    B,
+    C,
+}
+
+fn invalid_update() {
+    let mut fake = State::A;
+    let state = State::A;
+    #[loop_match]
+    loop {
+        fake = 'blk: {
+            //~^ ERROR: invalid update of the `loop_match` state
+            match state {
+                _ => State::B,
+            }
+        }
+    }
+}
+
+fn invalid_scrutinee() {
+    let state = State::A;
+    #[loop_match]
+    loop {
+        state = 'blk: {
+            match State::A {
+                //~^ ERROR: invalid match on `loop_match` state
+                _ => State::B,
+            }
+        }
+    }
+}
+
+fn bad_statements_1() {
+    let state = State::A;
+    #[loop_match]
+    loop {
+        1;
+        //~^ ERROR: statements are not allowed in this position within a `loop_match`
+        state = 'blk: {
+            match State::A {
+                _ => State::B,
+            }
+        }
+    }
+}
+
+fn bad_statements_2() {
+    let state = State::A;
+    #[loop_match]
+    loop {
+        state = 'blk: {
+            1;
+            //~^ ERROR: statements are not allowed in this position within a `loop_match`
+            match State::A {
+                _ => State::B,
+            }
+        }
+    }
+}
+
+fn bad_rhs_1() {
+    let state = State::A;
+    #[loop_match]
+    loop {
+        state = State::B
+        //~^ ERROR: this expression must be a single `match` wrapped in a labelled block
+    }
+}
+
+fn bad_rhs_2() {
+    let state = State::A;
+    #[loop_match]
+    loop {
+        state = 'blk: {
+            State::B
+            //~^ ERROR: this expression must be a single `match` wrapped in a labelled block
+        }
+    }
+}
+
+fn bad_rhs_3() {
+    let state = ();
+    #[loop_match]
+    loop {
+        state = 'blk: {
+            //~^ ERROR: this expression must be a single `match` wrapped in a labelled block
+        }
+    }
+}
+
+fn missing_assignment() {
+    let state = State::A;
+    #[loop_match]
+    loop {
+        () //~ ERROR:  expected a single assignment expression
+    }
+}
+
+fn empty_loop_body() {
+    let state = State::A;
+    #[loop_match]
+    loop {
+        //~^ ERROR:  expected a single assignment expression
+    }
+}
+
+fn break_without_value() {
+    let state = State::A;
+    #[loop_match]
+    'a: loop {
+        state = 'blk: {
+            match state {
+                State::A => {
+                    #[const_continue]
+                    break 'blk;
+                    //~^ ERROR: mismatched types
+                }
+                _ => break 'a,
+            }
+        }
+    }
+}
+
+fn break_without_value_unit() {
+    let state = ();
+    #[loop_match]
+    'a: loop {
+        state = 'blk: {
+            match state {
+                () => {
+                    #[const_continue]
+                    break 'blk;
+                    //~^ ERROR: a `const_continue` must break to a label with a value
+                }
+            }
+        }
+    }
+}