diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs index ae67bb5075e8f..faae5b7d609d9 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -6,7 +6,7 @@ use rustc_errors::codes::*; use rustc_errors::{Applicability, ErrorGuaranteed, MultiSpan, struct_span_code_err}; use rustc_hir::def::*; use rustc_hir::def_id::LocalDefId; -use rustc_hir::{self as hir, BindingMode, ByRef, HirId, MatchSource}; +use rustc_hir::{self as hir, BindingMode, ByRef, HirId, LocalSource, MatchSource, Node}; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::Level; use rustc_middle::bug; @@ -62,7 +62,7 @@ pub(crate) fn check_match(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Result<(), Err for param in thir.params.iter() { if let Some(box ref pattern) = param.pat { - visitor.check_binding_is_irrefutable(pattern, origin, None, None); + visitor.check_binding_is_irrefutable(pattern, origin, None, None, None); } } visitor.error @@ -161,7 +161,7 @@ impl<'p, 'tcx> Visitor<'p, 'tcx> for MatchVisitor<'p, 'tcx> { self.check_match(scrutinee, arms, MatchSource::Normal, span); } ExprKind::Let { box ref pat, expr } => { - self.check_let(pat, Some(expr), ex.span); + self.check_let(pat, Some(expr), ex.span, None); } ExprKind::LogicalOp { op: LogicalOp::And, .. } if !matches!(self.let_source, LetSource::None) => @@ -188,7 +188,12 @@ impl<'p, 'tcx> Visitor<'p, 'tcx> for MatchVisitor<'p, 'tcx> { let let_source = if else_block.is_some() { LetSource::LetElse } else { LetSource::PlainLet }; this.with_let_source(let_source, |this| { - this.check_let(pattern, initializer, span) + // FIXME(#145548): We get hir_id from the lint_level, replace it if thir has a better way to get it. + let hir_id = match lint_level { + LintLevel::Explicit(hir_id) => Some(hir_id), + _ => None, + }; + this.check_let(pattern, initializer, span, hir_id) }); visit::walk_stmt(this, stmt); }); @@ -439,11 +444,17 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { } #[instrument(level = "trace", skip(self))] - fn check_let(&mut self, pat: &'p Pat<'tcx>, scrutinee: Option, span: Span) { + fn check_let( + &mut self, + pat: &'p Pat<'tcx>, + scrutinee: Option, + span: Span, + hir_id: Option, + ) { assert!(self.let_source != LetSource::None); let scrut = scrutinee.map(|id| &self.thir[id]); if let LetSource::PlainLet = self.let_source { - self.check_binding_is_irrefutable(pat, "local binding", scrut, Some(span)) + self.check_binding_is_irrefutable(pat, "local binding", scrut, Some(span), hir_id) } else { let Ok(refutability) = self.is_let_irrefutable(pat, scrut) else { return }; if matches!(refutability, Irrefutable) { @@ -519,6 +530,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { "`for` loop binding", None, None, + None, ); } else { // span after scrutinee, or after `.match`. That is, the braces, arms, @@ -665,6 +677,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { origin: &str, scrut: Option<&Expr<'tcx>>, sp: Option, + hir_id: Option, ) { let pattern_ty = pat.ty; @@ -715,6 +728,8 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { && self.tcx.sess.source_map().is_span_accessible(span) && interpreted_as_const.is_none() && scrut.is_some() + // we only suggest `let else` when the pattern is not desugared from an assignment + && !self.is_from_destructing_assignment(hir_id) { let mut bindings = vec![]; pat.each_binding(|name, _, _, _| bindings.push(name)); @@ -768,6 +783,19 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { adt_defined_here, })); } + + /// Check if hir_id of LetStmt is desugared into a `let`, i.e., `LocalSource::AssignDesugar` + /// rather than a real `let` the user wrote. This helps suppress suggestions for `let` statements + /// when we're dealing with an assignment like `Some(x) = rhs`. See #145548. + fn is_from_destructing_assignment(&self, hir_id: Option) -> bool { + if let Some(hir_id) = hir_id { + // pat_hir_id is the hir_id of the pattern in the `let` statement/expr. + if let Node::LetStmt(let_stmt) = self.tcx.hir_node(hir_id) { + return matches!(let_stmt.source, LocalSource::AssignDesugar(_)); + } + } + false + } } /// Check if a by-value binding is by-value. That is, check if the binding's type is not `Copy`. diff --git a/tests/ui/destructuring-assignment/non-exhaustive-destructure.stderr b/tests/ui/destructuring-assignment/non-exhaustive-destructure.stderr index 88f7d2da47c28..f614550f83654 100644 --- a/tests/ui/destructuring-assignment/non-exhaustive-destructure.stderr +++ b/tests/ui/destructuring-assignment/non-exhaustive-destructure.stderr @@ -7,10 +7,6 @@ LL | None = Some(3); = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant = note: for more information, visit https://doc.rust-lang.org/book/ch19-02-refutability.html = note: the matched value is of type `Option` -help: you might want to use `if let` to ignore the variant that isn't matched - | -LL | if None = Some(3) { todo!() }; - | ++ +++++++++++ error: aborting due to 1 previous error diff --git a/tests/ui/let-else/suggest-with-let-issue-145548.rs b/tests/ui/let-else/suggest-with-let-issue-145548.rs new file mode 100644 index 0000000000000..1d7625d70e634 --- /dev/null +++ b/tests/ui/let-else/suggest-with-let-issue-145548.rs @@ -0,0 +1,18 @@ +// `Some(x) = Some(1)` is treated as a let statement/expr, +// so we should not suggest to add `let ... else{...}` + +fn foo1(){ + let x = 2; + Some(x) = Some(1); //~ ERROR refutable pattern in local binding [E0005] +} + +fn foo2(){ + let x = 2; + let Some(x) = Some(1); //~ ERROR refutable pattern in local binding [E0005] +} + +fn foo3(){ + Some(()) = Some(()); //~ ERROR refutable pattern in local binding [E0005] +} + +fn main() {} diff --git a/tests/ui/let-else/suggest-with-let-issue-145548.stderr b/tests/ui/let-else/suggest-with-let-issue-145548.stderr new file mode 100644 index 0000000000000..701bc4e4056bf --- /dev/null +++ b/tests/ui/let-else/suggest-with-let-issue-145548.stderr @@ -0,0 +1,37 @@ +error[E0005]: refutable pattern in local binding + --> $DIR/suggest-with-let-issue-145548.rs:6:5 + | +LL | Some(x) = Some(1); + | ^^^^^^^ pattern `None` not covered + | + = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant + = note: for more information, visit https://doc.rust-lang.org/book/ch19-02-refutability.html + = note: the matched value is of type `Option` + +error[E0005]: refutable pattern in local binding + --> $DIR/suggest-with-let-issue-145548.rs:11:9 + | +LL | let Some(x) = Some(1); + | ^^^^^^^ pattern `None` not covered + | + = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant + = note: for more information, visit https://doc.rust-lang.org/book/ch19-02-refutability.html + = note: the matched value is of type `Option` +help: you might want to use `let else` to handle the variant that isn't matched + | +LL | let Some(x) = Some(1) else { todo!() }; + | ++++++++++++++++ + +error[E0005]: refutable pattern in local binding + --> $DIR/suggest-with-let-issue-145548.rs:15:5 + | +LL | Some(()) = Some(()); + | ^^^^^^^^ pattern `None` not covered + | + = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant + = note: for more information, visit https://doc.rust-lang.org/book/ch19-02-refutability.html + = note: the matched value is of type `Option<()>` + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0005`.