[never_loop]: Fix FP with let..else statements.

This commit is contained in:
Yotam Ofek 2022-09-19 10:02:17 +00:00
parent e120fb10c6
commit d63aeceaa1
3 changed files with 71 additions and 20 deletions

View file

@ -42,6 +42,7 @@ pub(super) fn check(
} }
} }
#[derive(Copy, Clone)]
enum NeverLoopResult { enum NeverLoopResult {
// A break/return always get triggered but not necessarily for the main loop. // A break/return always get triggered but not necessarily for the main loop.
AlwaysBreak, AlwaysBreak,
@ -51,8 +52,8 @@ enum NeverLoopResult {
} }
#[must_use] #[must_use]
fn absorb_break(arg: &NeverLoopResult) -> NeverLoopResult { fn absorb_break(arg: NeverLoopResult) -> NeverLoopResult {
match *arg { match arg {
NeverLoopResult::AlwaysBreak | NeverLoopResult::Otherwise => NeverLoopResult::Otherwise, NeverLoopResult::AlwaysBreak | NeverLoopResult::Otherwise => NeverLoopResult::Otherwise,
NeverLoopResult::MayContinueMainLoop => NeverLoopResult::MayContinueMainLoop, NeverLoopResult::MayContinueMainLoop => NeverLoopResult::MayContinueMainLoop,
} }
@ -92,19 +93,29 @@ fn combine_branches(b1: NeverLoopResult, b2: NeverLoopResult) -> NeverLoopResult
} }
fn never_loop_block(block: &Block<'_>, main_loop_id: HirId) -> NeverLoopResult { fn never_loop_block(block: &Block<'_>, main_loop_id: HirId) -> NeverLoopResult {
let mut iter = block.stmts.iter().filter_map(stmt_to_expr).chain(block.expr); let mut iter = block
.stmts
.iter()
.filter_map(stmt_to_expr)
.chain(block.expr.map(|expr| (expr, None)));
never_loop_expr_seq(&mut iter, main_loop_id) never_loop_expr_seq(&mut iter, main_loop_id)
} }
fn never_loop_expr_seq<'a, T: Iterator<Item = &'a Expr<'a>>>(es: &mut T, main_loop_id: HirId) -> NeverLoopResult { fn never_loop_expr_seq<'a, T: Iterator<Item = (&'a Expr<'a>, Option<&'a Block<'a>>)>>(
es.map(|e| never_loop_expr(e, main_loop_id)) es: &mut T,
main_loop_id: HirId,
) -> NeverLoopResult {
es.map(|(e, els)| {
let e = never_loop_expr(e, main_loop_id);
els.map_or(e, |els| combine_branches(e, never_loop_block(els, main_loop_id)))
})
.fold(NeverLoopResult::Otherwise, combine_seq) .fold(NeverLoopResult::Otherwise, combine_seq)
} }
fn stmt_to_expr<'tcx>(stmt: &Stmt<'tcx>) -> Option<&'tcx Expr<'tcx>> { fn stmt_to_expr<'tcx>(stmt: &Stmt<'tcx>) -> Option<(&'tcx Expr<'tcx>, Option<&'tcx Block<'tcx>>)> {
match stmt.kind { match stmt.kind {
StmtKind::Semi(e, ..) | StmtKind::Expr(e, ..) => Some(e), StmtKind::Semi(e, ..) | StmtKind::Expr(e, ..) => Some((e, None)),
StmtKind::Local(local) => local.init, StmtKind::Local(local) => local.init.map(|init| (init, local.els)),
StmtKind::Item(..) => None, StmtKind::Item(..) => None,
} }
} }
@ -139,7 +150,7 @@ fn never_loop_expr(expr: &Expr<'_>, main_loop_id: HirId) -> NeverLoopResult {
| ExprKind::Index(e1, e2) => never_loop_expr_all(&mut [e1, e2].iter().copied(), main_loop_id), | ExprKind::Index(e1, e2) => never_loop_expr_all(&mut [e1, e2].iter().copied(), main_loop_id),
ExprKind::Loop(b, _, _, _) => { ExprKind::Loop(b, _, _, _) => {
// Break can come from the inner loop so remove them. // Break can come from the inner loop so remove them.
absorb_break(&never_loop_block(b, main_loop_id)) absorb_break(never_loop_block(b, main_loop_id))
}, },
ExprKind::If(e, e2, e3) => { ExprKind::If(e, e2, e3) => {
let e1 = never_loop_expr(e, main_loop_id); let e1 = never_loop_expr(e, main_loop_id);

View file

@ -1,3 +1,4 @@
#![feature(let_else)]
#![allow( #![allow(
clippy::single_match, clippy::single_match,
unused_assignments, unused_assignments,
@ -203,6 +204,32 @@ pub fn test17() {
}; };
} }
// Issue #9356: `continue` in else branch of let..else
pub fn test18() {
let x = Some(0);
let y = 0;
// might loop
let _ = loop {
let Some(x) = x else {
if y > 0 {
continue;
} else {
return;
}
};
break x;
};
// never loops
let _ = loop {
let Some(x) = x else {
return;
};
break x;
};
}
fn main() { fn main() {
test1(); test1();
test2(); test2();

View file

@ -1,5 +1,5 @@
error: this loop never actually loops error: this loop never actually loops
--> $DIR/never_loop.rs:10:5 --> $DIR/never_loop.rs:11:5
| |
LL | / loop { LL | / loop {
LL | | // clippy::never_loop LL | | // clippy::never_loop
@ -13,7 +13,7 @@ LL | | }
= note: `#[deny(clippy::never_loop)]` on by default = note: `#[deny(clippy::never_loop)]` on by default
error: this loop never actually loops error: this loop never actually loops
--> $DIR/never_loop.rs:32:5 --> $DIR/never_loop.rs:33:5
| |
LL | / loop { LL | / loop {
LL | | // never loops LL | | // never loops
@ -23,7 +23,7 @@ LL | | }
| |_____^ | |_____^
error: this loop never actually loops error: this loop never actually loops
--> $DIR/never_loop.rs:52:5 --> $DIR/never_loop.rs:53:5
| |
LL | / loop { LL | / loop {
LL | | // never loops LL | | // never loops
@ -35,7 +35,7 @@ LL | | }
| |_____^ | |_____^
error: this loop never actually loops error: this loop never actually loops
--> $DIR/never_loop.rs:54:9 --> $DIR/never_loop.rs:55:9
| |
LL | / while i == 0 { LL | / while i == 0 {
LL | | // never loops LL | | // never loops
@ -44,7 +44,7 @@ LL | | }
| |_________^ | |_________^
error: this loop never actually loops error: this loop never actually loops
--> $DIR/never_loop.rs:66:9 --> $DIR/never_loop.rs:67:9
| |
LL | / loop { LL | / loop {
LL | | // never loops LL | | // never loops
@ -56,7 +56,7 @@ LL | | }
| |_________^ | |_________^
error: this loop never actually loops error: this loop never actually loops
--> $DIR/never_loop.rs:102:5 --> $DIR/never_loop.rs:103:5
| |
LL | / while let Some(y) = x { LL | / while let Some(y) = x {
LL | | // never loops LL | | // never loops
@ -65,7 +65,7 @@ LL | | }
| |_____^ | |_____^
error: this loop never actually loops error: this loop never actually loops
--> $DIR/never_loop.rs:109:5 --> $DIR/never_loop.rs:110:5
| |
LL | / for x in 0..10 { LL | / for x in 0..10 {
LL | | // never loops LL | | // never loops
@ -82,7 +82,7 @@ LL | if let Some(x) = (0..10).next() {
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
error: this loop never actually loops error: this loop never actually loops
--> $DIR/never_loop.rs:157:5 --> $DIR/never_loop.rs:158:5
| |
LL | / 'outer: while a { LL | / 'outer: while a {
LL | | // never loops LL | | // never loops
@ -94,12 +94,25 @@ LL | | }
| |_____^ | |_____^
error: this loop never actually loops error: this loop never actually loops
--> $DIR/never_loop.rs:172:9 --> $DIR/never_loop.rs:173:9
| |
LL | / while false { LL | / while false {
LL | | break 'label; LL | | break 'label;
LL | | } LL | | }
| |_________^ | |_________^
error: aborting due to 9 previous errors error: this loop never actually loops
--> $DIR/never_loop.rs:224:13
|
LL | let _ = loop {
| _____________^
LL | | let Some(x) = x else {
LL | | return;
LL | | };
LL | |
LL | | break x;
LL | | };
| |_____^
error: aborting due to 10 previous errors