Skip to content

Commit

Permalink
fix: manual_memcpy wrong indexing for multi diemnsion arrays
Browse files Browse the repository at this point in the history
  • Loading branch information
granddaifuku authored Dec 25, 2023
1 parent 370615b commit 0337145
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 4 deletions.
17 changes: 17 additions & 0 deletions clippy_lints/src/loops/manual_memcpy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ pub(super) fn check<'tcx>(
let rhs = fetch_cloned_expr(rhs);
if let ExprKind::Index(base_left, idx_left, _) = lhs.kind
&& let ExprKind::Index(base_right, idx_right, _) = rhs.kind
&& is_index_suggestable(base_left) && is_index_suggestable(base_right)
&& let Some(ty) = get_slice_like_element_ty(cx, cx.typeck_results().expr_ty(base_left))
&& get_slice_like_element_ty(cx, cx.typeck_results().expr_ty(base_right)).is_some()
&& let Some((start_left, offset_left)) = get_details_from_idx(cx, idx_left, &starts)
Expand Down Expand Up @@ -486,3 +487,19 @@ fn is_array_length_equal_to_range(cx: &LateContext<'_>, start: &Expr<'_>, end: &
false
}
}

// Returns true if index is suggestable
// i.e. Returns false if patterns are Index(Path(_), Path(_)) or Index(Index(_, _, _), Path(_))
fn is_index_suggestable(expr: &Expr<'_>) -> bool {
if let ExprKind::Index(left_expr, right_expr, _) = expr.kind {
// Index(Path(_), Path(_), _) returns false
if matches!(left_expr.kind, ExprKind::Path(_)) {
return !matches!(right_expr.kind, ExprKind::Path(_));
}

// Nested Index pattern
return is_index_suggestable(left_expr) && !matches!(right_expr.kind, ExprKind::Path(_));
}

true
}
54 changes: 52 additions & 2 deletions tests/ui/manual_memcpy/without_loop_counters.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#![warn(clippy::needless_range_loop, clippy::manual_memcpy)]
#![allow(clippy::useless_vec)]
#![warn(clippy::manual_memcpy)]
#![allow(clippy::useless_vec, clippy::needless_range_loop)]
//@no-rustfix
const LOOP_OFFSET: usize = 5000;

Expand Down Expand Up @@ -158,6 +158,56 @@ pub fn manual_copy(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) {
//~^ ERROR: it looks like you're manually copying between slices
dst[i] = src[i];
}

// Don't trigger lint for following multi-dimension arrays
let src = [[0; 5]; 5];
for i in 0..5 {
dst[i] = src[i][i];
}
for i in 0..5 {
dst[i] = src[i][3];
}

let src = [0; 5];
let mut dst = [[0; 5]; 5];
for i in 0..5 {
dst[i][i] = src[i];
}

let src = [[[0; 5]; 5]; 5];
let mut dst = [0; 5];
for i in 0..5 {
dst[i] = src[i][i][i];
}
for i in 0..5 {
dst[i] = src[i][i][0];
}
for i in 0..5 {
dst[i] = src[i][0][i];
}
for i in 0..5 {
dst[i] = src[0][i][i];
}
for i in 0..5 {
dst[i] = src[0][i][1];
}
for i in 0..5 {
dst[i] = src[i][0][1];
}

// Trigger lint
let src = [[0; 5]; 5];
let mut dst = [0; 5];
for i in 0..5 {
//~^ ERROR: it looks like you're manually copying between slices
dst[i] = src[0][i];
}

let src = [[[0; 5]; 5]; 5];
for i in 0..5 {
//~^ ERROR: it looks like you're manually copying between slices
dst[i] = src[0][1][i];
}
}

#[warn(clippy::needless_range_loop, clippy::manual_memcpy)]
Expand Down
22 changes: 20 additions & 2 deletions tests/ui/manual_memcpy/without_loop_counters.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,31 @@ LL | | }
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src);`

error: it looks like you're manually copying between slices
--> $DIR/without_loop_counters.rs:165:5
--> $DIR/without_loop_counters.rs:201:5
|
LL | / for i in 0..5 {
LL | |
LL | | dst[i] = src[0][i];
LL | | }
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src[0]);`

error: it looks like you're manually copying between slices
--> $DIR/without_loop_counters.rs:207:5
|
LL | / for i in 0..5 {
LL | |
LL | | dst[i] = src[0][1][i];
LL | | }
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src[0][1]);`

error: it looks like you're manually copying between slices
--> $DIR/without_loop_counters.rs:215:5
|
LL | / for i in 0..src.len() {
LL | |
LL | | dst[i] = src[i].clone();
LL | | }
| |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..]);`

error: aborting due to 16 previous errors
error: aborting due to 18 previous errors

0 comments on commit 0337145

Please sign in to comment.