diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl index fae159103e70d..b6f9ed1225f88 100644 --- a/compiler/rustc_mir_build/messages.ftl +++ b/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 diff --git a/compiler/rustc_mir_build/src/builder/expr/into.rs b/compiler/rustc_mir_build/src/builder/expr/into.rs index 8873dc8391243..be2f8b02a29d4 100644 --- a/compiler/rustc_mir_build/src/builder/expr/into.rs +++ b/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 { diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index 17b22f25dbb0a..4e53cdfcb63b3 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/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, +} diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index b3413d235b2de..b9266a61a49bc 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/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 { + 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::>() - == discr.segments.iter().map(|seg| seg.ident).collect::>() => {} - _ => panic!(), + + if local(state) != Some(scrutinee_hir_id) { + dcx.emit_fatal(LoopMatchInvalidUpdate { + scrutinee: scrutinee.span, + lhs: state.span, + }) } ExprKind::LoopMatch { diff --git a/loop_match_todo.md b/loop_match_todo.md index 0042fbe1302df..ee29c3356b535 100644 --- a/loop_match_todo.md +++ b/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 diff --git a/tests/ui/match/loop-match-integer.rs b/tests/ui/loop-match/integer-patterns.rs similarity index 100% rename from tests/ui/match/loop-match-integer.rs rename to tests/ui/loop-match/integer-patterns.rs diff --git a/tests/ui/loop-match/invalid.rs b/tests/ui/loop-match/invalid.rs new file mode 100644 index 0000000000000..fb8862030382d --- /dev/null +++ b/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 + } + } + } + } +} diff --git a/tests/ui/loop-match/invalid.stderr b/tests/ui/loop-match/invalid.stderr new file mode 100644 index 0000000000000..70b23ff2b81d5 --- /dev/null +++ b/tests/ui/loop-match/invalid.stderr @@ -0,0 +1,85 @@ +error[E0308]: mismatched types + --> $DIR/invalid.rs:119:21 + | +LL | break 'blk; + | ^^^^^^^^^^ expected `State`, found `()` + | +help: give the `break` a value of the expected type + | +LL | break 'blk /* value */; + | +++++++++++ + +error: invalid update of the `loop_match` state + --> $DIR/invalid.rs:15:9 + | +LL | fake = 'blk: { + | ^^^^ +LL | +LL | match state { + | ----- the assignment must update this variable + +error: invalid match on `loop_match` state + --> $DIR/invalid.rs:29:19 + | +LL | match State::A { + | ^^^^^^^^ + | + = note: only matches on local variables are valid + +error: statements are not allowed in this position within a `loop_match` + --> $DIR/invalid.rs:41:9 + | +LL | 1; + | ^^ + +error: statements are not allowed in this position within a `loop_match` + --> $DIR/invalid.rs:56:13 + | +LL | 1; + | ^^ + +error: this expression must be a single `match` wrapped in a labelled block + --> $DIR/invalid.rs:69:17 + | +LL | state = State::B + | ^^^^^^^^ + +error: this expression must be a single `match` wrapped in a labelled block + --> $DIR/invalid.rs:79:13 + | +LL | State::B + | ^^^^^^^^ + +error: this expression must be a single `match` wrapped in a labelled block + --> $DIR/invalid.rs:89:17 + | +LL | state = 'blk: { + | _________________^ +LL | | +LL | | } + | |_________^ + +error: expected a single assignment expression + --> $DIR/invalid.rs:99:9 + | +LL | () + | ^^ + +error: expected a single assignment expression + --> $DIR/invalid.rs:106:10 + | +LL | loop { + | __________^ +LL | | +LL | | } + | |_____^ + +error: a `const_continue` must break to a label with a value + --> $DIR/invalid.rs:136:21 + | +LL | break 'blk; + | ^^^^^^^^^^ + +error: aborting due to 11 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/match/loop-match.rs b/tests/ui/loop-match/loop-match.rs similarity index 100% rename from tests/ui/match/loop-match.rs rename to tests/ui/loop-match/loop-match.rs diff --git a/tests/ui/match/loop-match-or-pattern.rs b/tests/ui/loop-match/or-patterns.rs similarity index 100% rename from tests/ui/match/loop-match-or-pattern.rs rename to tests/ui/loop-match/or-patterns.rs