Skip to content

Commit 7f96771

Browse files
est31est31
committed
Also consider match guards for divergence check
Plus, add some tests for the divergence check.
1 parent e42f70a commit 7f96771

File tree

3 files changed

+120
-16
lines changed

3 files changed

+120
-16
lines changed

‎clippy_lints/src/manual_let_else.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,11 @@ fn expr_diverges(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool {
194194
ControlFlow::Continue(Descend::No)
195195
},
196196
ExprKind::Match(match_expr, match_arms, _) => {
197-
let diverges =
198-
expr_diverges(cx, match_expr) || match_arms.iter().all(|arm| expr_diverges(cx, arm.body));
197+
let diverges = expr_diverges(cx, match_expr)
198+
|| match_arms.iter().all(|arm| {
199+
let guard_diverges = arm.guard.as_ref().map_or(false, |g| expr_diverges(cx, g.body()));
200+
guard_diverges || expr_diverges(cx, arm.body)
201+
});
199202
if diverges {
200203
return ControlFlow::Break(());
201204
}

‎tests/ui/manual_let_else.rs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
clippy::collapsible_else_if,
44
clippy::unused_unit,
55
clippy::let_unit_value,
6+
clippy::match_single_binding,
67
clippy::never_loop
78
)]
89
#![warn(clippy::manual_let_else)]
@@ -55,6 +56,38 @@ fn fire() {
5556
if true { return } else { panic!() }
5657
};
5758

