Skip to content

Commit 9520cba

Browse files
nahuakangY-Nak
nahuakang
authored and
Y-Nak
committed
Add check_infinite_loop to its own module
1 parent 1e5e541 commit 9520cba

File tree

2 files changed

+143
-132
lines changed

2 files changed

+143
-132
lines changed

‎clippy_lints/src/loops/infinite_loop.rs

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
use crate::consts::constant;
2+
use crate::utils::span_lint_and_then;
3+
use crate::utils::usage::mutated_variables;
4+
use if_chain::if_chain;
5+
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
6+
use rustc_hir::def::{DefKind, Res};
7+
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
8+
use rustc_hir::{def_id, Expr, ExprKind, HirId, QPath};
9+
use rustc_lint::LateContext;
10+
use rustc_middle::hir::map::Map;
11+
use std::iter::Iterator;
12+
13+
pub(super) fn check_infinite_loop<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) {
14+
if constant(cx, cx.typeck_results(), cond).is_some() {
15+
// A pure constant condition (e.g., `while false`) is not linted.
16+
return;
17+
}
18+
19+
let mut var_visitor = VarCollectorVisitor {
20+
cx,
21+
ids: FxHashSet::default(),
22+
def_ids: FxHashMap::default(),
23+
skip: false,
24+
};
25+
var_visitor.visit_expr(cond);
26+
if var_visitor.skip {
27+
return;
28+
}
29+
let used_in_condition = &var_visitor.ids;
30+
let no_cond_variable_mutated = if let Some(used_mutably) = mutated_variables(expr, cx) {
31+
used_in_condition.is_disjoint(&used_mutably)
32+
} else {
33+
return;
34+
};
35+
let mutable_static_in_cond = var_visitor.def_ids.iter().any(|(_, v)| *v);
36+
37+
let mut has_break_or_return_visitor = HasBreakOrReturnVisitor {
38+
has_break_or_return: false,
39+
};
40+
has_break_or_return_visitor.visit_expr(expr);
41+
let has_break_or_return = has_break_or_return_visitor.has_break_or_return;
42+
43+
if no_cond_variable_mutated && !mutable_static_in_cond {
44+
span_lint_and_then(
45+
cx,
46+
super::WHILE_IMMUTABLE_CONDITION,
47+
cond.span,
48+
"variables in the condition are not mutated in the loop body",
49+
|diag| {
50+
diag.note("this may lead to an infinite or to a never running loop");
51+
52+
if has_break_or_return {
53+
diag.note("this loop contains `return`s or `break`s");
54+
diag.help("rewrite it as `if cond { loop { } }`");
55+
}
56+
},
57+
);
58+
}
59+
}
60+
61+
struct HasBreakOrReturnVisitor {
62+
has_break_or_return: bool,
63+
}
64+
65+
impl<'tcx> Visitor<'tcx> for HasBreakOrReturnVisitor {
66+
type Map = Map<'tcx>;
67+
68+
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
69+
if self.has_break_or_return {
70+
return;
71+
}
72+
73+
match expr.kind {
74+
ExprKind::Ret(_) | ExprKind::Break(_, _) => {
75+
self.has_break_or_return = true;
76+
return;
77+
},
78+
_ => {},
79+
}
80+
81+
walk_expr(self, expr);
82+
}
83+
84+
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
85+
NestedVisitorMap::None
86+
}
87+
}
88+
89+
/// Collects the set of variables in an expression
90+
/// Stops analysis if a function call is found
91+
/// Note: In some cases such as `self`, there are no mutable annotation,
92+
/// All variables definition IDs are collected
93+
struct VarCollectorVisitor<'a, 'tcx> {
94+
cx: &'a LateContext<'tcx>,
95+
ids: FxHashSet<HirId>,
96+
def_ids: FxHashMap<def_id::DefId, bool>,
97+
skip: bool,
98+
}
99+
100+
impl<'a, 'tcx> VarCollectorVisitor<'a, 'tcx> {
101+
fn insert_def_id(&mut self, ex: &'tcx Expr<'_>) {
102+
if_chain! {
103+
if let ExprKind::Path(ref qpath) = ex.kind;
104+
if let QPath::Resolved(None, _) = *qpath;
105+
let res = self.cx.qpath_res(qpath, ex.hir_id);
106+
then {
107+
match res {
108+
Res::Local(hir_id) => {
109+
self.ids.insert(hir_id);
110+
},
111+
Res::Def(DefKind::Static, def_id) => {
112+
let mutable = self.cx.tcx.is_mutable_static(def_id);
113+
self.def_ids.insert(def_id, mutable);
114+
},
115+
_ => {},
116+
}
117+
}
118+
}
119+
}
120+
}
121+
122+
impl<'a, 'tcx> Visitor<'tcx> for VarCollectorVisitor<'a, 'tcx> {
123+
type Map = Map<'tcx>;
124+
125+
fn visit_expr(&mut self, ex: &'tcx Expr<'_>) {
126+
match ex.kind {
127+
ExprKind::Path(_) => self.insert_def_id(ex),
128+
// If there is any function/method call… we just stop analysis
129+
ExprKind::Call(..) | ExprKind::MethodCall(..) => self.skip = true,
130+
131+
_ => walk_expr(self, ex),
132+
}
133+
}
134+
135+
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
136+
NestedVisitorMap::None
137+
}
138+
}

