Skip to content

Commit b62319c

Browse files
committed
Auto merge of #10094 - EricWu2003:increment-visitor-fix, r=xFrednet
fix logic in IncrementVisitor There used to be a logical bug where IncrementVisitor would completely stop checking an expression/block after seeing a continue statement. I am a little unsure of whether my fix to `IncrementVisitor` is logically sound (I hope it makes sense). Let me know what you think, and thanks in advance for the review! fixes #10058 --- changelog: FP: [`explicit_counter_loop`]: No longer ignores counter changes after `continue` expressions [#10094](#10094) <!-- changelog_checked -->
2 parents 02f3959 + 97c12e0 commit b62319c

File tree

2 files changed

+33
-7
lines changed

2 files changed

+33
-7
lines changed

Diff for: clippy_lints/src/loops/utils.rs

+3-7
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ pub(super) struct IncrementVisitor<'a, 'tcx> {
2525
cx: &'a LateContext<'tcx>, // context reference
2626
states: HirIdMap<IncrementVisitorVarState>, // incremented variables
2727
depth: u32, // depth of conditional expressions
28-
done: bool,
2928
}
3029

3130
impl<'a, 'tcx> IncrementVisitor<'a, 'tcx> {
@@ -34,7 +33,6 @@ impl<'a, 'tcx> IncrementVisitor<'a, 'tcx> {
3433
cx,
3534
states: HirIdMap::default(),
3635
depth: 0,
37-
done: false,
3836
}
3937
}
4038

@@ -51,10 +49,6 @@ impl<'a, 'tcx> IncrementVisitor<'a, 'tcx> {
5149

5250
impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
5351
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
54-
if self.done {
55-
return;
56-
}
57-
5852
// If node is a variable
5953
if let Some(def_id) = path_to_local(expr) {
6054
if let Some(parent) = get_parent_expr(self.cx, expr) {
@@ -95,7 +89,9 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
9589
walk_expr(self, expr);
9690
self.depth -= 1;
9791
} else if let ExprKind::Continue(_) = expr.kind {
98-
self.done = true;
92+
// If we see a `continue` block, then we increment depth so that the IncrementVisitor
93+
// state will be set to DontWarn if we see the variable being modified anywhere afterwards.
94+
self.depth += 1;
9995
} else {
10096
walk_expr(self, expr);
10197
}

Diff for: tests/ui/explicit_counter_loop.rs

+30
Original file line numberDiff line numberDiff line change
@@ -189,3 +189,33 @@ mod issue_7920 {
189189
}
190190
}
191191
}
192+
193+
mod issue_10058 {
194+
pub fn test() {
195+
// should not lint since we are increasing counter potentially more than once in the loop
196+
let values = [0, 1, 0, 1, 1, 1, 0, 1, 0, 1];
197+
let mut counter = 0;
198+
for value in values {
199+
counter += 1;
200+
201+
if value == 0 {
202+
continue;
203+
}
204+
205+
counter += 1;
206+
}
207+
}
208+
209+
pub fn test2() {
210+
// should not lint since we are increasing counter potentially more than once in the loop
211+
let values = [0, 1, 0, 1, 1, 1, 0, 1, 0, 1];
212+
let mut counter = 0;
213+
for value in values {
214+
counter += 1;
215+
216+
if value != 0 {
217+
counter += 1;
218+
}
219+
}
220+
}
221+
}

0 commit comments

Comments
 (0)