Skip to content

Commit

Permalink
Auto merge of #6076 - rail-rain:fix_fp_explicit_counter_loop, r=matth…
Browse files Browse the repository at this point in the history
…iaskrgr

Fix a FP in `explicit_counter_loop`

Fixes #4677 and #6074

Fix a false positive in `explicit_counter_loop` where the loop counter is used after incremented, adjust the test so that counters are incremented at the end of the loop and add the test for this false positive.

---

changelog: Fix a false positive in `explicit_counter_loop` where the loop counter is used after incremented
  • Loading branch information
bors committed Sep 24, 2020
2 parents e636b88 + 5e393c7 commit 019c0d5
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 9 deletions.
6 changes: 5 additions & 1 deletion clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2134,7 +2134,7 @@ enum VarState {
DontWarn,
}

/// Scan a for loop for variables that are incremented exactly once.
/// Scan a for loop for variables that are incremented exactly once and not used after that.
struct IncrementVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>, // context reference
states: FxHashMap<HirId, VarState>, // incremented variables
Expand All @@ -2154,6 +2154,10 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
if let Some(def_id) = var_def_id(self.cx, expr) {
if let Some(parent) = get_parent_expr(self.cx, expr) {
let state = self.states.entry(def_id).or_insert(VarState::Initial);
if *state == VarState::IncrOnce {
*state = VarState::DontWarn;
return;
}

match parent.kind {
ExprKind::AssignOp(op, ref lhs, ref rhs) => {
Expand Down
29 changes: 21 additions & 8 deletions tests/ui/explicit_counter_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,54 +38,54 @@ mod issue_1219 {
let text = "banana";
let mut count = 0;
for ch in text.chars() {
println!("{}", count);
if ch == 'a' {
continue;
}
count += 1;
println!("{}", count);
}

// should not trigger the lint because the count is conditional
let text = "banana";
let mut count = 0;
for ch in text.chars() {
println!("{}", count);
if ch == 'a' {
count += 1;
}
println!("{}", count);
}

// should trigger the lint because the count is not conditional
let text = "banana";
let mut count = 0;
for ch in text.chars() {
println!("{}", count);
count += 1;
if ch == 'a' {
continue;
}
println!("{}", count);
}

// should trigger the lint because the count is not conditional
let text = "banana";
let mut count = 0;
for ch in text.chars() {
println!("{}", count);
count += 1;
for i in 0..2 {
let _ = 123;
}
println!("{}", count);
}

// should not trigger the lint because the count is incremented multiple times
let text = "banana";
let mut count = 0;
for ch in text.chars() {
println!("{}", count);
count += 1;
for i in 0..2 {
count += 1;
}
println!("{}", count);
}
}
}
Expand All @@ -96,30 +96,30 @@ mod issue_3308 {
let mut skips = 0;
let erasures = vec![];
for i in 0..10 {
println!("{}", skips);
while erasures.contains(&(i + skips)) {
skips += 1;
}
println!("{}", skips);
}

// should not trigger the lint because the count is incremented multiple times
let mut skips = 0;
for i in 0..10 {
println!("{}", skips);
let mut j = 0;
while j < 5 {
skips += 1;
j += 1;
}
println!("{}", skips);
}

// should not trigger the lint because the count is incremented multiple times
let mut skips = 0;
for i in 0..10 {
println!("{}", skips);
for j in 0..5 {
skips += 1;
}
println!("{}", skips);
}
}
}
Expand All @@ -145,3 +145,16 @@ mod issue_4732 {
let _closure = || println!("index: {}", index);
}
}

mod issue_4677 {
pub fn test() {
let slice = &[1, 2, 3];

// should not trigger the lint because the count is used after incremented
let mut count = 0;
for _i in slice {
count += 1;
println!("{}", count);
}
}
}

0 comments on commit 019c0d5

Please sign in to comment.