59+
// Diverging after an if still makes the block diverge:
60+
let v = if let Some(v_some) = g() {
61+
v_some
62+
} else {
63+
if true {}
64+
panic!();
65+
};
66+
67+
// A match diverges if all branches diverge:
68+
// Note: the corresponding let-else requires a ; at the end of the match
69+
// as otherwise the type checker does not turn it into a ! type.
70+
let v = if let Some(v_some) = g() {
71+
v_some
72+
} else {
73+
match () {
74+
_ if panic!() => {},
75+
_ => panic!(),
76+
}
77+
};
78+
79+
// An if's expression can cause divergence:
80+
let v = if let Some(v_some) = g() { v_some } else { if panic!() {} };
81+
82+
// An expression of a match can cause divergence:
83+
let v = if let Some(v_some) = g() {
84+
v_some
85+
} else {
86+
match panic!() {
87+
_ => {},
88+
}
89+
};
90+
5891
// Top level else if
5992
let v = if let Some(v_some) = g() {
6093
v_some

‎tests/ui/manual_let_else.stderr‎

Lines changed: 82 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
error: this could be rewritten as `let...else`
2-
--> $DIR/manual_let_else.rs:17:5
2+
--> $DIR/manual_let_else.rs:18:5
33
|
44
LL | let v = if let Some(v_some) = g() { v_some } else { return };
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { return };`
66
|
77
= note: `-D clippy::manual-let-else` implied by `-D warnings`
88

99
error: this could be rewritten as `let...else`
10-
--> $DIR/manual_let_else.rs:18:5
10+
--> $DIR/manual_let_else.rs:19:5
1111
|
1212
LL | / let v = if let Some(v_some) = g() {
1313
LL | | v_some
@@ -24,7 +24,7 @@ LL + };
2424
|
2525

2626
error: this could be rewritten as `let...else`
27-
--> $DIR/manual_let_else.rs:24:5
27+
--> $DIR/manual_let_else.rs:25:5
2828
|
2929
LL | / let v = if let Some(v) = g() {
3030
LL | | // Blocks around the identity should have no impact
@@ -45,25 +45,25 @@ LL + };
4545
|
4646

4747
error: this could be rewritten as `let...else`
48-
--> $DIR/manual_let_else.rs:37:9
48+
--> $DIR/manual_let_else.rs:38:9
4949
|
5050
LL | let v = if let Some(v_some) = g() { v_some } else { continue };
5151
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { continue };`
5252

5353
error: this could be rewritten as `let...else`
54-
--> $DIR/manual_let_else.rs:38:9
54+
--> $DIR/manual_let_else.rs:39:9
5555
|
5656
LL | let v = if let Some(v_some) = g() { v_some } else { break };
5757
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { break };`
5858

5959
error: this could be rewritten as `let...else`
60-
--> $DIR/manual_let_else.rs:42:5
60+
--> $DIR/manual_let_else.rs:43:5
6161
|
6262
LL | let v = if let Some(v_some) = g() { v_some } else { panic!() };
6363
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { panic!() };`
6464

6565
error: this could be rewritten as `let...else`
66-
--> $DIR/manual_let_else.rs:45:5
66+
--> $DIR/manual_let_else.rs:46:5
6767
|
6868
LL | / let v = if let Some(v_some) = g() {
6969
LL | | v_some
@@ -80,7 +80,7 @@ LL + };
8080
|
8181

8282
error: this could be rewritten as `let...else`
83-
--> $DIR/manual_let_else.rs:52:5
83+
--> $DIR/manual_let_else.rs:53:5
8484
|
8585
LL | / let v = if let Some(v_some) = g() {
8686
LL | | v_some
@@ -97,7 +97,75 @@ LL + };
9797
|
9898

9999
error: this could be rewritten as `let...else`
100-
--> $DIR/manual_let_else.rs:59:5
100+
--> $DIR/manual_let_else.rs:60:5
101+
|
102+
LL | / let v = if let Some(v_some) = g() {
103+
LL | | v_some
104+
LL | | } else {
105+
LL | | if true {}
106+
LL | | panic!();
107+
LL | | };
108+
| |______^
109+
|
110+
help: consider writing
111+
|
112+
LL ~ let Some(v_some) = g() else {
113+
LL + if true {}
114+
LL + panic!();
115+
LL + };
116+
|
117+
118+
error: this could be rewritten as `let...else`
119+
--> $DIR/manual_let_else.rs:70:5
120+
|
121+
LL | / let v = if let Some(v_some) = g() {
122+
LL | | v_some
123+
LL | | } else {
124+
LL | | match () {
125+
... |
126+
LL | | }
127+
LL | | };
128+
| |______^
129+
|
130+
help: consider writing
131+
|
132+
LL ~ let Some(v_some) = g() else {
133+
LL + match () {
134+
LL + _ if panic!() => {},
135+
LL + _ => panic!(),
136+
LL + }
137+
LL + };
138+
|
139+
140+
error: this could be rewritten as `let...else`
141+
--> $DIR/manual_let_else.rs:80:5
142+
|
143+
LL | let v = if let Some(v_some) = g() { v_some } else { if panic!() {} };
144+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { if panic!() {} };`
145+
146+
error: this could be rewritten as `let...else`
147+
--> $DIR/manual_let_else.rs:83:5
148+
|
149+
LL | / let v = if let Some(v_some) = g() {
150+
LL | | v_some
151+
LL | | } else {
152+
LL | | match panic!() {
153+
LL | | _ => {},
154+
LL | | }
155+
LL | | };
156+
| |______^
157+
|
158+
help: consider writing
159+
|
160+
LL ~ let Some(v_some) = g() else {
161+
LL + match panic!() {
162+
LL + _ => {},
163+
LL + }
164+
LL + };
165+
|
166+
167+
error: this could be rewritten as `let...else`
168+
--> $DIR/manual_let_else.rs:92:5
101169
|
102170
LL | / let v = if let Some(v_some) = g() {
103171
LL | | v_some
@@ -118,7 +186,7 @@ LL + } };
118186
|
119187

120188
error: this could be rewritten as `let...else`
121-
--> $DIR/manual_let_else.rs:68:5
189+
--> $DIR/manual_let_else.rs:101:5
122190
|
123191
LL | / let v = if let Some(v_some) = g() {
124192
LL | | v_some
@@ -147,7 +215,7 @@ LL + };
147215
|
148216

149217
error: this could be rewritten as `let...else`
150-
--> $DIR/manual_let_else.rs:85:5
218+
--> $DIR/manual_let_else.rs:118:5
151219
|
152220
LL | / let (v, w) = if let Some(v_some) = g().map(|v| (v, 42)) {
153221
LL | | v_some
@@ -164,7 +232,7 @@ LL + };
164232
|
165233

166234
error: this could be rewritten as `let...else`
167-
--> $DIR/manual_let_else.rs:92:5
235+
--> $DIR/manual_let_else.rs:125:5
168236
|
169237
LL | / let v = if let (Some(v_some), w_some) = (g(), 0) {
170238
LL | | (w_some, v_some)
@@ -181,7 +249,7 @@ LL + };
181249
|
182250

183251
error: this could be rewritten as `let...else`
184-
--> $DIR/manual_let_else.rs:101:13
252+
--> $DIR/manual_let_else.rs:134:13
185253
|
186254
LL | let $n = if let Some(v) = $e { v } else { return };
187255
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { return };`
@@ -191,5 +259,5 @@ LL | create_binding_if_some!(w, g());
191259
|
192260
= note: this error originates in the macro `create_binding_if_some` (in Nightly builds, run with -Z macro-backtrace for more info)
193261

194-
error: aborting due to 13 previous errors
262+
error: aborting due to 17 previous errors
195263

0 commit comments

Comments
 (0)