Skip to content

Commit 52bd7bb

Browse files
committed
relax needless_range_loop so that it reports only direct indexing
1 parent a54baad commit 52bd7bb

File tree

1 file changed

+16
-4
lines changed

1 file changed

+16
-4
lines changed

clippy_lints/src/loops.rs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -925,14 +925,16 @@ fn check_for_loop_range<'a, 'tcx>(
925925
cx: cx,
926926
var: canonical_id,
927927
indexed: HashMap::new(),
928+
indexed_directly: HashMap::new(),
928929
referenced: HashSet::new(),
929930
nonindex: false,
930931
};
931932
walk_expr(&mut visitor, body);
932933

933-
// linting condition: we only indexed one variable
934-
if visitor.indexed.len() == 1 {
935-
let (indexed, indexed_extent) = visitor.indexed.into_iter().next().expect(
934+
// linting condition: we only indexed one variable, and indexed it directly
935+
// (`indexed_directly` is subset of `indexed`)
936+
if visitor.indexed.len() == 1 && visitor.indexed_directly.len() == 1 {
937+
let (indexed, indexed_extent) = visitor.indexed_directly.into_iter().next().expect(
936938
"already checked that we have exactly 1 element",
937939
);
938940

@@ -1481,6 +1483,9 @@ struct VarVisitor<'a, 'tcx: 'a> {
14811483
var: ast::NodeId,
14821484
/// indexed variables, the extend is `None` for global
14831485
indexed: HashMap<Name, Option<region::Scope>>,
1486+
/// subset of `indexed` of vars that are indexed directly: `v[i]`
1487+
/// this will not contain cases like `v[calc_index(i)]` or `v[(i + 4) % N]`
1488+
indexed_directly: HashMap<Name, Option<region::Scope>>,
14841489
/// Any names that are used outside an index operation.
14851490
/// Used to detect things like `&mut vec` used together with `vec[i]`
14861491
referenced: HashSet<Name>,
@@ -1499,7 +1504,8 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> {
14991504
let QPath::Resolved(None, ref seqvar) = *seqpath,
15001505
seqvar.segments.len() == 1,
15011506
], {
1502-
let index_used = same_var(self.cx, idx, self.var) || {
1507+
let index_used_directly = same_var(self.cx, idx, self.var);
1508+
let index_used = index_used_directly || {
15031509
let mut used_visitor = LocalUsedVisitor {
15041510
cx: self.cx,
15051511
local: self.var,
@@ -1519,10 +1525,16 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> {
15191525
let parent_def_id = self.cx.tcx.hir.local_def_id(parent_id);
15201526
let extent = self.cx.tcx.region_scope_tree(parent_def_id).var_scope(hir_id.local_id);
15211527
self.indexed.insert(seqvar.segments[0].name, Some(extent));
1528+
if index_used_directly {
1529+
self.indexed_directly.insert(seqvar.segments[0].name, Some(extent));
1530+
}
15221531
return; // no need to walk further *on the variable*
15231532
}
15241533
Def::Static(..) | Def::Const(..) => {
15251534
self.indexed.insert(seqvar.segments[0].name, None);
1535+
if index_used_directly {
1536+
self.indexed_directly.insert(seqvar.segments[0].name, None);
1537+
}
15261538
return; // no need to walk further *on the variable*
15271539
}
15281540
_ => (),

0 commit comments

Comments
 (0)