Skip to content

Commit 4f31ff8

Browse files
Natural-selection1Natural-selection1
committed
Not linting irrefutable_let_patterns on let chains
inline rest of the check try fix ci errors inline in check_let
1 parent fbd6934 commit 4f31ff8

10 files changed

Lines changed: 196 additions & 307 deletions

File tree

‎compiler/rustc_mir_build/src/errors.rs‎

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -874,52 +874,6 @@ pub(crate) struct UpperRangeBoundCannotBeMin {
874874
pub(crate) span: Span,
875875
}
876876

877-
#[derive(LintDiagnostic)]
878-
#[diag(
879-
"leading irrefutable {$count ->
880-
[one] pattern
881-
*[other] patterns
882-
} in let chain"
883-
)]
884-
#[note(
885-
"{$count ->
886-
[one] this pattern
887-
*[other] these patterns
888-
} will always match"
889-
)]
890-
#[help(
891-
"consider moving {$count ->
892-
[one] it
893-
*[other] them
894-
} outside of the construct"
895-
)]
896-
pub(crate) struct LeadingIrrefutableLetPatterns {
897-
pub(crate) count: usize,
898-
}
899-
900-
#[derive(LintDiagnostic)]
901-
#[diag(
902-
"trailing irrefutable {$count ->
903-
[one] pattern
904-
*[other] patterns
905-
} in let chain"
906-
)]
907-
#[note(
908-
"{$count ->
909-
[one] this pattern
910-
*[other] these patterns
911-
} will always match"
912-
)]
913-
#[help(
914-
"consider moving {$count ->
915-
[one] it
916-
*[other] them
917-
} into the body"
918-
)]
919-
pub(crate) struct TrailingIrrefutableLetPatterns {
920-
pub(crate) count: usize,
921-
}
922-
923877
#[derive(LintDiagnostic)]
924878
#[diag("pattern binding `{$name}` is named the same as one of the variants of the type `{$ty_path}`", code = E0170)]
925879
pub(crate) struct BindingsWithVariantName {

‎compiler/rustc_mir_build/src/thir/pattern/check_match.rs‎

Lines changed: 8 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,9 @@ impl<'p, 'tcx> Visitor<'p, 'tcx> for MatchVisitor<'p, 'tcx> {
167167
{
168168
let mut chain_refutabilities = Vec::new();
169169
let Ok(()) = self.visit_land(ex, &mut chain_refutabilities) else { return };
170-
// If at least one of the operands is a `let ... = ...`.
171-
if chain_refutabilities.iter().any(|x| x.is_some()) {
172-
self.check_let_chain(chain_refutabilities, ex.span);
170+
// Lint only single irrefutable let binding.
171+
if let [Some((_, Irrefutable))] = chain_refutabilities[..] {
172+
self.lint_single_let(ex.span);
173173
}
174174
return;
175175
}
@@ -430,18 +430,9 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
430430
assert!(self.let_source != LetSource::None);
431431
let scrut = scrutinee.map(|id| &self.thir[id]);
432432
if let LetSource::PlainLet = self.let_source {
433-
self.check_binding_is_irrefutable(pat, "local binding", scrut, Some(span))
434-
} else {
435-
let Ok(refutability) = self.is_let_irrefutable(pat, scrut) else { return };
436-
if matches!(refutability, Irrefutable) {
437-
report_irrefutable_let_patterns(
438-
self.tcx,
439-
self.hir_source,
440-
self.let_source,
441-
1,
442-
span,
443-
);
444-
}
433+
self.check_binding_is_irrefutable(pat, "local binding", scrut, Some(span));
434+
} else if let Ok(Irrefutable) = self.is_let_irrefutable(pat, scrut) {
435+
self.lint_single_let(span);
445436
}
446437
}
447438

@@ -549,74 +540,8 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
549540
}
550541

551542
#[instrument(level = "trace", skip(self))]
552-
fn check_let_chain(
553-
&mut self,
554-
chain_refutabilities: Vec<Option<(Span, RefutableFlag)>>,
555-
whole_chain_span: Span,
556-
) {
557-
assert!(self.let_source != LetSource::None);
558-
559-
if chain_refutabilities.iter().all(|r| matches!(*r, Some((_, Irrefutable)))) {
560-
// The entire chain is made up of irrefutable `let` statements
561-
report_irrefutable_let_patterns(
562-
self.tcx,
563-
self.hir_source,
564-
self.let_source,
565-
chain_refutabilities.len(),
566-
whole_chain_span,
567-
);
568-
return;
569-
}
570-
571-
if let Some(until) =
572-
chain_refutabilities.iter().position(|r| !matches!(*r, Some((_, Irrefutable))))
573-
&& until > 0
574-
{
575-
// The chain has a non-zero prefix of irrefutable `let` statements.
576-
577-
// Check if the let source is while, for there is no alternative place to put a prefix,
578-
// and we shouldn't lint.
579-
// For let guards inside a match, prefixes might use bindings of the match pattern,
580-
// so can't always be moved out.
581-
// For `else if let`, an extra indentation level would be required to move the bindings.
582-
// FIXME: Add checking whether the bindings are actually used in the prefix,
583-
// and lint if they are not.
584-
if !matches!(
585-
self.let_source,
586-
LetSource::WhileLet | LetSource::IfLetGuard | LetSource::ElseIfLet
587-
) {
588-
// Emit the lint
589-
let prefix = &chain_refutabilities[..until];
590-
let span_start = prefix[0].unwrap().0;
591-
let span_end = prefix.last().unwrap().unwrap().0;
592-
let span = span_start.to(span_end);
593-
let count = prefix.len();
594-
self.tcx.emit_node_span_lint(
595-
IRREFUTABLE_LET_PATTERNS,
596-
self.hir_source,
597-
span,
598-
LeadingIrrefutableLetPatterns { count },
599-
);
600-
}
601-
}
602-
603-
if let Some(from) =
604-
chain_refutabilities.iter().rposition(|r| !matches!(*r, Some((_, Irrefutable))))
605-
&& from != (chain_refutabilities.len() - 1)
606-
{
607-
// The chain has a non-empty suffix of irrefutable `let` statements
608-
let suffix = &chain_refutabilities[from + 1..];
609-
let span_start = suffix[0].unwrap().0;
610-
let span_end = suffix.last().unwrap().unwrap().0;
611-
let span = span_start.to(span_end);
612-
let count = suffix.len();
613-
self.tcx.emit_node_span_lint(
614-
IRREFUTABLE_LET_PATTERNS,
615-
self.hir_source,
616-
span,
617-
TrailingIrrefutableLetPatterns { count },
618-
);
619-
}
543+
fn lint_single_let(&mut self, let_span: Span) {
544+
report_irrefutable_let_patterns(self.tcx, self.hir_source, self.let_source, 1, let_span);
620545
}
621546

622547
fn analyze_binding(

‎compiler/rustc_mir_transform/src/check_alignment.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ fn insert_alignment_check<'tcx>(
8282

8383
// If this target does not have reliable alignment, further limit the mask by anding it with
8484
// the mask for the highest reliable alignment.
85-
#[allow(irrefutable_let_patterns)]
85+
#[cfg_attr(bootstrap, expect(irrefutable_let_patterns))]
8686
if let max_align = tcx.sess.target.max_reliable_alignment()
8787
&& max_align < Align::MAX
8888
{

‎src/tools/miri/src/shims/foreign_items.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -851,7 +851,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
851851
// Fallback to shims in submodules.
852852
_ => {
853853
// Math shims
854-
#[expect(irrefutable_let_patterns)]
854+
#[cfg_attr(bootstrap, expect(irrefutable_let_patterns))]
855855
if let res = shims::math::EvalContextExt::emulate_foreign_item_inner(
856856
this, link_name, abi, args, dest,
857857
)? && !matches!(res, EmulateItemResult::NotSupported)

‎tests/ui/binding/irrefutable-in-let-chains.rs‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
// https://github.com/rust-lang/rust/issues/139369
2+
// Test that the lint `irrefutable_let_patterns` now
3+
// only checks single let binding.
4+
//@ edition: 2024
5+
//@ check-pass
6+
7+
use std::ops::Range;
8+
9+
fn main() {
10+
let opt = Some(None..Some(1));
11+
12+
// test `if let`
13+
if let first = &opt {}
14+
//~^ WARN irrefutable `if let` pattern
15+
16+
if let first = &opt && let Some(second) = first {}
17+
18+
if let first = &opt && let (a, b) = (1, 2) {}
19+
20+
if let first = &opt && let None = Some(1) {}
21+
22+
if 4 * 2 == 0 && let first = &opt {}
23+
24+
if let first = &opt
25+
&& let Some(second) = first
26+
&& let None = second.start
27+
&& let v = 0
28+
{}
29+
30+
if let Range { start: local_start, end: _ } = (None..Some(1)) {}
31+
//~^ WARN irrefutable `if let` pattern
32+
33+
if let Range { start: local_start, end: _ } = (None..Some(1))
34+
&& let None = local_start
35+
{}
36+
37+
if let (a, b, c) = (Some(1), Some(1), Some(1)) {}
38+
//~^ WARN irrefutable `if let` pattern
39+
40+
if let (a, b, c) = (Some(1), Some(1), Some(1)) && let None = Some(1) {}
41+
42+
if let Some(ref first) = opt
43+
&& let Range { start: local_start, end: _ } = first
44+
&& let None = local_start
45+
{}
46+
47+
// test `else if let`
48+
if opt == Some(None..None) {
49+
} else if let x = opt.clone().map(|_| 1) {
50+
//~^ WARN irrefutable `if let` pattern
51+
}
52+
53+
if opt == Some(None..None) {
54+
} else if let x = opt.clone().map(|_| 1)
55+
&& x == Some(1)
56+
{}
57+
58+
if opt == Some(None..None) {
59+
} else if opt.is_some() && let x = &opt
60+
{}
61+
62+
if opt == Some(None..None) {
63+
} else {
64+
if let x = opt.clone().map(|_| 1) && x == Some(1)
65+
{}
66+
}
67+
68+
// test `if let guard`
69+
match opt {
70+
Some(ref first) if let second = first => {}
71+
//~^ WARN irrefutable `if let` guard pattern
72+
_ => {}
73+
}
74+
75+
match opt {
76+
Some(ref first)
77+
if let second = first
78+
&& let _third = second
79+
&& let v = 4 + 4 => {}
80+
_ => {}
81+
}
82+
83+
match opt {
84+
Some(ref first)
85+
if let Range { start: local_start, end: _ } = first
86+
&& let None = local_start => {}
87+
_ => {}
88+
}
89+
90+
match opt {
91+
Some(ref first)
92+
if let Range { start: Some(_), end: local_end } = first
93+
&& let v = local_end
94+
&& let w = v => {}
95+
_ => {}
96+
}
97+
98+
// test `while let`
99+
while let first = &opt {}
100+
//~^ WARN irrefutable `while let` pattern
101+
102+
while let first = &opt
103+
&& let (a, b) = (1, 2)
104+
{}
105+
106+
while let first = &opt
107+
&& let Some(second) = first
108+
&& let None = second.start
109+
{}
110+
111+
while let Some(ref first) = opt
112+
&& let second = first
113+
&& let _third = second
114+
{}
115+
116+
while let Some(ref first) = opt
117+
&& let Range { start: local_start, end: _ } = first
118+
&& let None = local_start
119+
{}
120+
}

‎tests/ui/binding/irrefutable-in-let-chains.stderr‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
warning: irrefutable `if let` pattern
2+
--> $DIR/irrefutable-in-let-chains.rs:13:8
3+
|
4+
LL | if let first = &opt {}
5+
| ^^^^^^^^^^^^^^^^
6+
|
7+
= note: this pattern will always match, so the `if let` is useless
8+
= help: consider replacing the `if let` with a `let`
9+
= note: `#[warn(irrefutable_let_patterns)]` on by default
10+
11+
warning: irrefutable `if let` pattern
12+
--> $DIR/irrefutable-in-let-chains.rs:30:8
13+
|
14+
LL | if let Range { start: local_start, end: _ } = (None..Some(1)) {}
15+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
16+
|
17+
= note: this pattern will always match, so the `if let` is useless
18+
= help: consider replacing the `if let` with a `let`
19+
20+
warning: irrefutable `if let` pattern
21+
--> $DIR/irrefutable-in-let-chains.rs:37:8
22+
|
23+
LL | if let (a, b, c) = (Some(1), Some(1), Some(1)) {}
24+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
25+
|
26+
= note: this pattern will always match, so the `if let` is useless
27+
= help: consider replacing the `if let` with a `let`
28+
29+
warning: irrefutable `if let` pattern
30+
--> $DIR/irrefutable-in-let-chains.rs:49:15
31+
|
32+
LL | } else if let x = opt.clone().map(|_| 1) {
33+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
34+
|
35+
= note: this pattern will always match, so the `if let` is useless
36+
= help: consider replacing the `if let` with a `let`
37+
38+
warning: irrefutable `if let` guard pattern
39+
--> $DIR/irrefutable-in-let-chains.rs:70:28
40+
|
41+
LL | Some(ref first) if let second = first => {}
42+
| ^^^^^^^^^^^^^^^^^^
43+
|
44+
= note: this pattern will always match, so the guard is useless
45+
= help: consider removing the guard and adding a `let` inside the match arm
46+
47+
warning: irrefutable `while let` pattern
48+
--> $DIR/irrefutable-in-let-chains.rs:99:11
49+
|
50+
LL | while let first = &opt {}
51+
| ^^^^^^^^^^^^^^^^
52+
|
53+
= note: this pattern will always match, so the loop will never exit
54+
= help: consider instead using a `loop { ... }` with a `let` inside it
55+
56+
warning: 6 warnings emitted
57+

‎tests/ui/rfcs/rfc-2294-if-let-guard/warns.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
#[deny(irrefutable_let_patterns)]
2-
32
fn irrefutable_let_guard() {
43
match Some(()) {
54
Some(x) if let () = x => {}
@@ -21,7 +20,6 @@ fn trailing_irrefutable_pattern_binding() {
2120
fn trailing_irrefutable_in_let_chain() {
2221
match Some(5) {
2322
Some(x) if let Some(y) = Some(x) && let z = 0 => {}
24-
//~^ ERROR trailing irrefutable pattern in let chain
2523
_ => {}
2624
}
2725
}

0 commit comments

Comments
 (0)