Skip to content

Commit 455d047

Browse files
nahuakangY-Nak
nahuakang
authored and
Y-Nak
committed
Refactor never loop to its own module
1 parent 5b870e1 commit 455d047

File tree

2 files changed

+176
-169
lines changed

2 files changed

+176
-169
lines changed

‎clippy_lints/src/loops/mod.rs

Lines changed: 4 additions & 169 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ mod infinite_loop;
88
mod manual_flatten;
99
mod manual_memcpy;
1010
mod needless_collect;
11+
mod never_loop;
1112
mod same_item_push;
1213
mod utils;
1314

@@ -16,21 +17,18 @@ use crate::utils::usage::mutated_variables;
1617
use crate::utils::{
1718
get_enclosing_block, get_trait_def_id, higher, implements_trait, is_in_panic_handler, is_no_std_crate,
1819
is_refutable, last_path_segment, match_trait_method, path_to_local, path_to_local_id, paths,
19-
snippet_with_applicability, span_lint, span_lint_and_help, span_lint_and_sugg, sugg,
20+
snippet_with_applicability, span_lint_and_help, span_lint_and_sugg, sugg,
2021
};
2122
use if_chain::if_chain;
2223
use rustc_errors::Applicability;
2324
use rustc_hir::intravisit::{walk_block, walk_expr, NestedVisitorMap, Visitor};
24-
use rustc_hir::{
25-
Block, Expr, ExprKind, HirId, InlineAsmOperand, LoopSource, MatchSource, Node, Pat, PatKind, Stmt, StmtKind,
26-
};
25+
use rustc_hir::{Block, Expr, ExprKind, HirId, LoopSource, MatchSource, Node, Pat, PatKind, StmtKind};
2726
use rustc_lint::{LateContext, LateLintPass, LintContext};
2827
use rustc_middle::hir::map::Map;
2928
use rustc_middle::lint::in_external_macro;
3029
use rustc_session::{declare_lint_pass, declare_tool_lint};
3130
use rustc_span::source_map::Span;
3231
use rustc_span::symbol::sym;
33-
use std::iter::{once, Iterator};
3432
use utils::{
3533
get_span_of_entire_for_loop, make_iterator_snippet, IncrementVisitor, InitializeVisitor, LoopNestVisitor, Nesting,
3634
};
@@ -567,12 +565,7 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
567565
}
568566

569567
// check for never_loop
570-
if let ExprKind::Loop(ref block, _, _, _) = expr.kind {
571-
match never_loop_block(block, expr.hir_id) {
572-
NeverLoopResult::AlwaysBreak => span_lint(cx, NEVER_LOOP, expr.span, "this loop never actually loops"),
573-
NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (),
574-
}
575-
}
568+
never_loop::check_never_loop(cx, expr);
576569

577570
// check for `loop { if let {} else break }` that could be `while let`
578571
// (also matches an explicit "match" instead of "if let")
@@ -689,164 +682,6 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
689682
}
690683
}
691684

