Skip to content

Commit f320b64

Browse files
authored
Fix explicit_counter_loop FP when the initializer is not integral (#16647)
Closes #16640 changelog: [`explicit_counter_loop`] fix FP when the initializer is not integral
2 parents e4de5a8 + 5218b85 commit f320b64

2 files changed

Lines changed: 80 additions & 60 deletions

File tree

clippy_lints/src/loops/explicit_counter_loop.rs

Lines changed: 65 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -28,72 +28,77 @@ pub(super) fn check<'tcx>(
2828

2929
// For each candidate, check the parent block to see if
3030
// it's initialized to zero at the start of the loop.
31-
if let Some(block) = get_enclosing_block(cx, expr.hir_id) {
32-
for id in increment_visitor.into_results() {
33-
let mut initialize_visitor = InitializeVisitor::new(cx, expr, id);
34-
walk_block(&mut initialize_visitor, block);
31+
let Some(block) = get_enclosing_block(cx, expr.hir_id) else {
32+
return;
33+
};
3534

36-
if let Some((name, ty, initializer)) = initialize_visitor.get_result() {
37-
let is_zero = is_integer_const(cx, initializer, 0);
38-
let mut applicability = Applicability::MaybeIncorrect;
39-
let span = expr.span.with_hi(arg.span.hi());
40-
let loop_label = label.map_or(String::new(), |l| format!("{}: ", l.ident.name));
35+
for id in increment_visitor.into_results() {
36+
let mut initialize_visitor = InitializeVisitor::new(cx, expr, id);
37+
walk_block(&mut initialize_visitor, block);
4138

42-
span_lint_and_then(
43-
cx,
44-
EXPLICIT_COUNTER_LOOP,
45-
span,
46-
format!("the variable `{name}` is used as a loop counter"),
47-
|diag| {
48-
let pat_snippet = snippet_with_applicability(cx, pat.span, "item", &mut applicability);
49-
let iter_snippet = make_iterator_snippet(cx, arg, &mut applicability);
50-
let int_name = match ty.map(Ty::kind) {
51-
Some(ty::Uint(UintTy::Usize)) | None => {
52-
if is_zero {
53-
diag.span_suggestion(
54-
span,
55-
"consider using",
56-
format!(
57-
"{loop_label}for ({name}, {pat_snippet}) in {iter_snippet}.enumerate()",
58-
),
59-
applicability,
60-
);
61-
return;
62-
}
63-
None
64-
},
65-
Some(ty::Int(int_ty)) => Some(int_ty.name_str()),
66-
Some(ty::Uint(uint_ty)) => Some(uint_ty.name_str()),
67-
_ => None,
68-
}
69-
.filter(|_| is_integer_literal_untyped(initializer));
70-
71-
let initializer = Sugg::hir_from_snippet(cx, initializer, |span| {
72-
let snippet = snippet_with_applicability(cx, span, "..", &mut applicability);
73-
if let Some(int_name) = int_name {
74-
return Cow::Owned(format!("{snippet}_{int_name}"));
75-
}
76-
snippet
77-
});
39+
let Some((name, ty, initializer)) = initialize_visitor.get_result() else {
40+
continue;
41+
};
42+
if !cx.typeck_results().expr_ty(initializer).is_integral() {
43+
continue;
44+
}
7845

79-
diag.span_suggestion(
80-
span,
81-
"consider using",
82-
format!(
83-
"{loop_label}for ({name}, {pat_snippet}) in ({}).zip({iter_snippet})",
84-
initializer.range(&EMPTY, RangeLimits::HalfOpen)
85-
),
86-
applicability,
87-
);
46+
let is_zero = is_integer_const(cx, initializer, 0);
47+
let mut applicability = Applicability::MaybeIncorrect;
48+
let span = expr.span.with_hi(arg.span.hi());
49+
let loop_label = label.map_or(String::new(), |l| format!("{}: ", l.ident.name));
8850

89-
if is_zero && let Some(int_name) = int_name {
90-
diag.note(format!(
91-
"`{name}` is of type `{int_name}`, making it ineligible for `Iterator::enumerate`"
92-
));
51+
span_lint_and_then(
52+
cx,
53+
EXPLICIT_COUNTER_LOOP,
54+
span,
55+
format!("the variable `{name}` is used as a loop counter"),
56+
|diag| {
57+
let pat_snippet = snippet_with_applicability(cx, pat.span, "item", &mut applicability);
58+
let iter_snippet = make_iterator_snippet(cx, arg, &mut applicability);
59+
let int_name = match ty.map(Ty::kind) {
60+
Some(ty::Uint(UintTy::Usize)) | None => {
61+
if is_zero {
62+
diag.span_suggestion(
63+
span,
64+
"consider using",
65+
format!("{loop_label}for ({name}, {pat_snippet}) in {iter_snippet}.enumerate()"),
66+
applicability,
67+
);
68+
return;
9369
}
70+
None
9471
},
72+
Some(ty::Int(int_ty)) => Some(int_ty.name_str()),
73+
Some(ty::Uint(uint_ty)) => Some(uint_ty.name_str()),
74+
_ => None,
75+
}
76+
.filter(|_| is_integer_literal_untyped(initializer));
77+
78+
let initializer = Sugg::hir_from_snippet(cx, initializer, |span| {
79+
let snippet = snippet_with_applicability(cx, span, "..", &mut applicability);
80+
if let Some(int_name) = int_name {
81+
return Cow::Owned(format!("{snippet}_{int_name}"));
82+
}
83+
snippet
84+
});
85+
86+
diag.span_suggestion(
87+
span,
88+
"consider using",
89+
format!(
90+
"{loop_label}for ({name}, {pat_snippet}) in ({}).zip({iter_snippet})",
91+
initializer.range(&EMPTY, RangeLimits::HalfOpen)
92+
),
93+
applicability,
9594
);
96-
}
97-
}
95+
96+
if is_zero && let Some(int_name) = int_name {
97+
diag.note(format!(
98+
"`{name}` is of type `{int_name}`, making it ineligible for `Iterator::enumerate`"
99+
));
100+
}
101+
},
102+
);
98103
}
99104
}

tests/ui/explicit_counter_loop.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,3 +317,18 @@ fn issue16612(v: Vec<u8>, s: i64) {
317317
j += 1;
318318
}
319319
}
320+
321+
fn issue16640(x: &[u8]) {
322+
struct Priority(u8);
323+
324+
impl core::ops::AddAssign<u8> for Priority {
325+
fn add_assign(&mut self, rhs: u8) {
326+
self.0 += rhs
327+
}
328+
}
329+
330+
let mut priority = Priority(1);
331+
for _val in x {
332+
priority += 1;
333+
}
334+
}

0 commit comments

Comments
 (0)