From 4a2e08af22ee97e97b21a1cd75d80a1a2f746a3e Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Wed, 21 Aug 2024 23:29:51 -0400 Subject: [PATCH] Don't lint `irrefutable_let_patterns` on leading patterns if `else if` let-chains. --- .../src/thir/pattern/check_match.rs | 25 +++++++++++++------ .../irrefutable-lets.disallowed.stderr | 20 ++++++++++++++- .../irrefutable-lets.rs | 20 +++++++++++++++ 3 files changed, 57 insertions(+), 8 deletions(-) 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 bc1acd51c6911..a8ca19b788785 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -79,6 +79,8 @@ enum LetSource { IfLetGuard, LetElse, WhileLet, + Else, + ElseIfLet, } struct MatchVisitor<'p, 'tcx> { @@ -129,15 +131,20 @@ impl<'p, 'tcx> Visitor<'p, 'tcx> for MatchVisitor<'p, 'tcx> { // Give a specific `let_source` for the condition. let let_source = match ex.span.desugaring_kind() { Some(DesugaringKind::WhileLoop) => LetSource::WhileLet, - _ => LetSource::IfLet, + _ => match self.let_source { + LetSource::Else => LetSource::ElseIfLet, + _ => LetSource::IfLet, + }, }; self.with_let_source(let_source, |this| this.visit_expr(&self.thir[cond])); self.with_let_source(LetSource::None, |this| { this.visit_expr(&this.thir[then]); - if let Some(else_) = else_opt { - this.visit_expr(&this.thir[else_]); - } }); + if let Some(else_) = else_opt { + self.with_let_source(LetSource::Else, |this| { + this.visit_expr(&this.thir[else_]) + }); + } return; } ExprKind::Match { scrutinee, scrutinee_hir_id: _, box ref arms, match_source } => { @@ -569,9 +576,13 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> { // and we shouldn't lint. // For let guards inside a match, prefixes might use bindings of the match pattern, // so can't always be moved out. + // For `else if let`, an extra indentation level would be required to move the bindings. // FIXME: Add checking whether the bindings are actually used in the prefix, // and lint if they are not. - if !matches!(self.let_source, LetSource::WhileLet | LetSource::IfLetGuard) { + if !matches!( + self.let_source, + LetSource::WhileLet | LetSource::IfLetGuard | LetSource::ElseIfLet + ) { // Emit the lint let prefix = &chain_refutabilities[..until]; let span_start = prefix[0].unwrap().0; @@ -902,8 +913,8 @@ fn report_irrefutable_let_patterns( } match source { - LetSource::None | LetSource::PlainLet => bug!(), - LetSource::IfLet => emit_diag!(IrrefutableLetPatternsIfLet), + LetSource::None | LetSource::PlainLet | LetSource::Else => bug!(), + LetSource::IfLet | LetSource::ElseIfLet => emit_diag!(IrrefutableLetPatternsIfLet), LetSource::IfLetGuard => emit_diag!(IrrefutableLetPatternsIfLetGuard), LetSource::LetElse => emit_diag!(IrrefutableLetPatternsLetElse), LetSource::WhileLet => emit_diag!(IrrefutableLetPatternsWhileLet), diff --git a/tests/ui/rfcs/rfc-2497-if-let-chains/irrefutable-lets.disallowed.stderr b/tests/ui/rfcs/rfc-2497-if-let-chains/irrefutable-lets.disallowed.stderr index be4a523155825..130d0296c5ece 100644 --- a/tests/ui/rfcs/rfc-2497-if-let-chains/irrefutable-lets.disallowed.stderr +++ b/tests/ui/rfcs/rfc-2497-if-let-chains/irrefutable-lets.disallowed.stderr @@ -111,5 +111,23 @@ LL | while let Some(ref first) = opt && let second = first && let _third = s = note: these patterns will always match = help: consider moving them into the body -error: aborting due to 12 previous errors +error: trailing irrefutable pattern in let chain + --> $DIR/irrefutable-lets.rs:87:12 + | +LL | && let x = &opt + | ^^^^^^^^^^^^ + | + = note: this pattern will always match + = help: consider moving it into the body + +error: leading irrefutable pattern in let chain + --> $DIR/irrefutable-lets.rs:93:12 + | +LL | if let x = opt.clone().map(|_| 1) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this pattern will always match + = help: consider moving it outside of the construct + +error: aborting due to 14 previous errors diff --git a/tests/ui/rfcs/rfc-2497-if-let-chains/irrefutable-lets.rs b/tests/ui/rfcs/rfc-2497-if-let-chains/irrefutable-lets.rs index bd4df337614a0..e7d69f89773ee 100644 --- a/tests/ui/rfcs/rfc-2497-if-let-chains/irrefutable-lets.rs +++ b/tests/ui/rfcs/rfc-2497-if-let-chains/irrefutable-lets.rs @@ -75,4 +75,24 @@ fn main() { && let Range { start: local_start, end: _ } = first && let None = local_start { } + + // No error. An extra nesting level would be required for the `else if`. + if opt == Some(None..None) { + } else if let x = opt.clone().map(|_| 1) + && x == Some(1) + {} + + if opt == Some(None..None) { + } else if opt.is_some() + && let x = &opt + //[disallowed]~^ ERROR trailing irrefutable pattern in let chain + {} + + if opt == Some(None..None) { + } else { + if let x = opt.clone().map(|_| 1) + //[disallowed]~^ ERROR leading irrefutable pattern in let chain + && x == Some(1) + {} + } }