692-
enum NeverLoopResult {
693-
// A break/return always get triggered but not necessarily for the main loop.
694-
AlwaysBreak,
695-
// A continue may occur for the main loop.
696-
MayContinueMainLoop,
697-
Otherwise,
698-
}
699-
700-
#[must_use]
701-
fn absorb_break(arg: &NeverLoopResult) -> NeverLoopResult {
702-
match *arg {
703-
NeverLoopResult::AlwaysBreak | NeverLoopResult::Otherwise => NeverLoopResult::Otherwise,
704-
NeverLoopResult::MayContinueMainLoop => NeverLoopResult::MayContinueMainLoop,
705-
}
706-
}
707-
708-
// Combine two results for parts that are called in order.
709-
#[must_use]
710-
fn combine_seq(first: NeverLoopResult, second: NeverLoopResult) -> NeverLoopResult {
711-
match first {
712-
NeverLoopResult::AlwaysBreak | NeverLoopResult::MayContinueMainLoop => first,
713-
NeverLoopResult::Otherwise => second,
714-
}
715-
}
716-
717-
// Combine two results where both parts are called but not necessarily in order.
718-
#[must_use]
719-
fn combine_both(left: NeverLoopResult, right: NeverLoopResult) -> NeverLoopResult {
720-
match (left, right) {
721-
(NeverLoopResult::MayContinueMainLoop, _) | (_, NeverLoopResult::MayContinueMainLoop) => {
722-
NeverLoopResult::MayContinueMainLoop
723-
},
724-
(NeverLoopResult::AlwaysBreak, _) | (_, NeverLoopResult::AlwaysBreak) => NeverLoopResult::AlwaysBreak,
725-
(NeverLoopResult::Otherwise, NeverLoopResult::Otherwise) => NeverLoopResult::Otherwise,
726-
}
727-
}
728-
729-
// Combine two results where only one of the part may have been executed.
730-
#[must_use]
731-
fn combine_branches(b1: NeverLoopResult, b2: NeverLoopResult) -> NeverLoopResult {
732-
match (b1, b2) {
733-
(NeverLoopResult::AlwaysBreak, NeverLoopResult::AlwaysBreak) => NeverLoopResult::AlwaysBreak,
734-
(NeverLoopResult::MayContinueMainLoop, _) | (_, NeverLoopResult::MayContinueMainLoop) => {
735-
NeverLoopResult::MayContinueMainLoop
736-
},
737-
(NeverLoopResult::Otherwise, _) | (_, NeverLoopResult::Otherwise) => NeverLoopResult::Otherwise,
738-
}
739-
}
740-
741-
fn never_loop_block(block: &Block<'_>, main_loop_id: HirId) -> NeverLoopResult {
742-
let stmts = block.stmts.iter().map(stmt_to_expr);
743-
let expr = once(block.expr.as_deref());
744-
let mut iter = stmts.chain(expr).flatten();
745-
never_loop_expr_seq(&mut iter, main_loop_id)
746-
}
747-
748-
fn stmt_to_expr<'tcx>(stmt: &Stmt<'tcx>) -> Option<&'tcx Expr<'tcx>> {
749-
match stmt.kind {
750-
StmtKind::Semi(ref e, ..) | StmtKind::Expr(ref e, ..) => Some(e),
751-
StmtKind::Local(ref local) => local.init.as_deref(),
752-
_ => None,
753-
}
754-
}
755-
756-
fn never_loop_expr(expr: &Expr<'_>, main_loop_id: HirId) -> NeverLoopResult {
757-
match expr.kind {
758-
ExprKind::Box(ref e)
759-
| ExprKind::Unary(_, ref e)
760-
| ExprKind::Cast(ref e, _)
761-
| ExprKind::Type(ref e, _)
762-
| ExprKind::Field(ref e, _)
763-
| ExprKind::AddrOf(_, _, ref e)
764-
| ExprKind::Struct(_, _, Some(ref e))
765-
| ExprKind::Repeat(ref e, _)
766-
| ExprKind::DropTemps(ref e) => never_loop_expr(e, main_loop_id),
767-
ExprKind::Array(ref es) | ExprKind::MethodCall(_, _, ref es, _) | ExprKind::Tup(ref es) => {
768-
never_loop_expr_all(&mut es.iter(), main_loop_id)
769-
},
770-
ExprKind::Call(ref e, ref es) => never_loop_expr_all(&mut once(&**e).chain(es.iter()), main_loop_id),
771-
ExprKind::Binary(_, ref e1, ref e2)
772-
| ExprKind::Assign(ref e1, ref e2, _)
773-
| ExprKind::AssignOp(_, ref e1, ref e2)
774-
| ExprKind::Index(ref e1, ref e2) => never_loop_expr_all(&mut [&**e1, &**e2].iter().cloned(), main_loop_id),
775-
ExprKind::Loop(ref b, _, _, _) => {
776-
// Break can come from the inner loop so remove them.
777-
absorb_break(&never_loop_block(b, main_loop_id))
778-
},
779-
ExprKind::If(ref e, ref e2, ref e3) => {
780-
let e1 = never_loop_expr(e, main_loop_id);
781-
let e2 = never_loop_expr(e2, main_loop_id);
782-
let e3 = e3
783-
.as_ref()
784-
.map_or(NeverLoopResult::Otherwise, |e| never_loop_expr(e, main_loop_id));
785-
combine_seq(e1, combine_branches(e2, e3))
786-
},
787-
ExprKind::Match(ref e, ref arms, _) => {
788-
let e = never_loop_expr(e, main_loop_id);
789-
if arms.is_empty() {
790-
e
791-
} else {
792-
let arms = never_loop_expr_branch(&mut arms.iter().map(|a| &*a.body), main_loop_id);
793-
combine_seq(e, arms)
794-
}
795-
},
796-
ExprKind::Block(ref b, _) => never_loop_block(b, main_loop_id),
797-
ExprKind::Continue(d) => {
798-
let id = d
799-
.target_id
800-
.expect("target ID can only be missing in the presence of compilation errors");
801-
if id == main_loop_id {
802-
NeverLoopResult::MayContinueMainLoop
803-
} else {
804-
NeverLoopResult::AlwaysBreak
805-
}
806-
},
807-
ExprKind::Break(_, ref e) | ExprKind::Ret(ref e) => e.as_ref().map_or(NeverLoopResult::AlwaysBreak, |e| {
808-
combine_seq(never_loop_expr(e, main_loop_id), NeverLoopResult::AlwaysBreak)
809-
}),
810-
ExprKind::InlineAsm(ref asm) => asm
811-
.operands
812-
.iter()
813-
.map(|(o, _)| match o {
814-
InlineAsmOperand::In { expr, .. }
815-
| InlineAsmOperand::InOut { expr, .. }
816-
| InlineAsmOperand::Const { expr }
817-
| InlineAsmOperand::Sym { expr } => never_loop_expr(expr, main_loop_id),
818-
InlineAsmOperand::Out { expr, .. } => never_loop_expr_all(&mut expr.iter(), main_loop_id),
819-
InlineAsmOperand::SplitInOut { in_expr, out_expr, .. } => {
820-
never_loop_expr_all(&mut once(in_expr).chain(out_expr.iter()), main_loop_id)
821-
},
822-
})
823-
.fold(NeverLoopResult::Otherwise, combine_both),
824-
ExprKind::Struct(_, _, None)
825-
| ExprKind::Yield(_, _)
826-
| ExprKind::Closure(_, _, _, _, _)
827-
| ExprKind::LlvmInlineAsm(_)
828-
| ExprKind::Path(_)
829-
| ExprKind::ConstBlock(_)
830-
| ExprKind::Lit(_)
831-
| ExprKind::Err => NeverLoopResult::Otherwise,
832-
}
833-
}
834-
835-
fn never_loop_expr_seq<'a, T: Iterator<Item = &'a Expr<'a>>>(es: &mut T, main_loop_id: HirId) -> NeverLoopResult {
836-
es.map(|e| never_loop_expr(e, main_loop_id))
837-
.fold(NeverLoopResult::Otherwise, combine_seq)
838-
}
839-
840-
fn never_loop_expr_all<'a, T: Iterator<Item = &'a Expr<'a>>>(es: &mut T, main_loop_id: HirId) -> NeverLoopResult {
841-
es.map(|e| never_loop_expr(e, main_loop_id))
842-
.fold(NeverLoopResult::Otherwise, combine_both)
843-
}
844-
845-
fn never_loop_expr_branch<'a, T: Iterator<Item = &'a Expr<'a>>>(e: &mut T, main_loop_id: HirId) -> NeverLoopResult {
846-
e.map(|e| never_loop_expr(e, main_loop_id))
847-
.fold(NeverLoopResult::AlwaysBreak, combine_branches)
848-
}
849-
850685
fn check_for_loop<'tcx>(
851686
cx: &LateContext<'tcx>,
852687
pat: &'tcx Pat<'_>,

