Skip to content

Commit 62d9409

Browse files
committed
implement unoptimized code logic for [infinite_loops]
1 parent 406d953 commit 62d9409

File tree

6 files changed

+532
-2
lines changed

6 files changed

+532
-2
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5147,6 +5147,7 @@ Released 2018-09-13
51475147
[`inefficient_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inefficient_to_string
51485148
[`infallible_destructuring_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#infallible_destructuring_match
51495149
[`infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_iter
5150+
[`infinite_loops`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_loops
51505151
[`inherent_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string
51515152
[`inherent_to_string_shadow_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string_shadow_display
51525153
[`init_numbered_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#init_numbered_fields

clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
263263
crate::loops::EXPLICIT_INTO_ITER_LOOP_INFO,
264264
crate::loops::EXPLICIT_ITER_LOOP_INFO,
265265
crate::loops::FOR_KV_MAP_INFO,
266+
crate::loops::INFINITE_LOOPS_INFO,
266267
crate::loops::ITER_NEXT_LOOP_INFO,
267268
crate::loops::MANUAL_FIND_INFO,
268269
crate::loops::MANUAL_FLATTEN_INFO,
+82
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
use clippy_utils::diagnostics::span_lint_and_help;
2+
use clippy_utils::is_lint_allowed;
3+
use hir::intravisit::{walk_expr, Visitor};
4+
use hir::{Block, Destination, Expr, ExprKind};
5+
use rustc_ast::Label;
6+
use rustc_hir as hir;
7+
use rustc_lint::LateContext;
8+
9+
use super::INFINITE_LOOPS;
10+
11+
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, loop_block: &Block<'_>, label: Option<Label>) {
12+
if is_lint_allowed(cx, INFINITE_LOOPS, expr.hir_id) {
13+
return;
14+
}
15+
16+
// First, find any `break` or `return` without entering any inner loop,
17+
// then, find `return` or labeled `break` which breaks this loop with entering inner loop,
18+
// otherwise this loop is a infinite loop.
19+
let mut direct_br_or_ret_finder = DirectBreakOrRetFinder::default();
20+
direct_br_or_ret_finder.visit_block(loop_block);
21+
22+
let is_finite_loop = if direct_br_or_ret_finder.found {
23+
true
24+
} else {
25+
let mut inner_br_or_ret_finder = InnerBreakOrRetFinder::with_label(label);
26+
inner_br_or_ret_finder.visit_block(loop_block);
27+
inner_br_or_ret_finder.found
28+
};
29+
30+
if !is_finite_loop {
31+
span_lint_and_help(
32+
cx,
33+
INFINITE_LOOPS,
34+
expr.span,
35+
"infinite loop detected",
36+
None,
37+
"consider adding `break` or `return` statement in the loop block",
38+
);
39+
}
40+
}
41+
42+
/// Find direct `break` or `return` without entering sub loop.
43+
#[derive(Default)]
44+
struct DirectBreakOrRetFinder {
45+
found: bool,
46+
}
47+
48+
impl<'hir> Visitor<'hir> for DirectBreakOrRetFinder {
49+
fn visit_expr(&mut self, ex: &'hir Expr<'_>) {
50+
match &ex.kind {
51+
ExprKind::Break(..) | ExprKind::Ret(..) => self.found = true,
52+
ExprKind::Loop(..) => (),
53+
_ => walk_expr(self, ex),
54+
}
55+
}
56+
}
57+
58+
/// Find `break` or `return` with entering inner loops, and find a break with corresponding label
59+
struct InnerBreakOrRetFinder {
60+
label: Option<Label>,
61+
found: bool,
62+
}
63+
64+
impl InnerBreakOrRetFinder {
65+
fn with_label(label: Option<Label>) -> Self {
66+
Self { label, found: false }
67+
}
68+
}
69+
70+
impl<'hir> Visitor<'hir> for InnerBreakOrRetFinder {
71+
fn visit_expr(&mut self, ex: &'hir Expr<'_>) {
72+
match &ex.kind {
73+
ExprKind::Break(Destination { label, .. }, ..) => {
74+
if label.is_some() && label == &self.label {
75+
self.found = true;
76+
}
77+
},
78+
ExprKind::Ret(..) => self.found = true,
79+
_ => walk_expr(self, ex),
80+
}
81+
}
82+
}

clippy_lints/src/loops/mod.rs

+69-2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ mod explicit_counter_loop;
33
mod explicit_into_iter_loop;
44
mod explicit_iter_loop;
55
mod for_kv_map;
6+
mod infinite_loops;
67
mod iter_next_loop;
78
mod manual_find;
89
mod manual_flatten;
@@ -22,7 +23,7 @@ mod while_let_on_iterator;
2223

2324
use clippy_config::msrvs::Msrv;
2425
use clippy_utils::higher;
25-
use rustc_hir::{Expr, ExprKind, LoopSource, Pat};
26+
use rustc_hir::{self as hir, Expr, ExprKind, LoopSource, Pat};
2627
use rustc_lint::{LateContext, LateLintPass};
2728
use rustc_session::{declare_tool_lint, impl_lint_pass};
2829
use rustc_span::source_map::Span;
@@ -635,15 +636,59 @@ declare_clippy_lint! {
635636
"checking for emptiness of a `Vec` in the loop condition and popping an element in the body"
636637
}
637638

639+
declare_clippy_lint! {
640+
/// ### What it does
641+
/// Checks for infinite loops in a function where the return type is not `!`
642+
/// and lint accordingly.
643+
///
644+
/// ### Why is this bad?
645+
/// A loop should be gently exited somewhere, or at lease mark its parent function as
646+
/// never return (`!`).
647+
///
648+
/// ### Example
649+
/// ```no_run
650+
/// fn run_forever() {
651+
/// loop {
652+
/// // do something
653+
/// }
654+
/// }
655+
/// ```
656+
/// If infinite loops are as intended:
657+
/// ```no_run
658+
/// fn run_forever() -> ! {
659+
/// loop {
660+
/// // do something
661+
/// }
662+
/// }
663+
/// ```
664+
/// Otherwise add a `break` or `return` condition:
665+
/// ```no_run
666+
/// fn run_forever() {
667+
/// loop {
668+
/// // do something
669+
/// if condition {
670+
/// break;
671+
/// }
672+
/// }
673+
/// }
674+
/// ```
675+
#[clippy::version = "1.75.0"]
676+
pub INFINITE_LOOPS,
677+
restriction,
678+
"possibly unintended infinite loops"
679+
}
680+
638681
pub struct Loops {
639682
msrv: Msrv,
640683
enforce_iter_loop_reborrow: bool,
684+
in_never_return_fn: bool,
641685
}
642686
impl Loops {
643687
pub fn new(msrv: Msrv, enforce_iter_loop_reborrow: bool) -> Self {
644688
Self {
645689
msrv,
646690
enforce_iter_loop_reborrow,
691+
in_never_return_fn: false,
647692
}
648693
}
649694
}
@@ -669,6 +714,7 @@ impl_lint_pass!(Loops => [
669714
MANUAL_FIND,
670715
MANUAL_WHILE_LET_SOME,
671716
UNUSED_ENUMERATE_INDEX,
717+
INFINITE_LOOPS,
672718
]);
673719

674720
impl<'tcx> LateLintPass<'tcx> for Loops {
@@ -707,10 +753,13 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
707753
// check for `loop { if let {} else break }` that could be `while let`
708754
// (also matches an explicit "match" instead of "if let")
709755
// (even if the "match" or "if let" is used for declaration)
710-
if let ExprKind::Loop(block, _, LoopSource::Loop, _) = expr.kind {
756+
if let ExprKind::Loop(block, label, LoopSource::Loop, _) = expr.kind {
711757
// also check for empty `loop {}` statements, skipping those in #[panic_handler]
712758
empty_loop::check(cx, expr, block);
713759
while_let_loop::check(cx, expr, block);
760+
if !self.in_never_return_fn {
761+
infinite_loops::check(cx, expr, block, label);
762+
}
714763
}
715764

716765
while_let_on_iterator::check(cx, expr);
@@ -722,6 +771,24 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
722771
}
723772
}
724773

774+
fn check_fn(
775+
&mut self,
776+
_: &LateContext<'tcx>,
777+
_: hir::intravisit::FnKind<'tcx>,
778+
decl: &'tcx hir::FnDecl<'tcx>,
779+
_: &'tcx hir::Body<'tcx>,
780+
_: Span,
781+
_: rustc_span::def_id::LocalDefId,
782+
) {
783+
self.in_never_return_fn = matches!(
784+
decl.output,
785+
hir::FnRetTy::Return(hir::Ty {
786+
kind: hir::TyKind::Never,
787+
..
788+
})
789+
);
790+
}
791+
725792
extract_msrv_attr!(LateContext);
726793
}
727794

0 commit comments

Comments
 (0)