Skip to content

Commit afb5ebb

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

File tree

3 files changed

+87
-5
lines changed

3 files changed

+87
-5
lines changed

clippy_lints/src/loops/needless_range_loop.rs

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use clippy_utils::diagnostics::span_lint_and_then;
33
use clippy_utils::source::snippet;
44
use clippy_utils::ty::has_iter_method;
55
use clippy_utils::visitors::is_local_used;
6-
use clippy_utils::{SpanlessEq, contains_name, higher, is_integer_const, sugg};
6+
use clippy_utils::{SpanlessEq, contains_name, higher, is_integer_const, peel_hir_expr_while, sugg};
77
use rustc_ast::ast;
88
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
99
use rustc_errors::Applicability;
@@ -253,12 +253,38 @@ struct VarVisitor<'a, 'tcx> {
253253

254254
impl<'tcx> VarVisitor<'_, 'tcx> {
255255
fn check(&mut self, idx: &'tcx Expr<'_>, seqexpr: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) -> bool {
256-
let index_used_directly = matches!(idx.kind, ExprKind::Path(_));
256+
let mut used_cnt = 0;
257+
// It is `true` if all indices are direct
258+
let mut index_used_directly = true;
259+
260+
// Handle initial index
261+
if is_local_used(self.cx, idx, self.var) {
262+
used_cnt += 1;
263+
index_used_directly &= matches!(idx.kind, ExprKind::Path(_));
264+
}
265+
// Handle nested indices
266+
let seqexpr = peel_hir_expr_while(seqexpr, |e| {
267+
if let ExprKind::Index(e, idx, _) = e.kind {
268+
if is_local_used(self.cx, idx, self.var) {
269+
used_cnt += 1;
270+
index_used_directly &= matches!(idx.kind, ExprKind::Path(_));
271+
}
272+
Some(e)
273+
} else {
274+
None
275+
}
276+
});
277+
278+
match used_cnt {
279+
0 => return true,
280+
n if n > 1 => self.nonindex = true, // Optimize code like `a[i][i]`
281+
_ => {},
282+
}
283+
257284
if let ExprKind::Path(ref seqpath) = seqexpr.kind
258285
// the indexed container is referenced by a name
259286
&& let QPath::Resolved(None, seqvar) = *seqpath
260287
&& seqvar.segments.len() == 1
261-
&& is_local_used(self.cx, idx, self.var)
262288
{
263289
if self.prefer_mutable {
264290
self.indexed_mut.insert(seqvar.segments[0].ident.name);
@@ -320,7 +346,7 @@ impl<'tcx> Visitor<'tcx> for VarVisitor<'_, 'tcx> {
320346
.and_then(|def_id| self.cx.tcx.trait_of_assoc(def_id))
321347
&& ((meth.ident.name == sym::index && self.cx.tcx.lang_items().index_trait() == Some(trait_id))
322348
|| (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)
349+
&& !self.check(args_1, args_0, expr)
324350
{
325351
return;
326352
}

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)