‎clippy_lints/src/loops/mod.rs

Lines changed: 5 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ mod for_loop_over_map_kv;
44
mod for_loop_range;
55
mod for_mut_range_bound;
66
mod for_single_element_loop;
7+
mod infinite_loop;
78
mod manual_flatten;
89
mod utils;
910

10-
use crate::consts::constant;
1111
use crate::utils::sugg::Sugg;
1212
use crate::utils::usage::mutated_variables;
1313
use crate::utils::{
@@ -18,13 +18,13 @@ use crate::utils::{
1818
};
1919
use if_chain::if_chain;
2020
use rustc_ast::ast;
21-
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
21+
use rustc_data_structures::fx::FxHashMap;
2222
use rustc_errors::Applicability;
2323
use rustc_hir::def::{DefKind, Res};
2424
use rustc_hir::intravisit::{walk_block, walk_expr, walk_pat, walk_stmt, NestedVisitorMap, Visitor};
2525
use rustc_hir::{
26-
def_id, BinOpKind, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, GenericArg, HirId, InlineAsmOperand,
27-
Local, LoopSource, MatchSource, Mutability, Node, Pat, PatKind, QPath, Stmt, StmtKind,
26+
BinOpKind, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, GenericArg, HirId, InlineAsmOperand, Local,
27+
LoopSource, MatchSource, Mutability, Node, Pat, PatKind, QPath, Stmt, StmtKind,
2828
};
2929
use rustc_lint::{LateContext, LateLintPass, LintContext};
3030
use rustc_middle::hir::map::Map;
@@ -683,7 +683,7 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
683683
}
684684

685685
if let Some((cond, body)) = higher::while_loop(&expr) {
686-
check_infinite_loop(cx, cond, body);
686+
infinite_loop::check_infinite_loop(cx, cond, body);
687687
}
688688

689689
check_needless_collect(expr, cx);
@@ -1895,133 +1895,6 @@ impl<'tcx> Visitor<'tcx> for LoopNestVisitor {
18951895
}
18961896
}
18971897

