Skip to content

Commit 7200237

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

File tree

8 files changed

+204
-234
lines changed

8 files changed

+204
-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+
// For let chains, don't lint.
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: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
// https://github.com/rust-lang/rust/issues/139369
2+
// Check that the lint `irrefutable_let_patterns` now
3+
// only checks let chain with exactly one 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+
// check `if let`
15+
if let first = &opt {}
16+
//~^ WARN irrefutable `if let` pattern
17+
18+
if let first = &opt
19+
&& let Some(second) = first
20+
{}
21+
22+
if let first = &opt
23+
&& let (a, b) = (1, 2)
24+
{}
25+
26+
if let first = &opt
27+
&& let None = Some(1)
28+
{}
29+
30+
if 4 * 2 == 0
31+
&& let first = &opt
32+
{}
33+
34+
if let first = &opt
35+
&& let Some(second) = first
36+
&& let None = second.start
37+
&& let v = 0
38+
{}
39+
40+
if let Range { start: local_start, end: _ } = (None..Some(1)) {}
41+
//~^ WARN irrefutable `if let` pattern
42+
43+
if let Range { start: local_start, end: _ } = (None..Some(1))
44+
&& let None = local_start
45+
{}
46+
47+
if let (a, b, c) = (Some(1), Some(1), Some(1)) {}
48+
//~^ WARN irrefutable `if let` pattern
49+
50+
if let (a, b, c) = (Some(1), Some(1), Some(1))
51+
&& let None = Some(1)
52+
{}
53+
54+
if let Some(ref first) = opt
55+
&& let Range { start: local_start, end: _ } = first
56+
&& let None = local_start
57+
{}
58+
59+
// check `else if let`
60+
if opt == Some(None..None) {
61+
} else if let x = opt.clone().map(|_| 1) {
62+
//~^ WARN irrefutable `if let` pattern
63+
}
64+
65+
if opt == Some(None..None) {
66+
} else if let x = opt.clone().map(|_| 1)
67+
&& x == Some(1)
68+
{
69+
}
70+
71+
if opt == Some(None..None) {
72+
} else if opt.is_some()
73+
&& let x = &opt
74+
{
75+
}
76+
77+
if opt == Some(None..None) {
78+
} else {
79+
if let x = opt.clone().map(|_| 1)
80+
&& x == Some(1)
81+
{}
82+
}
83+
84+
// check `if let guard`
85+
match opt {
86+
Some(ref first) if let second = first => {}
87+
//~^ WARN irrefutable `if let` guard pattern
88+
_ => {}
89+
}
90+
91+
match opt {
92+
Some(ref first)
93+
if let second = first
94+
&& let _third = second
95+
&& let v = 4 + 4 => {}
96+
_ => {}
97+
}
98+
99+
match opt {
100+
Some(ref first)
101+
if let Range { start: local_start, end: _ } = first
102+
&& let None = local_start => {}
103+
_ => {}
104+
}
105+
106+
match opt {
107+
Some(ref first)
108+
if let Range { start: Some(_), end: local_end } = first
109+
&& let v = local_end
110+
&& let w = v => {}
111+
_ => {}
112+
}
113+
114+
// check `while let`
115+
while let first = &opt {}
116+
//~^ WARN irrefutable `while let` pattern
117+
118+
while let first = &opt
119+
&& let (a, b) = (1, 2)
120+
{}
121+
122+
while let first = &opt
123+
&& let Some(second) = first
124+
&& let None = second.start
125+
{}
126+
127+
while let Some(ref first) = opt
128+
&& let second = first
129+
&& let _third = second
130+
{}
131+
132+
while let Some(ref first) = opt
133+
&& let Range { start: local_start, end: _ } = first
134+
&& let None = local_start
135+
{}
136+
}

‎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:40: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:47: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:61: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:86: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:115: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)