Skip to content

Commit

Permalink
Rollup merge of #129394 - Jarcho:irrefutable_let_patterns, r=Nadrieril
Browse files Browse the repository at this point in the history
Don't lint `irrefutable_let_patterns` on leading patterns if `else if` let-chains

fixes #128661

Is there any preference where the test goes? There looks to be several places it could fit.
  • Loading branch information
matthiaskrgr authored Oct 30, 2024
2 parents 16422db + 4a2e08a commit 87d348b
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 8 deletions.
25 changes: 18 additions & 7 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ enum LetSource {
IfLetGuard,
LetElse,
WhileLet,
Else,
ElseIfLet,
}

struct MatchVisitor<'p, 'tcx> {
Expand Down Expand Up @@ -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 } => {
Expand Down Expand Up @@ -573,9 +580,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;
Expand Down Expand Up @@ -906,8 +917,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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

20 changes: 20 additions & 0 deletions tests/ui/rfcs/rfc-2497-if-let-chains/irrefutable-lets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{}
}
}

0 comments on commit 87d348b

Please sign in to comment.