Fix a FP in explicit_counter_loop

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.
This commit is contained in:
rail 2020-06-10 14:05:32 +12:00
parent 29b12f22fc
commit 5e393c7747
2 changed files with 26 additions and 9 deletions

View file

@ -2134,7 +2134,7 @@ enum VarState {
DontWarn, 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> { struct IncrementVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>, // context reference cx: &'a LateContext<'tcx>, // context reference
states: FxHashMap<HirId, VarState>, // incremented variables states: FxHashMap<HirId, VarState>, // incremented variables
@ -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(def_id) = var_def_id(self.cx, expr) {
if let Some(parent) = get_parent_expr(self.cx, expr) { if let Some(parent) = get_parent_expr(self.cx, expr) {
let state = self.states.entry(def_id).or_insert(VarState::Initial); let state = self.states.entry(def_id).or_insert(VarState::Initial);
if *state == VarState::IncrOnce {
*state = VarState::DontWarn;
return;
}
match parent.kind { match parent.kind {
ExprKind::AssignOp(op, ref lhs, ref rhs) => { ExprKind::AssignOp(op, ref lhs, ref rhs) => {

View file

@ -38,54 +38,54 @@ mod issue_1219 {
let text = "banana"; let text = "banana";
let mut count = 0; let mut count = 0;
for ch in text.chars() { for ch in text.chars() {
println!("{}", count);
if ch == 'a' { if ch == 'a' {
continue; continue;
} }
count += 1; count += 1;
println!("{}", count);
} }
// should not trigger the lint because the count is conditional // should not trigger the lint because the count is conditional
let text = "banana"; let text = "banana";
let mut count = 0; let mut count = 0;
for ch in text.chars() { for ch in text.chars() {
println!("{}", count);
if ch == 'a' { if ch == 'a' {
count += 1; count += 1;
} }
println!("{}", count);
} }
// should trigger the lint because the count is not conditional // should trigger the lint because the count is not conditional
let text = "banana"; let text = "banana";
let mut count = 0; let mut count = 0;
for ch in text.chars() { for ch in text.chars() {
println!("{}", count);
count += 1; count += 1;
if ch == 'a' { if ch == 'a' {
continue; continue;
} }
println!("{}", count);
} }
// should trigger the lint because the count is not conditional // should trigger the lint because the count is not conditional
let text = "banana"; let text = "banana";
let mut count = 0; let mut count = 0;
for ch in text.chars() { for ch in text.chars() {
println!("{}", count);
count += 1; count += 1;
for i in 0..2 { for i in 0..2 {
let _ = 123; let _ = 123;
} }
println!("{}", count);
} }
// should not trigger the lint because the count is incremented multiple times // should not trigger the lint because the count is incremented multiple times
let text = "banana"; let text = "banana";
let mut count = 0; let mut count = 0;
for ch in text.chars() { for ch in text.chars() {
println!("{}", count);
count += 1; count += 1;
for i in 0..2 { for i in 0..2 {
count += 1; count += 1;
} }
println!("{}", count);
} }
} }
} }
@ -96,30 +96,30 @@ mod issue_3308 {
let mut skips = 0; let mut skips = 0;
let erasures = vec![]; let erasures = vec![];
for i in 0..10 { for i in 0..10 {
println!("{}", skips);
while erasures.contains(&(i + skips)) { while erasures.contains(&(i + skips)) {
skips += 1; skips += 1;
} }
println!("{}", skips);
} }
// should not trigger the lint because the count is incremented multiple times // should not trigger the lint because the count is incremented multiple times
let mut skips = 0; let mut skips = 0;
for i in 0..10 { for i in 0..10 {
println!("{}", skips);
let mut j = 0; let mut j = 0;
while j < 5 { while j < 5 {
skips += 1; skips += 1;
j += 1; j += 1;
} }
println!("{}", skips);
} }
// should not trigger the lint because the count is incremented multiple times // should not trigger the lint because the count is incremented multiple times
let mut skips = 0; let mut skips = 0;
for i in 0..10 { for i in 0..10 {
println!("{}", skips);
for j in 0..5 { for j in 0..5 {
skips += 1; skips += 1;
} }
println!("{}", skips);
} }
} }
} }
@ -145,3 +145,16 @@ mod issue_4732 {
let _closure = || println!("index: {}", index); 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);
}
}
}