Skip to content

Commit

Permalink
stop warning never-returning functions and add more test cases
Browse files Browse the repository at this point in the history
  • Loading branch information
@J-ZhengLi
J-ZhengLi committed Nov 21, 2023
1 parent 2d9fc6d commit 1d15de5
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 29 deletions.
56 changes: 36 additions & 20 deletions clippy_lints/src/loops/infinite_loops.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_lint_allowed;
use clippy_utils::{fn_def_id, is_lint_allowed};
use hir::intravisit::{walk_expr, Visitor};
use hir::{Block, Destination, Expr, ExprKind, FnRetTy, Ty, TyKind};
use rustc_ast::Label;
Expand All @@ -9,10 +9,10 @@ use rustc_lint::LateContext;

use super::INFINITE_LOOPS;

pub(super) fn check(
cx: &LateContext<'_>,
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &Expr<'_>,
loop_block: &Block<'_>,
loop_block: &'tcx Block<'_>,
label: Option<Label>,
parent_fn_ret_ty: FnRetTy<'_>,
) {
Expand All @@ -31,17 +31,23 @@ pub(super) fn check(
// First, find any `break` or `return` without entering any inner loop,
// then, find `return` or labeled `break` which breaks this loop with entering inner loop,
// otherwise this loop is a infinite loop.
let mut direct_br_or_ret_finder = BreakOrRetFinder::default();
direct_br_or_ret_finder.visit_block(loop_block);
let mut direct_visitor = LoopVisitor {
cx,
label,
is_finite: false,
enter_nested_loop: false,
};
direct_visitor.visit_block(loop_block);

let is_finite_loop = direct_br_or_ret_finder.found || {
let mut inner_br_or_ret_finder = BreakOrRetFinder {
let is_finite_loop = direct_visitor.is_finite || {
let mut inner_loop_visitor = LoopVisitor {
cx,
label,
is_finite: false,
enter_nested_loop: true,
..Default::default()
};
inner_br_or_ret_finder.visit_block(loop_block);
inner_br_or_ret_finder.found
inner_loop_visitor.visit_block(loop_block);
inner_loop_visitor.is_finite
};

if !is_finite_loop {
Expand All @@ -56,37 +62,47 @@ pub(super) fn check(
} else {
diag.span_help(
expr.span,
"if this is not intended, add a `break` or `return` condition in this loop",
"if this is not intended, try adding a `break` or `return` condition in this loop",
);
}
});
}
}

#[derive(Default)]
struct BreakOrRetFinder {
struct LoopVisitor<'hir, 'tcx> {
cx: &'hir LateContext<'tcx>,
label: Option<Label>,
found: bool,
is_finite: bool,
enter_nested_loop: bool,
}

impl<'hir> Visitor<'hir> for BreakOrRetFinder {
impl<'hir> Visitor<'hir> for LoopVisitor<'hir, '_> {
fn visit_expr(&mut self, ex: &'hir Expr<'_>) {
match &ex.kind {
ExprKind::Break(Destination { label, .. }, ..) => {
// When entering nested loop, only by breaking this loop's label
// would be considered as exiting this loop.
if self.enter_nested_loop {
if label.is_some() && *label == self.label {
self.found = true;
self.is_finite = true;
}
} else {
self.found = true;
self.is_finite = true;
}
},
ExprKind::Ret(..) => self.found = true,
ExprKind::Ret(..) => self.is_finite = true,
ExprKind::Loop(..) if !self.enter_nested_loop => (),
_ => walk_expr(self, ex),
_ => {
// Calls to a function that never return
if let Some(did) = fn_def_id(self.cx, ex) {
let fn_ret_ty = self.cx.tcx.fn_sig(did).skip_binder().output().skip_binder();
if fn_ret_ty.is_never() {
self.is_finite = true;
return;
}
}
walk_expr(self, ex);
},
}
}
}
52 changes: 52 additions & 0 deletions tests/ui/infinite_loops.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ fn no_break() {
do_something();
}
}
fn no_break_return_some_ty() -> Option<u8> {
loop {
do_something();
return None;
}
loop {
//~^ ERROR: infinite loop detected
do_something();
}
}

fn no_break_never_ret() -> ! {
loop {
Expand Down Expand Up @@ -256,4 +266,46 @@ fn ret_in_macro(opt: Option<u8>) {
}
}

fn panic_like_macros_1() {
loop {
do_something();
panic!();
}
}

fn panic_like_macros_2() {
let mut x = 0;

loop {
do_something();
if true {
todo!();
}
}
loop {
do_something();
x += 1;
assert_eq!(x, 0);
}
loop {
do_something();
assert!(x % 2 == 0);
}
loop {
do_something();
match Some(1) {
Some(n) => println!("{n}"),
None => unreachable!("It won't happen"),
}
}
}

fn exit_directly(cond: bool) {
loop {
if cond {
std::process::exit(0);
}
}
}

fn main() {}
34 changes: 25 additions & 9 deletions tests/ui/infinite_loops.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,23 @@ LL | fn no_break() -> ! {
| ++++

error: infinite loop detected
--> $DIR/infinite_loops.rs:21:5
--> $DIR/infinite_loops.rs:18:5
|
LL | / loop {
LL | | do_something();
LL | | }
| |_____^
|
help: if this is not intended, add a `break` or `return` condition in this loop
--> $DIR/infinite_loops.rs:18:5
|
LL | / loop {
LL | | do_something();
LL | | }
| |_____^

error: infinite loop detected
--> $DIR/infinite_loops.rs:30:5
|
LL | / loop {
LL | | fn inner_fn() -> ! {
Expand All @@ -31,7 +47,7 @@ LL | fn no_break_never_ret_noise() -> ! {
| ++++

error: infinite loop detected
--> $DIR/infinite_loops.rs:64:5
--> $DIR/infinite_loops.rs:73:5
|
LL | / loop {
LL | |
Expand All @@ -48,7 +64,7 @@ LL | fn break_inner_but_not_outer_1(cond: bool) -> ! {
| ++++

error: infinite loop detected
--> $DIR/infinite_loops.rs:75:5
--> $DIR/infinite_loops.rs:84:5
|
LL | / loop {
LL | |
Expand All @@ -65,7 +81,7 @@ LL | fn break_inner_but_not_outer_2(cond: bool) -> ! {
| ++++

error: infinite loop detected
--> $DIR/infinite_loops.rs:89:9
--> $DIR/infinite_loops.rs:98:9
|
LL | / loop {
LL | |
Expand All @@ -79,7 +95,7 @@ LL | fn break_outer_but_not_inner() -> ! {
| ++++

error: infinite loop detected
--> $DIR/infinite_loops.rs:112:9
--> $DIR/infinite_loops.rs:121:9
|
LL | / loop {
LL | |
Expand All @@ -96,7 +112,7 @@ LL | fn break_wrong_loop(cond: bool) -> ! {
| ++++

error: infinite loop detected
--> $DIR/infinite_loops.rs:152:5
--> $DIR/infinite_loops.rs:161:5
|
LL | / loop {
LL | |
Expand All @@ -113,7 +129,7 @@ LL | fn match_like() -> ! {
| ++++

error: infinite loop detected
--> $DIR/infinite_loops.rs:193:5
--> $DIR/infinite_loops.rs:202:5
|
LL | / loop {
LL | |
Expand All @@ -127,7 +143,7 @@ LL | fn match_like() -> ! {
| ++++

error: infinite loop detected
--> $DIR/infinite_loops.rs:198:5
--> $DIR/infinite_loops.rs:207:5
|
LL | / loop {
LL | |
Expand All @@ -143,5 +159,5 @@ help: if this is intentional, consider specifing `!` as function return
LL | fn match_like() -> ! {
| ++++

error: aborting due to 9 previous errors
error: aborting due to 10 previous errors

0 comments on commit 1d15de5

Please sign in to comment.