Auto merge of #11778 - granddaifuku:fix/reduce-indexing-manual_memcpy-when-array-length-is-equal-to-range, r=blyxyas
fix: [manual_memcpy] reduce indexing suggestions when array length is equal to loop range fixes: #11689 This PR improves `manual_memcpy` suggestions by reducing unnecessary indexing. For example, ```rust let src = [0, 1, 2, 3, 4]; let mut dest = [0; 4]; for i in 0..4 { dest[i] = src[i]; { ``` Clippy suggests `dest[..4].copy_from_slice(&src[..4]);`. I reduced this suggestion as `dest.copy_from_slice(&src[..4]);`. (Removed needless indexing.) changelog: [`manual_memcpy`]: Reduce indexing suggestions when array length is equal to loop range
This commit is contained in:
commit
ca8f33e19b
@ -178,7 +178,9 @@ fn print_offset(offset: MinifyingSugg<'static>) -> MinifyingSugg<'static> {
|
||||
let dst_base_str = snippet(cx, dst.base.span, "???");
|
||||
let src_base_str = snippet(cx, src.base.span, "???");
|
||||
|
||||
let dst = if dst_offset == sugg::EMPTY && dst_limit == sugg::EMPTY {
|
||||
let dst = if (dst_offset == sugg::EMPTY && dst_limit == sugg::EMPTY)
|
||||
|| is_array_length_equal_to_range(cx, start, end, dst.base)
|
||||
{
|
||||
dst_base_str
|
||||
} else {
|
||||
format!("{dst_base_str}[{}..{}]", dst_offset.maybe_par(), dst_limit.maybe_par()).into()
|
||||
@ -190,11 +192,13 @@ fn print_offset(offset: MinifyingSugg<'static>) -> MinifyingSugg<'static> {
|
||||
"clone_from_slice"
|
||||
};
|
||||
|
||||
format!(
|
||||
"{dst}.{method_str}(&{src_base_str}[{}..{}]);",
|
||||
src_offset.maybe_par(),
|
||||
src_limit.maybe_par()
|
||||
)
|
||||
let src = if is_array_length_equal_to_range(cx, start, end, src.base) {
|
||||
src_base_str
|
||||
} else {
|
||||
format!("{src_base_str}[{}..{}]", src_offset.maybe_par(), src_limit.maybe_par()).into()
|
||||
};
|
||||
|
||||
format!("{dst}.{method_str}(&{src});")
|
||||
}
|
||||
|
||||
/// a wrapper of `Sugg`. Besides what `Sugg` do, this removes unnecessary `0`;
|
||||
@ -452,3 +456,34 @@ fn get_loop_counters<'a, 'tcx>(
|
||||
.into()
|
||||
})
|
||||
}
|
||||
|
||||
fn is_array_length_equal_to_range(cx: &LateContext<'_>, start: &Expr<'_>, end: &Expr<'_>, arr: &Expr<'_>) -> bool {
|
||||
fn extract_lit_value(expr: &Expr<'_>) -> Option<u128> {
|
||||
if let ExprKind::Lit(lit) = expr.kind
|
||||
&& let ast::LitKind::Int(value, _) = lit.node
|
||||
{
|
||||
Some(value)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
let arr_ty = cx.typeck_results().expr_ty(arr).peel_refs();
|
||||
|
||||
if let ty::Array(_, s) = arr_ty.kind() {
|
||||
let size: u128 = if let Some(size) = s.try_eval_target_usize(cx.tcx, cx.param_env) {
|
||||
size.into()
|
||||
} else {
|
||||
return false;
|
||||
};
|
||||
|
||||
let range = match (extract_lit_value(start), extract_lit_value(end)) {
|
||||
(Some(start_value), Some(end_value)) => end_value - start_value,
|
||||
_ => return false,
|
||||
};
|
||||
|
||||
size == range
|
||||
} else {
|
||||
false
|
||||
}
|
||||
}
|
||||
|
@ -138,6 +138,26 @@ fn index(&self, _: usize) -> &i32 {
|
||||
for i in 0..dst.len() {
|
||||
dst[i] = src[i];
|
||||
}
|
||||
|
||||
// Range is equal to array length
|
||||
let src = [0, 1, 2, 3, 4];
|
||||
let mut dst = [0; 4];
|
||||
for i in 0..4 {
|
||||
//~^ ERROR: it looks like you're manually copying between slices
|
||||
dst[i] = src[i];
|
||||
}
|
||||
|
||||
let mut dst = [0; 6];
|
||||
for i in 0..5 {
|
||||
//~^ ERROR: it looks like you're manually copying between slices
|
||||
dst[i] = src[i];
|
||||
}
|
||||
|
||||
let mut dst = [0; 5];
|
||||
for i in 0..5 {
|
||||
//~^ ERROR: it looks like you're manually copying between slices
|
||||
dst[i] = src[i];
|
||||
}
|
||||
}
|
||||
|
||||
#[warn(clippy::needless_range_loop, clippy::manual_memcpy)]
|
||||
|
@ -106,7 +106,7 @@ LL | / for i in 0..5 {
|
||||
LL | |
|
||||
LL | | dst[i - 0] = src[i];
|
||||
LL | | }
|
||||
| |_____^ help: try replacing the loop by: `dst[..5].copy_from_slice(&src[..5]);`
|
||||
| |_____^ help: try replacing the loop by: `dst[..5].copy_from_slice(&src);`
|
||||
|
||||
error: it looks like you're manually copying between slices
|
||||
--> $DIR/without_loop_counters.rs:121:5
|
||||
@ -120,11 +120,38 @@ LL | | }
|
||||
error: it looks like you're manually copying between slices
|
||||
--> $DIR/without_loop_counters.rs:145:5
|
||||
|
|
||||
LL | / for i in 0..4 {
|
||||
LL | |
|
||||
LL | | dst[i] = src[i];
|
||||
LL | | }
|
||||
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src[..4]);`
|
||||
|
||||
error: it looks like you're manually copying between slices
|
||||
--> $DIR/without_loop_counters.rs:151:5
|
||||
|
|
||||
LL | / for i in 0..5 {
|
||||
LL | |
|
||||
LL | | dst[i] = src[i];
|
||||
LL | | }
|
||||
| |_____^ help: try replacing the loop by: `dst[..5].copy_from_slice(&src);`
|
||||
|
||||
error: it looks like you're manually copying between slices
|
||||
--> $DIR/without_loop_counters.rs:157:5
|
||||
|
|
||||
LL | / for i in 0..5 {
|
||||
LL | |
|
||||
LL | | dst[i] = src[i];
|
||||
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
|
||||
|
|
||||
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 13 previous errors
|
||||
error: aborting due to 16 previous errors
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user