1898-
fn check_infinite_loop<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) {
1899-
if constant(cx, cx.typeck_results(), cond).is_some() {
1900-
// A pure constant condition (e.g., `while false`) is not linted.
1901-
return;
1902-
}
1903-
1904-
let mut var_visitor = VarCollectorVisitor {
1905-
cx,
1906-
ids: FxHashSet::default(),
1907-
def_ids: FxHashMap::default(),
1908-
skip: false,
1909-
};
1910-
var_visitor.visit_expr(cond);
1911-
if var_visitor.skip {
1912-
return;
1913-
}
1914-
let used_in_condition = &var_visitor.ids;
1915-
let no_cond_variable_mutated = if let Some(used_mutably) = mutated_variables(expr, cx) {
1916-
used_in_condition.is_disjoint(&used_mutably)
1917-
} else {
1918-
return;
1919-
};
1920-
let mutable_static_in_cond = var_visitor.def_ids.iter().any(|(_, v)| *v);
1921-
1922-
let mut has_break_or_return_visitor = HasBreakOrReturnVisitor {
1923-
has_break_or_return: false,
1924-
};
1925-
has_break_or_return_visitor.visit_expr(expr);
1926-
let has_break_or_return = has_break_or_return_visitor.has_break_or_return;
1927-
1928-
if no_cond_variable_mutated && !mutable_static_in_cond {
1929-
span_lint_and_then(
1930-
cx,
1931-
WHILE_IMMUTABLE_CONDITION,
1932-
cond.span,
1933-
"variables in the condition are not mutated in the loop body",
1934-
|diag| {
1935-
diag.note("this may lead to an infinite or to a never running loop");
1936-
1937-
if has_break_or_return {
1938-
diag.note("this loop contains `return`s or `break`s");
1939-
diag.help("rewrite it as `if cond { loop { } }`");
1940-
}
1941-
},
1942-
);
1943-
}
1944-
}
1945-
1946-
struct HasBreakOrReturnVisitor {
1947-
has_break_or_return: bool,
1948-
}
1949-
1950-
impl<'tcx> Visitor<'tcx> for HasBreakOrReturnVisitor {
1951-
type Map = Map<'tcx>;
1952-
1953-
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
1954-
if self.has_break_or_return {
1955-
return;
1956-
}
1957-
1958-
match expr.kind {
1959-
ExprKind::Ret(_) | ExprKind::Break(_, _) => {
1960-
self.has_break_or_return = true;
1961-
return;
1962-
},
1963-
_ => {},
1964-
}
1965-
1966-
walk_expr(self, expr);
1967-
}
1968-
1969-
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
1970-
NestedVisitorMap::None
1971-
}
1972-
}
1973-
1974-
/// Collects the set of variables in an expression
1975-
/// Stops analysis if a function call is found
1976-
/// Note: In some cases such as `self`, there are no mutable annotation,
1977-
/// All variables definition IDs are collected
1978-
struct VarCollectorVisitor<'a, 'tcx> {
1979-
cx: &'a LateContext<'tcx>,
1980-
ids: FxHashSet<HirId>,
1981-
def_ids: FxHashMap<def_id::DefId, bool>,
1982-
skip: bool,
1983-
}
1984-
1985-
impl<'a, 'tcx> VarCollectorVisitor<'a, 'tcx> {
1986-
fn insert_def_id(&mut self, ex: &'tcx Expr<'_>) {
1987-
if_chain! {
1988-
if let ExprKind::Path(ref qpath) = ex.kind;
1989-
if let QPath::Resolved(None, _) = *qpath;
1990-
let res = self.cx.qpath_res(qpath, ex.hir_id);
1991-
then {
1992-
match res {
1993-
Res::Local(hir_id) => {
1994-
self.ids.insert(hir_id);
1995-
},
1996-
Res::Def(DefKind::Static, def_id) => {
1997-
let mutable = self.cx.tcx.is_mutable_static(def_id);
1998-
self.def_ids.insert(def_id, mutable);
1999-
},
2000-
_ => {},
2001-
}
2002-
}
2003-
}
2004-
}
2005-
}
2006-
2007-
impl<'a, 'tcx> Visitor<'tcx> for VarCollectorVisitor<'a, 'tcx> {
2008-
type Map = Map<'tcx>;
2009-
2010-
fn visit_expr(&mut self, ex: &'tcx Expr<'_>) {
2011-
match ex.kind {
2012-
ExprKind::Path(_) => self.insert_def_id(ex),
2013-
// If there is any function/method call… we just stop analysis
2014-
ExprKind::Call(..) | ExprKind::MethodCall(..) => self.skip = true,
2015-
2016-
_ => walk_expr(self, ex),
2017-
}
2018-
}
2019-
2020-
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
2021-
NestedVisitorMap::None
2022-
}
2023-
}
2024-
20251898
const NEEDLESS_COLLECT_MSG: &str = "avoid using `collect()` when not needed";
20261899

20271900
fn check_needless_collect<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) {

0 commit comments

Comments
 (0)