Skip to content

Commit 720f19f

Browse files
committed
Implement detecting manual_memcpy with loop counters
1 parent b4b4da1 commit 720f19f

File tree

1 file changed

+66
-24
lines changed

1 file changed

+66
-24
lines changed

clippy_lints/src/loops.rs

Lines changed: 66 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -913,37 +913,62 @@ fn get_offset<'tcx>(
913913
}
914914
}
915915

916-
fn get_assignments<'tcx>(body: &'tcx Expr<'tcx>) -> impl Iterator<Item = Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>> {
917-
fn get_assignment<'tcx>(e: &'tcx Expr<'tcx>) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> {
918-
if let ExprKind::Assign(lhs, rhs, _) = e.kind {
919-
Some((lhs, rhs))
920-
} else {
921-
None
922-
}
916+
fn get_assignment<'tcx>(e: &'tcx Expr<'tcx>) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> {
917+
if let ExprKind::Assign(lhs, rhs, _) = e.kind {
918+
Some((lhs, rhs))
919+
} else {
920+
None
923921
}
922+
}
924923

925-
// This is one of few ways to return different iterators
926-
// derived from: https://stackoverflow.com/questions/29760668/conditionally-iterate-over-one-of-several-possible-iterators/52064434#52064434
927-
let mut iter_a = None;
928-
let mut iter_b = None;
924+
fn get_assignments<'a: 'c, 'tcx: 'c, 'c>(
925+
cx: &'a LateContext<'a, 'tcx>,
926+
stmts: &'tcx [Stmt<'tcx>],
927+
expr: Option<&'tcx Expr<'tcx>>,
928+
loop_counters: &'c [Start<'tcx>],
929+
) -> impl Iterator<Item = Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>> + 'c {
930+
stmts
931+
.iter()
932+
.filter_map(move |stmt| match stmt.kind {
933+
StmtKind::Local(..) | StmtKind::Item(..) => None,
934+
StmtKind::Expr(e) | StmtKind::Semi(e) => if_chain! {
935+
if let ExprKind::AssignOp(_, var, _) = e.kind;
936+
// skip StartKind::Range
937+
if loop_counters.iter().skip(1).any(|counter| Some(counter.id) == var_def_id(cx, var));
938+
then { None } else { Some(e) }
939+
},
940+
})
941+
.chain(expr.into_iter())
942+
.map(get_assignment)
943+
}
929944

930-
if let ExprKind::Block(b, _) = body.kind {
931-
let Block { stmts, expr, .. } = *b;
945+
fn get_loop_counters<'a, 'tcx>(
946+
cx: &'a LateContext<'a, 'tcx>,
947+
body: &'tcx Block<'tcx>,
948+
expr: &'tcx Expr<'_>,
949+
) -> Option<impl Iterator<Item = Start<'tcx>> + 'a> {
950+
// Look for variables that are incremented once per loop iteration.
951+
let mut increment_visitor = IncrementVisitor::new(cx);
952+
walk_block(&mut increment_visitor, body);
932953

933-
iter_a = stmts
934-
.iter()
935-
.filter_map(|stmt| match stmt.kind {
936-
StmtKind::Local(..) | StmtKind::Item(..) => None,
937-
StmtKind::Expr(e) | StmtKind::Semi(e) => Some(e),
954+
// For each candidate, check the parent block to see if
955+
// it's initialized to zero at the start of the loop.
956+
if let Some(block) = get_enclosing_block(&cx, expr.hir_id) {
957+
increment_visitor
958+
.into_results()
959+
.filter_map(move |var_id| {
960+
let mut initialize_visitor = InitializeVisitor::new(cx, expr, var_id);
961+
walk_block(&mut initialize_visitor, block);
962+
963+
initialize_visitor.get_result().map(|(_, initializer)| Start {
964+
id: var_id,
965+
kind: StartKind::Counter { initializer },
966+
})
938967
})
939-
.chain(expr.into_iter())
940-
.map(get_assignment)
941968
.into()
942969
} else {
943-
iter_b = Some(get_assignment(body))
970+
None
944971
}
945-
946-
iter_a.into_iter().flatten().chain(iter_b.into_iter())
947972
}
948973

949974
fn build_manual_memcpy_suggestion<'tcx>(
@@ -1047,9 +1072,26 @@ fn detect_manual_memcpy<'tcx>(
10471072
id: canonical_id,
10481073
kind: StartKind::Range,
10491074
}];
1075+
1076+
// This is one of few ways to return different iterators
1077+
// derived from: https://stackoverflow.com/questions/29760668/conditionally-iterate-over-one-of-several-possible-iterators/52064434#52064434
1078+
let mut iter_a = None;
1079+
let mut iter_b = None;
1080+
1081+
if let ExprKind::Block(block, _) = body.kind {
1082+
if let Some(loop_counters) = get_loop_counters(cx, block, expr) {
1083+
starts.extend(loop_counters);
1084+
}
1085+
iter_a = Some(get_assignments(cx, block.stmts, block.expr, &starts));
1086+
} else {
1087+
iter_b = Some(get_assignment(body));
1088+
}
1089+
10501090
// The only statements in the for loops can be indexed assignments from
10511091
// indexed retrievals.
1052-
let big_sugg = get_assignments(body)
1092+
let assignments = iter_a.into_iter().flatten().chain(iter_b.into_iter());
1093+
1094+
let big_sugg = assignments
10531095
.map(|o| {
10541096
o.and_then(|(lhs, rhs)| {
10551097
let rhs = fetch_cloned_expr(rhs);

0 commit comments

Comments
 (0)