Skip to content

Commit f4b0354

Browse files
committed
Fix false positive of needless_range_loop when meeting multidimensional array
1 parent 3e86f05 commit f4b0354

File tree

3 files changed

+88
-10
lines changed

3 files changed

+88
-10
lines changed

clippy_lints/src/loops/needless_range_loop.rs

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -252,13 +252,31 @@ struct VarVisitor<'a, 'tcx> {
252252
}
253253

254254
impl<'tcx> VarVisitor<'_, 'tcx> {
255-
fn check(&mut self, idx: &'tcx Expr<'_>, seqexpr: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) -> bool {
256-
let index_used_directly = matches!(idx.kind, ExprKind::Path(_));
255+
fn check<'a>(&mut self, idxs: &mut Vec<&'tcx Expr<'a>>, seqexpr: &'tcx Expr<'a>, expr: &'tcx Expr<'_>) -> bool {
256+
// Handle multi-dimensional array
257+
if let ExprKind::Index(seqexpr, idx, _) = seqexpr.kind {
258+
idxs.push(idx);
259+
// Recursive call
260+
return self.check(idxs, seqexpr, expr);
261+
}
262+
263+
// Ensure idx contains the target index; otherwise, it's meaningless.
264+
let used_idxs: Vec<_> = idxs
265+
.iter()
266+
.filter(|&&idx| is_local_used(self.cx, idx, self.var))
267+
.collect();
268+
269+
match used_idxs.len() {
270+
0 => return true,
271+
n if n > 1 => self.nonindex = true, // Optimize code like `a[i][i]`
272+
_ => {},
273+
}
274+
275+
let index_used_directly = used_idxs.iter().all(|idx| matches!(idx.kind, ExprKind::Path(_)));
257276
if let ExprKind::Path(ref seqpath) = seqexpr.kind
258277
// the indexed container is referenced by a name
259278
&& let QPath::Resolved(None, seqvar) = *seqpath
260279
&& seqvar.segments.len() == 1
261-
&& is_local_used(self.cx, idx, self.var)
262280
{
263281
if self.prefer_mutable {
264282
self.indexed_mut.insert(seqvar.segments[0].ident.name);
@@ -320,16 +338,20 @@ impl<'tcx> Visitor<'tcx> for VarVisitor<'_, 'tcx> {
320338
.and_then(|def_id| self.cx.tcx.trait_of_assoc(def_id))
321339
&& ((meth.ident.name == sym::index && self.cx.tcx.lang_items().index_trait() == Some(trait_id))
322340
|| (meth.ident.name == sym::index_mut && self.cx.tcx.lang_items().index_mut_trait() == Some(trait_id)))
323-
&& !self.check(args_1, args_0, expr)
324341
{
325-
return;
342+
let mut idxs: Vec<&Expr<'_>> = vec![args_1];
343+
if !self.check(&mut idxs, args_0, expr) {
344+
return;
345+
}
326346
}
327347

328-
if let ExprKind::Index(seqexpr, idx, _) = expr.kind
348+
if let ExprKind::Index(seqexpr, idx, _) = expr.kind {
349+
// Only handle the top `Index` expr
350+
let mut idxs: Vec<&Expr<'_>> = vec![idx];
329351
// an index op
330-
&& !self.check(idx, seqexpr, expr)
331-
{
332-
return;
352+
if !self.check(&mut idxs, seqexpr, expr) {
353+
return;
354+
}
333355
}
334356

335357
if let ExprKind::Path(QPath::Resolved(None, path)) = expr.kind

tests/ui/needless_range_loop.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,3 +210,35 @@ fn needless_loop() {
210210
black_box([1, 2, 3, 4, 5, 6, 7, 8][i]);
211211
}
212212
}
213+
214+
fn issue_15068() {
215+
let a = vec![vec![0u8; MAX_LEN]; MAX_LEN];
216+
let b = vec![0u8; MAX_LEN];
217+
218+
for i in 0..MAX_LEN {
219+
// no error
220+
let _ = a[0][i];
221+
let _ = b[i];
222+
}
223+
224+
for i in 0..MAX_LEN {
225+
// no error
226+
let _ = a[i][0];
227+
let _ = b[i];
228+
}
229+
230+
for i in 0..MAX_LEN {
231+
// no error
232+
let _ = a[i][b[i] as usize];
233+
}
234+
235+
for i in 0..MAX_LEN {
236+
//~^ needless_range_loop
237+
let _ = a[i][i];
238+
}
239+
240+
for i in 0..MAX_LEN {
241+
//~^ needless_range_loop
242+
let _ = a[0][i];
243+
}
244+
}

tests/ui/needless_range_loop.stderr

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,5 +168,29 @@ LL - for i in 0..vec.len() {
168168
LL + for (i, <item>) in vec.iter_mut().enumerate() {
169169
|
170170

171-
error: aborting due to 14 previous errors
171+
error: the loop variable `i` is used to index `a`
172+
--> tests/ui/needless_range_loop.rs:235:14
173+
|
174+
LL | for i in 0..MAX_LEN {
175+
| ^^^^^^^^^^
176+
|
177+
help: consider using an iterator and enumerate()
178+
|
179+
LL - for i in 0..MAX_LEN {
180+
LL + for (i, <item>) in a.iter().enumerate().take(MAX_LEN) {
181+
|
182+
183+
error: the loop variable `i` is only used to index `a`
184+
--> tests/ui/needless_range_loop.rs:240:14
185+
|
186+
LL | for i in 0..MAX_LEN {
187+
| ^^^^^^^^^^
188+
|
189+
help: consider using an iterator
190+
|
191+
LL - for i in 0..MAX_LEN {
192+
LL + for <item> in a.iter().take(MAX_LEN) {
193+
|
194+
195+
error: aborting due to 16 previous errors
172196

0 commit comments

Comments
 (0)