Skip to content

Commit 07a68e3

Browse files
Natural-selection1Natural-selection1
committed
Not linting irrefutable_let_patterns on let chains
1 parent 9f2ef0f commit 07a68e3

File tree

8 files changed

+190
-234
lines changed

8 files changed

+190
-234
lines changed

‎compiler/rustc_mir_build/src/errors.rs‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -765,22 +765,6 @@ pub(crate) struct LowerRangeBoundMustBeLessThanUpper {
765765
pub(crate) span: Span,
766766
}
767767

768-
#[derive(LintDiagnostic)]
769-
#[diag(mir_build_leading_irrefutable_let_patterns)]
770-
#[note]
771-
#[help]
772-
pub(crate) struct LeadingIrrefutableLetPatterns {
773-
pub(crate) count: usize,
774-
}
775-
776-
#[derive(LintDiagnostic)]
777-
#[diag(mir_build_trailing_irrefutable_let_patterns)]
778-
#[note]
779-
#[help]
780-
pub(crate) struct TrailingIrrefutableLetPatterns {
781-
pub(crate) count: usize,
782-
}
783-
784768
#[derive(LintDiagnostic)]
785769
#[diag(mir_build_bindings_with_variant_name, code = E0170)]
786770
pub(crate) struct BindingsWithVariantName {

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

Lines changed: 6 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,9 @@ impl<'p, 'tcx> Visitor<'p, 'tcx> for MatchVisitor<'p, 'tcx> {
168168
{
169169
let mut chain_refutabilities = Vec::new();
170170
let Ok(()) = self.visit_land(ex, &mut chain_refutabilities) else { return };
171-
// If at least one of the operands is a `let ... = ...`.
172-
if chain_refutabilities.iter().any(|x| x.is_some()) {
173-
self.check_let_chain(chain_refutabilities, ex.span);
171+
// Check only single let binding
172+
if let [Some((_, refutability))] = chain_refutabilities[..] {
173+
self.check_single_let(refutability, ex.span);
174174
}
175175
return;
176176
}
@@ -562,73 +562,16 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
562562
}
563563

564564
#[instrument(level = "trace", skip(self))]
565-
fn check_let_chain(
566-
&mut self,
567-
chain_refutabilities: Vec<Option<(Span, RefutableFlag)>>,
568-
whole_chain_span: Span,
569-
) {
565+
fn check_single_let(&mut self, refutability: RefutableFlag, whole_chain_span: Span) {
570566
assert!(self.let_source != LetSource::None);
571-
572-
if chain_refutabilities.iter().all(|r| matches!(*r, Some((_, Irrefutable)))) {
573-
// The entire chain is made up of irrefutable `let` statements
567+
if matches!(refutability, Irrefutable) {
574568
report_irrefutable_let_patterns(
575569
self.tcx,
576570
self.lint_level,
577571
self.let_source,
578-
chain_refutabilities.len(),
572+
1,
579573
whole_chain_span,
580574
);
581-
return;
582-
}
583-
584-
if let Some(until) =
585-
chain_refutabilities.iter().position(|r| !matches!(*r, Some((_, Irrefutable))))
586-
&& until > 0
587-
{
588-
// The chain has a non-zero prefix of irrefutable `let` statements.
589-
590-
// Check if the let source is while, for there is no alternative place to put a prefix,
591-
// and we shouldn't lint.
592-
// For let guards inside a match, prefixes might use bindings of the match pattern,
593-
// so can't always be moved out.
594-
// For `else if let`, an extra indentation level would be required to move the bindings.
595-
// FIXME: Add checking whether the bindings are actually used in the prefix,
596-
// and lint if they are not.
597-
if !matches!(
598-
self.let_source,
599-
LetSource::WhileLet | LetSource::IfLetGuard | LetSource::ElseIfLet
600-
) {
601-
// Emit the lint
602-
let prefix = &chain_refutabilities[..until];
603-
let span_start = prefix[0].unwrap().0;
604-
let span_end = prefix.last().unwrap().unwrap().0;
605-
let span = span_start.to(span_end);
606-
let count = prefix.len();
607-
self.tcx.emit_node_span_lint(
608-
IRREFUTABLE_LET_PATTERNS,
609-
self.lint_level,
610-
span,
611-
LeadingIrrefutableLetPatterns { count },
612-
);
613-
}
614-
}
615-
616-
if let Some(from) =
617-
chain_refutabilities.iter().rposition(|r| !matches!(*r, Some((_, Irrefutable))))
618-
&& from != (chain_refutabilities.len() - 1)
619-
{
620-
// The chain has a non-empty suffix of irrefutable `let` statements
621-
let suffix = &chain_refutabilities[from + 1..];
622-
let span_start = suffix[0].unwrap().0;
623-
let span_end = suffix.last().unwrap().unwrap().0;
624-
let span = span_start.to(span_end);
625-
let count = suffix.len();
626-
self.tcx.emit_node_span_lint(
627-
IRREFUTABLE_LET_PATTERNS,
628-
self.lint_level,
629-
span,
630-
TrailingIrrefutableLetPatterns { count },
631-
);
632575
}
633576
}
634577

‎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
@@ -900,7 +900,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
900900
// Fallback to shims in submodules.
901901
_ => {
902902
// Math shims
903-
#[expect(irrefutable_let_patterns)]
903+
#[cfg_attr(bootstrap, expect(irrefutable_let_patterns))]
904904
if let res = shims::math::EvalContextExt::emulate_foreign_item_inner(
905905
this, link_name, abi, args, dest,
906906
)? && !matches!(res, EmulateItemResult::NotSupported)

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

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

‎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:15: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:32: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:39: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:51: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:72: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:101: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+

0 commit comments

Comments
 (0)