Skip to content

Commit a9d42e6

Browse files
committed
fix: reduce [manual_memcpy] indexing when array length is same to loop range
Format refactor: extract function to shrink function length fix: remove cmp to calculate range fix: replace if_chain with let chains
1 parent c3a6b37 commit a9d42e6

File tree

3 files changed

+90
-8
lines changed

3 files changed

+90
-8
lines changed

clippy_lints/src/loops/manual_memcpy.rs

+41-6
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,9 @@ fn build_manual_memcpy_suggestion<'tcx>(
178178
let dst_base_str = snippet(cx, dst.base.span, "???");
179179
let src_base_str = snippet(cx, src.base.span, "???");
180180

181-
let dst = if dst_offset == sugg::EMPTY && dst_limit == sugg::EMPTY {
181+
let dst = if (dst_offset == sugg::EMPTY && dst_limit == sugg::EMPTY)
182+
|| is_array_length_equal_to_range(cx, start, end, dst.base)
183+
{
182184
dst_base_str
183185
} else {
184186
format!("{dst_base_str}[{}..{}]", dst_offset.maybe_par(), dst_limit.maybe_par()).into()
@@ -190,11 +192,13 @@ fn build_manual_memcpy_suggestion<'tcx>(
190192
"clone_from_slice"
191193
};
192194

193-
format!(
194-
"{dst}.{method_str}(&{src_base_str}[{}..{}]);",
195-
src_offset.maybe_par(),
196-
src_limit.maybe_par()
197-
)
195+
let src = if is_array_length_equal_to_range(cx, start, end, src.base) {
196+
src_base_str
197+
} else {
198+
format!("{src_base_str}[{}..{}]", src_offset.maybe_par(), src_limit.maybe_par()).into()
199+
};
200+
201+
format!("{dst}.{method_str}(&{src});")
198202
}
199203

200204
/// a wrapper of `Sugg`. Besides what `Sugg` do, this removes unnecessary `0`;
@@ -452,3 +456,34 @@ fn get_loop_counters<'a, 'tcx>(
452456
.into()
453457
})
454458
}
459+
460+
fn is_array_length_equal_to_range(cx: &LateContext<'_>, start: &Expr<'_>, end: &Expr<'_>, arr: &Expr<'_>) -> bool {
461+
fn extract_lit_value(expr: &Expr<'_>) -> Option<u128> {
462+
if let ExprKind::Lit(lit) = expr.kind
463+
&& let ast::LitKind::Int(value, _) = lit.node
464+
{
465+
Some(value)
466+
} else {
467+
None
468+
}
469+
}
470+
471+
let arr_ty = cx.typeck_results().expr_ty(arr).peel_refs();
472+
473+
if let ty::Array(_, s) = arr_ty.kind() {
474+
let size: u128 = if let Some(size) = s.try_eval_target_usize(cx.tcx, cx.param_env) {
475+
size.into()
476+
} else {
477+
return false;
478+
};
479+
480+
let range = match (extract_lit_value(start), extract_lit_value(end)) {
481+
(Some(start_value), Some(end_value)) => end_value - start_value,
482+
_ => return false,
483+
};
484+
485+
size == range
486+
} else {
487+
false
488+
}
489+
}

tests/ui/manual_memcpy/without_loop_counters.rs

+20
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,26 @@ pub fn manual_copy(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) {
138138
for i in 0..dst.len() {
139139
dst[i] = src[i];
140140
}
141+
142+
// Range is equal to array length
143+
let src = [0, 1, 2, 3, 4];
144+
let mut dst = [0; 4];
145+
for i in 0..4 {
146+
//~^ ERROR: it looks like you're manually copying between slices
147+
dst[i] = src[i];
148+
}
149+
150+
let mut dst = [0; 6];
151+
for i in 0..5 {
152+
//~^ ERROR: it looks like you're manually copying between slices
153+
dst[i] = src[i];
154+
}
155+
156+
let mut dst = [0; 5];
157+
for i in 0..5 {
158+
//~^ ERROR: it looks like you're manually copying between slices
159+
dst[i] = src[i];
160+
}
141161
}
142162

143163
#[warn(clippy::needless_range_loop, clippy::manual_memcpy)]

tests/ui/manual_memcpy/without_loop_counters.stderr

+29-2
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ LL | / for i in 0..5 {
106106
LL | |
107107
LL | | dst[i - 0] = src[i];
108108
LL | | }
109-
| |_____^ help: try replacing the loop by: `dst[..5].copy_from_slice(&src[..5]);`
109+
| |_____^ help: try replacing the loop by: `dst[..5].copy_from_slice(&src);`
110110

111111
error: it looks like you're manually copying between slices
112112
--> $DIR/without_loop_counters.rs:121:5
@@ -120,11 +120,38 @@ LL | | }
120120
error: it looks like you're manually copying between slices
121121
--> $DIR/without_loop_counters.rs:145:5
122122
|
123+
LL | / for i in 0..4 {
124+
LL | |
125+
LL | | dst[i] = src[i];
126+
LL | | }
127+
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src[..4]);`
128+
129+
error: it looks like you're manually copying between slices
130+
--> $DIR/without_loop_counters.rs:151:5
131+
|
132+
LL | / for i in 0..5 {
133+
LL | |
134+
LL | | dst[i] = src[i];
135+
LL | | }
136+
| |_____^ help: try replacing the loop by: `dst[..5].copy_from_slice(&src);`
137+
138+
error: it looks like you're manually copying between slices
139+
--> $DIR/without_loop_counters.rs:157:5
140+
|
141+
LL | / for i in 0..5 {
142+
LL | |
143+
LL | | dst[i] = src[i];
144+
LL | | }
145+
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src);`
146+
147+
error: it looks like you're manually copying between slices
148+
--> $DIR/without_loop_counters.rs:165:5
149+
|
123150
LL | / for i in 0..src.len() {
124151
LL | |
125152
LL | | dst[i] = src[i].clone();
126153
LL | | }
127154
| |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..]);`
128155

129-
error: aborting due to 13 previous errors
156+
error: aborting due to 16 previous errors
130157

0 commit comments

Comments
 (0)