Skip to content

Commit b9906ac

Browse files
committed
Auto merge of rust-lang#11450 - digama0:never_loop2, r=llogiq
`never_loop` catches `loop { panic!() }` * Depends on: rust-lang#11447 This is an outgrowth of rust-lang#11447 which I felt would best be done as a separate PR because it yields significant new results. This uses typecheck results to determine divergence, meaning we can now detect cases like `loop { std::process::abort() }` or `loop { panic!() }`. A downside is that `loop { unimplemented!() }` is also being linted, which is arguably a false positive. I'm not really sure how to check this from HIR though, and it seems best to leave this epicycle for a later PR. changelog: [`never_loop`]: Now lints on `loop { panic!() }` and similar constructs
2 parents b65e544 + 44f64ac commit b9906ac

16 files changed

+281
-193
lines changed

clippy_lints/src/loops/never_loop.rs

+128-128
Large diffs are not rendered by default.

tests/ui/crashes/ice-360.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ fn main() {}
33
fn no_panic<T>(slice: &[T]) {
44
let mut iter = slice.iter();
55
loop {
6-
//~^ ERROR: this loop could be written as a `while let` loop
6+
//~^ ERROR: this loop never actually loops
7+
//~| ERROR: this loop could be written as a `while let` loop
78
//~| NOTE: `-D clippy::while-let-loop` implied by `-D warnings`
89
let _ = match iter.next() {
910
Some(ele) => ele,

tests/ui/crashes/ice-360.stderr

+17-3
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,24 @@
1+
error: this loop never actually loops
2+
--> $DIR/ice-360.rs:5:5
3+
|
4+
LL | / loop {
5+
LL | |
6+
LL | |
7+
LL | |
8+
... |
9+
LL | |
10+
LL | | }
11+
| |_____^
12+
|
13+
= note: `#[deny(clippy::never_loop)]` on by default
14+
115
error: this loop could be written as a `while let` loop
216
--> $DIR/ice-360.rs:5:5
317
|
418
LL | / loop {
519
LL | |
620
LL | |
7-
LL | | let _ = match iter.next() {
21+
LL | |
822
... |
923
LL | |
1024
LL | | }
@@ -13,13 +27,13 @@ LL | | }
1327
= note: `-D clippy::while-let-loop` implied by `-D warnings`
1428

1529
error: empty `loop {}` wastes CPU cycles
16-
--> $DIR/ice-360.rs:12:9
30+
--> $DIR/ice-360.rs:13:9
1731
|
1832
LL | loop {}
1933
| ^^^^^^^
2034
|
2135
= help: you should either use `panic!()` or add `std::thread::sleep(..);` to the loop body
2236
= note: `-D clippy::empty-loop` implied by `-D warnings`
2337

24-
error: aborting due to 2 previous errors
38+
error: aborting due to 3 previous errors
2539

tests/ui/empty_loop.rs

+3
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,20 @@ use proc_macros::{external, inline_macros};
77

88
fn should_trigger() {
99
loop {}
10+
#[allow(clippy::never_loop)]
1011
loop {
1112
loop {}
1213
}
1314

15+
#[allow(clippy::never_loop)]
1416
'outer: loop {
1517
'inner: loop {}
1618
}
1719
}
1820

1921
#[inline_macros]
2022
fn should_not_trigger() {
23+
#[allow(clippy::never_loop)]
2124
loop {
2225
panic!("This is fine")
2326
}

tests/ui/empty_loop.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,15 @@ LL | loop {}
88
= note: `-D clippy::empty-loop` implied by `-D warnings`
99

1010
error: empty `loop {}` wastes CPU cycles
11-
--> $DIR/empty_loop.rs:11:9
11+
--> $DIR/empty_loop.rs:12:9
1212
|
1313
LL | loop {}
1414
| ^^^^^^^
1515
|
1616
= help: you should either use `panic!()` or add `std::thread::sleep(..);` to the loop body
1717

1818
error: empty `loop {}` wastes CPU cycles
19-
--> $DIR/empty_loop.rs:15:9
19+
--> $DIR/empty_loop.rs:17:9
2020
|
2121
LL | 'inner: loop {}
2222
| ^^^^^^^^^^^^^^^

tests/ui/iter_out_of_bounds.rs

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ fn opaque_empty_iter() -> impl Iterator<Item = ()> {
88
}
99

1010
fn main() {
11+
#[allow(clippy::never_loop)]
1112
for _ in [1, 2, 3].iter().skip(4) {
1213
//~^ ERROR: this `.skip()` call skips more items than the iterator will produce
1314
unreachable!();

tests/ui/iter_out_of_bounds.stderr

+14-14
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this `.skip()` call skips more items than the iterator will produce
2-
--> $DIR/iter_out_of_bounds.rs:11:14
2+
--> $DIR/iter_out_of_bounds.rs:12:14
33
|
44
LL | for _ in [1, 2, 3].iter().skip(4) {
55
| ^^^^^^^^^^^^^^^^^^^^^^^^
@@ -12,103 +12,103 @@ LL | #![deny(clippy::iter_out_of_bounds)]
1212
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
1313

1414
error: this `.take()` call takes more items than the iterator will produce
15-
--> $DIR/iter_out_of_bounds.rs:15:19
15+
--> $DIR/iter_out_of_bounds.rs:16:19
1616
|
1717
LL | for (i, _) in [1, 2, 3].iter().take(4).enumerate() {
1818
| ^^^^^^^^^^^^^^^^^^^^^^^^
1919
|
2020
= note: this operation is useless and the returned iterator will simply yield the same items
2121

2222
error: this `.take()` call takes more items than the iterator will produce
23-
--> $DIR/iter_out_of_bounds.rs:21:14
23+
--> $DIR/iter_out_of_bounds.rs:22:14
2424
|
2525
LL | for _ in (&&&&&&[1, 2, 3]).iter().take(4) {}
2626
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2727
|
2828
= note: this operation is useless and the returned iterator will simply yield the same items
2929

3030
error: this `.skip()` call skips more items than the iterator will produce
31-
--> $DIR/iter_out_of_bounds.rs:24:14
31+
--> $DIR/iter_out_of_bounds.rs:25:14
3232
|
3333
LL | for _ in [1, 2, 3].iter().skip(4) {}
3434
| ^^^^^^^^^^^^^^^^^^^^^^^^
3535
|
3636
= note: this operation is useless and will create an empty iterator
3737

3838
error: this `.skip()` call skips more items than the iterator will produce
39-
--> $DIR/iter_out_of_bounds.rs:27:14
39+
--> $DIR/iter_out_of_bounds.rs:28:14
4040
|
4141
LL | for _ in [1; 3].iter().skip(4) {}
4242
| ^^^^^^^^^^^^^^^^^^^^^
4343
|
4444
= note: this operation is useless and will create an empty iterator
4545

4646
error: this `.skip()` call skips more items than the iterator will produce
47-
--> $DIR/iter_out_of_bounds.rs:33:14
47+
--> $DIR/iter_out_of_bounds.rs:34:14
4848
|
4949
LL | for _ in vec![1, 2, 3].iter().skip(4) {}
5050
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5151
|
5252
= note: this operation is useless and will create an empty iterator
5353

5454
error: this `.skip()` call skips more items than the iterator will produce
55-
--> $DIR/iter_out_of_bounds.rs:36:14
55+
--> $DIR/iter_out_of_bounds.rs:37:14
5656
|
5757
LL | for _ in vec![1; 3].iter().skip(4) {}
5858
| ^^^^^^^^^^^^^^^^^^^^^^^^^
5959
|
6060
= note: this operation is useless and will create an empty iterator
6161

6262
error: this `.skip()` call skips more items than the iterator will produce
63-
--> $DIR/iter_out_of_bounds.rs:40:14
63+
--> $DIR/iter_out_of_bounds.rs:41:14
6464
|
6565
LL | for _ in x.iter().skip(4) {}
6666
| ^^^^^^^^^^^^^^^^
6767
|
6868
= note: this operation is useless and will create an empty iterator
6969

7070
error: this `.skip()` call skips more items than the iterator will produce
71-
--> $DIR/iter_out_of_bounds.rs:44:14
71+
--> $DIR/iter_out_of_bounds.rs:45:14
7272
|
7373
LL | for _ in x.iter().skip(n) {}
7474
| ^^^^^^^^^^^^^^^^
7575
|
7676
= note: this operation is useless and will create an empty iterator
7777

7878
error: this `.skip()` call skips more items than the iterator will produce
79-
--> $DIR/iter_out_of_bounds.rs:49:14
79+
--> $DIR/iter_out_of_bounds.rs:50:14
8080
|
8181
LL | for _ in empty().skip(1) {}
8282
| ^^^^^^^^^^^^^^^
8383
|
8484
= note: this operation is useless and will create an empty iterator
8585

8686
error: this `.take()` call takes more items than the iterator will produce
87-
--> $DIR/iter_out_of_bounds.rs:52:14
87+
--> $DIR/iter_out_of_bounds.rs:53:14
8888
|
8989
LL | for _ in empty().take(1) {}
9090
| ^^^^^^^^^^^^^^^
9191
|
9292
= note: this operation is useless and the returned iterator will simply yield the same items
9393

9494
error: this `.skip()` call skips more items than the iterator will produce
95-
--> $DIR/iter_out_of_bounds.rs:55:14
95+
--> $DIR/iter_out_of_bounds.rs:56:14
9696
|
9797
LL | for _ in std::iter::once(1).skip(2) {}
9898
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
9999
|
100100
= note: this operation is useless and will create an empty iterator
101101

102102
error: this `.take()` call takes more items than the iterator will produce
103-
--> $DIR/iter_out_of_bounds.rs:58:14
103+
--> $DIR/iter_out_of_bounds.rs:59:14
104104
|
105105
LL | for _ in std::iter::once(1).take(2) {}
106106
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
107107
|
108108
= note: this operation is useless and the returned iterator will simply yield the same items
109109

110110
error: this `.take()` call takes more items than the iterator will produce
111-
--> $DIR/iter_out_of_bounds.rs:61:14
111+
--> $DIR/iter_out_of_bounds.rs:62:14
112112
|
113113
LL | for x in [].iter().take(1) {
114114
| ^^^^^^^^^^^^^^^^^

tests/ui/needless_collect_indirect.rs

+6
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,7 @@ mod issue_8553 {
260260
let w = v.iter().collect::<Vec<_>>();
261261
//~^ ERROR: avoid using `collect()` when not needed
262262
// Do lint
263+
#[allow(clippy::never_loop)]
263264
for _ in 0..w.len() {
264265
todo!();
265266
}
@@ -270,6 +271,7 @@ mod issue_8553 {
270271
let v: Vec<usize> = vec.iter().map(|i| i * i).collect();
271272
let w = v.iter().collect::<Vec<_>>();
272273
// Do not lint, because w is used.
274+
#[allow(clippy::never_loop)]
273275
for _ in 0..w.len() {
274276
todo!();
275277
}
@@ -283,6 +285,7 @@ mod issue_8553 {
283285
let mut w = v.iter().collect::<Vec<_>>();
284286
//~^ ERROR: avoid using `collect()` when not needed
285287
// Do lint
288+
#[allow(clippy::never_loop)]
286289
while 1 == w.len() {
287290
todo!();
288291
}
@@ -293,6 +296,7 @@ mod issue_8553 {
293296
let mut v: Vec<usize> = vec.iter().map(|i| i * i).collect();
294297
let mut w = v.iter().collect::<Vec<_>>();
295298
// Do not lint, because w is used.
299+
#[allow(clippy::never_loop)]
296300
while 1 == w.len() {
297301
todo!();
298302
}
@@ -306,6 +310,7 @@ mod issue_8553 {
306310
let mut w = v.iter().collect::<Vec<_>>();
307311
//~^ ERROR: avoid using `collect()` when not needed
308312
// Do lint
313+
#[allow(clippy::never_loop)]
309314
while let Some(i) = Some(w.len()) {
310315
todo!();
311316
}
@@ -316,6 +321,7 @@ mod issue_8553 {
316321
let mut v: Vec<usize> = vec.iter().map(|i| i * i).collect();
317322
let mut w = v.iter().collect::<Vec<_>>();
318323
// Do not lint, because w is used.
324+
#[allow(clippy::never_loop)]
319325
while let Some(i) = Some(w.len()) {
320326
todo!();
321327
}

tests/ui/needless_collect_indirect.stderr

+5-2
Original file line numberDiff line numberDiff line change
@@ -230,11 +230,12 @@ help: take the original Iterator's count instead of collecting it and finding th
230230
LL ~
231231
LL |
232232
LL | // Do lint
233+
LL | #[allow(clippy::never_loop)]
233234
LL ~ for _ in 0..v.iter().count() {
234235
|
235236

236237
error: avoid using `collect()` when not needed
237-
--> $DIR/needless_collect_indirect.rs:283:30
238+
--> $DIR/needless_collect_indirect.rs:285:30
238239
|
239240
LL | let mut w = v.iter().collect::<Vec<_>>();
240241
| ^^^^^^^
@@ -247,11 +248,12 @@ help: take the original Iterator's count instead of collecting it and finding th
247248
LL ~
248249
LL |
249250
LL | // Do lint
251+
LL | #[allow(clippy::never_loop)]
250252
LL ~ while 1 == v.iter().count() {
251253
|
252254

253255
error: avoid using `collect()` when not needed
254-
--> $DIR/needless_collect_indirect.rs:306:30
256+
--> $DIR/needless_collect_indirect.rs:310:30
255257
|
256258
LL | let mut w = v.iter().collect::<Vec<_>>();
257259
| ^^^^^^^
@@ -264,6 +266,7 @@ help: take the original Iterator's count instead of collecting it and finding th
264266
LL ~
265267
LL |
266268
LL | // Do lint
269+
LL | #[allow(clippy::never_loop)]
267270
LL ~ while let Some(i) = Some(v.iter().count()) {
268271
|
269272

tests/ui/never_loop.rs

+45-2
Original file line numberDiff line numberDiff line change
@@ -337,10 +337,8 @@ pub fn test26() {
337337

338338
pub fn test27() {
339339
loop {
340-
//~^ ERROR: this loop never actually loops
341340
'label: {
342341
let x = true;
343-
// Lints because we cannot prove it's always `true`
344342
if x {
345343
break 'label;
346344
}
@@ -349,6 +347,51 @@ pub fn test27() {
349347
}
350348
}
351349

350+
// issue 11004
351+
pub fn test29() {
352+
loop {
353+
'label: {
354+
if true {
355+
break 'label;
356+
}
357+
return;
358+
}
359+
}
360+
}
361+
362+
pub fn test30() {
363+
'a: loop {
364+
'b: {
365+
for j in 0..2 {
366+
if j == 1 {
367+
break 'b;
368+
}
369+
}
370+
break 'a;
371+
}
372+
}
373+
}
374+
375+
pub fn test31(b: bool) {
376+
'a: loop {
377+
'b: {
378+
'c: loop {
379+
//~^ ERROR: this loop never actually loops
380+
if b { break 'c } else { break 'b }
381+
}
382+
continue 'a;
383+
}
384+
break 'a;
385+
}
386+
}
387+
388+
pub fn test32(b: bool) {
389+
loop {
390+
//~^ ERROR: this loop never actually loops
391+
panic!("oh no");
392+
}
393+
}
394+
352395
fn main() {
353396
test1();
354397
test2();

tests/ui/never_loop.stderr

+12-6
Original file line numberDiff line numberDiff line change
@@ -153,16 +153,22 @@ LL | if let Some(_) = (0..20).next() {
153153
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
154154

155155
error: this loop never actually loops
156-
--> $DIR/never_loop.rs:339:5
156+
--> $DIR/never_loop.rs:378:13
157+
|
158+
LL | / 'c: loop {
159+
LL | |
160+
LL | | if b { break 'c } else { break 'b }
161+
LL | | }
162+
| |_____________^
163+
164+
error: this loop never actually loops
165+
--> $DIR/never_loop.rs:389:5
157166
|
158167
LL | / loop {
159168
LL | |
160-
LL | | 'label: {
161-
LL | | let x = true;
162-
... |
163-
LL | | }
169+
LL | | panic!("oh no");
164170
LL | | }
165171
| |_____^
166172

167-
error: aborting due to 14 previous errors
173+
error: aborting due to 15 previous errors
168174

0 commit comments

Comments
 (0)