‎clippy_lints/src/loops/never_loop.rs

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
use super::NEVER_LOOP;
2+
use crate::utils::span_lint;
3+
use rustc_hir::{Block, Expr, ExprKind, HirId, InlineAsmOperand, Stmt, StmtKind};
4+
use rustc_lint::LateContext;
5+
use std::iter::{once, Iterator};
6+
7+
pub(super) fn check_never_loop(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
8+
if let ExprKind::Loop(ref block, _, _, _) = expr.kind {
9+
match never_loop_block(block, expr.hir_id) {
10+
NeverLoopResult::AlwaysBreak => span_lint(cx, NEVER_LOOP, expr.span, "this loop never actually loops"),
11+
NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (),
12+
}
13+
}
14+
}
15+
16+
enum NeverLoopResult {
17+
// A break/return always get triggered but not necessarily for the main loop.
18+
AlwaysBreak,
19+
// A continue may occur for the main loop.
20+
MayContinueMainLoop,
21+
Otherwise,
22+
}
23+
24+
#[must_use]
25+
fn absorb_break(arg: &NeverLoopResult) -> NeverLoopResult {
26+
match *arg {
27+
NeverLoopResult::AlwaysBreak | NeverLoopResult::Otherwise => NeverLoopResult::Otherwise,
28+
NeverLoopResult::MayContinueMainLoop => NeverLoopResult::MayContinueMainLoop,
29+
}
30+
}
31+
32+
// Combine two results for parts that are called in order.
33+
#[must_use]
34+
fn combine_seq(first: NeverLoopResult, second: NeverLoopResult) -> NeverLoopResult {
35+
match first {
36+
NeverLoopResult::AlwaysBreak | NeverLoopResult::MayContinueMainLoop => first,
37+
NeverLoopResult::Otherwise => second,
38+
}
39+
}
40+
41+
// Combine two results where both parts are called but not necessarily in order.
42+
#[must_use]
43+
fn combine_both(left: NeverLoopResult, right: NeverLoopResult) -> NeverLoopResult {
44+
match (left, right) {
45+
(NeverLoopResult::MayContinueMainLoop, _) | (_, NeverLoopResult::MayContinueMainLoop) => {
46+
NeverLoopResult::MayContinueMainLoop
47+
},
48+
(NeverLoopResult::AlwaysBreak, _) | (_, NeverLoopResult::AlwaysBreak) => NeverLoopResult::AlwaysBreak,
49+
(NeverLoopResult::Otherwise, NeverLoopResult::Otherwise) => NeverLoopResult::Otherwise,
50+
}
51+
}
52+
53+
// Combine two results where only one of the part may have been executed.
54+
#[must_use]
55+
fn combine_branches(b1: NeverLoopResult, b2: NeverLoopResult) -> NeverLoopResult {
56+
match (b1, b2) {
57+
(NeverLoopResult::AlwaysBreak, NeverLoopResult::AlwaysBreak) => NeverLoopResult::AlwaysBreak,
58+
(NeverLoopResult::MayContinueMainLoop, _) | (_, NeverLoopResult::MayContinueMainLoop) => {
59+
NeverLoopResult::MayContinueMainLoop
60+
},
61+
(NeverLoopResult::Otherwise, _) | (_, NeverLoopResult::Otherwise) => NeverLoopResult::Otherwise,
62+
}
63+
}
64+
65+
fn never_loop_block(block: &Block<'_>, main_loop_id: HirId) -> NeverLoopResult {
66+
let stmts = block.stmts.iter().map(stmt_to_expr);
67+
let expr = once(block.expr.as_deref());
68+
let mut iter = stmts.chain(expr).flatten();
69+
never_loop_expr_seq(&mut iter, main_loop_id)
70+
}
71+
72+
fn never_loop_expr_seq<'a, T: Iterator<Item = &'a Expr<'a>>>(es: &mut T, main_loop_id: HirId) -> NeverLoopResult {
73+
es.map(|e| never_loop_expr(e, main_loop_id))
74+
.fold(NeverLoopResult::Otherwise, combine_seq)
75+
}
76+
77+
fn stmt_to_expr<'tcx>(stmt: &Stmt<'tcx>) -> Option<&'tcx Expr<'tcx>> {
78+
match stmt.kind {
79+
StmtKind::Semi(ref e, ..) | StmtKind::Expr(ref e, ..) => Some(e),
80+
StmtKind::Local(ref local) => local.init.as_deref(),
81+
_ => None,
82+
}
83+
}
84+
85+
fn never_loop_expr(expr: &Expr<'_>, main_loop_id: HirId) -> NeverLoopResult {
86+
match expr.kind {
87+
ExprKind::Box(ref e)
88+
| ExprKind::Unary(_, ref e)
89+
| ExprKind::Cast(ref e, _)
90+
| ExprKind::Type(ref e, _)
91+
| ExprKind::Field(ref e, _)
92+
| ExprKind::AddrOf(_, _, ref e)
93+
| ExprKind::Struct(_, _, Some(ref e))
94+
| ExprKind::Repeat(ref e, _)
95+
| ExprKind::DropTemps(ref e) => never_loop_expr(e, main_loop_id),
96+
ExprKind::Array(ref es) | ExprKind::MethodCall(_, _, ref es, _) | ExprKind::Tup(ref es) => {
97+
never_loop_expr_all(&mut es.iter(), main_loop_id)
98+
},
99+
ExprKind::Call(ref e, ref es) => never_loop_expr_all(&mut once(&**e).chain(es.iter()), main_loop_id),
100+
ExprKind::Binary(_, ref e1, ref e2)
101+
| ExprKind::Assign(ref e1, ref e2, _)
102+
| ExprKind::AssignOp(_, ref e1, ref e2)
103+
| ExprKind::Index(ref e1, ref e2) => never_loop_expr_all(&mut [&**e1, &**e2].iter().cloned(), main_loop_id),
104+
ExprKind::Loop(ref b, _, _, _) => {
105+
// Break can come from the inner loop so remove them.
106+
absorb_break(&never_loop_block(b, main_loop_id))
107+
},
108+
ExprKind::If(ref e, ref e2, ref e3) => {
109+
let e1 = never_loop_expr(e, main_loop_id);
110+
let e2 = never_loop_expr(e2, main_loop_id);
111+
let e3 = e3
112+
.as_ref()
113+
.map_or(NeverLoopResult::Otherwise, |e| never_loop_expr(e, main_loop_id));
114+
combine_seq(e1, combine_branches(e2, e3))
115+
},
116+
ExprKind::Match(ref e, ref arms, _) => {
117+
let e = never_loop_expr(e, main_loop_id);
118+
if arms.is_empty() {
119+
e
120+
} else {
121+
let arms = never_loop_expr_branch(&mut arms.iter().map(|a| &*a.body), main_loop_id);
122+
combine_seq(e, arms)
123+
}
124+
},
125+
ExprKind::Block(ref b, _) => never_loop_block(b, main_loop_id),
126+
ExprKind::Continue(d) => {
127+
let id = d
128+
.target_id
129+
.expect("target ID can only be missing in the presence of compilation errors");
130+
if id == main_loop_id {
131+
NeverLoopResult::MayContinueMainLoop
132+
} else {
133+
NeverLoopResult::AlwaysBreak
134+
}
135+
},
136+
ExprKind::Break(_, ref e) | ExprKind::Ret(ref e) => e.as_ref().map_or(NeverLoopResult::AlwaysBreak, |e| {
137+
combine_seq(never_loop_expr(e, main_loop_id), NeverLoopResult::AlwaysBreak)
138+
}),
139+
ExprKind::InlineAsm(ref asm) => asm
140+
.operands
141+
.iter()
142+
.map(|(o, _)| match o {
143+
InlineAsmOperand::In { expr, .. }
144+
| InlineAsmOperand::InOut { expr, .. }
145+
| InlineAsmOperand::Const { expr }
146+
| InlineAsmOperand::Sym { expr } => never_loop_expr(expr, main_loop_id),
147+
InlineAsmOperand::Out { expr, .. } => never_loop_expr_all(&mut expr.iter(), main_loop_id),
148+
InlineAsmOperand::SplitInOut { in_expr, out_expr, .. } => {
149+
never_loop_expr_all(&mut once(in_expr).chain(out_expr.iter()), main_loop_id)
150+
},
151+
})
152+
.fold(NeverLoopResult::Otherwise, combine_both),
153+
ExprKind::Struct(_, _, None)
154+
| ExprKind::Yield(_, _)
155+
| ExprKind::Closure(_, _, _, _, _)
156+
| ExprKind::LlvmInlineAsm(_)
157+
| ExprKind::Path(_)
158+
| ExprKind::ConstBlock(_)
159+
| ExprKind::Lit(_)
160+
| ExprKind::Err => NeverLoopResult::Otherwise,
161+
}
162+
}
163+
164+
fn never_loop_expr_all<'a, T: Iterator<Item = &'a Expr<'a>>>(es: &mut T, main_loop_id: HirId) -> NeverLoopResult {
165+
es.map(|e| never_loop_expr(e, main_loop_id))
166+
.fold(NeverLoopResult::Otherwise, combine_both)
167+
}
168+
169+
fn never_loop_expr_branch<'a, T: Iterator<Item = &'a Expr<'a>>>(e: &mut T, main_loop_id: HirId) -> NeverLoopResult {
170+
e.map(|e| never_loop_expr(e, main_loop_id))
171+
.fold(NeverLoopResult::AlwaysBreak, combine_branches)
172+
}

0 commit comments

Comments
 (0)