Skip to content

Commit 3925def

Browse files
JarchoJarcho
committed
Suggest copy_from_slice for manual_memcpy when possible
1 parent 14f3445 commit 3925def

File tree

3 files changed

+44
-35
lines changed

3 files changed

+44
-35
lines changed

‎clippy_lints/src/loops/manual_memcpy.rs

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use super::{IncrementVisitor, InitializeVisitor, MANUAL_MEMCPY};
22
use clippy_utils::diagnostics::span_lint_and_sugg;
33
use clippy_utils::source::snippet;
44
use clippy_utils::sugg::Sugg;
5+
use clippy_utils::ty::is_copy;
56
use clippy_utils::{get_enclosing_block, higher, path_to_local, sugg};
67
use if_chain::if_chain;
78
use rustc_ast::ast;
@@ -61,23 +62,23 @@ pub(super) fn check<'tcx>(
6162
if_chain! {
6263
if let ExprKind::Index(base_left, idx_left) = lhs.kind;
6364
if let ExprKind::Index(base_right, idx_right) = rhs.kind;
64-
if is_slice_like(cx, cx.typeck_results().expr_ty(base_left));
65-
if is_slice_like(cx, cx.typeck_results().expr_ty(base_right));
65+
if let Some(ty) = get_slice_like_element_ty(cx, cx.typeck_results().expr_ty(base_left));
66+
if get_slice_like_element_ty(cx, cx.typeck_results().expr_ty(base_right)).is_some();
6667
if let Some((start_left, offset_left)) = get_details_from_idx(cx, idx_left, &starts);
6768
if let Some((start_right, offset_right)) = get_details_from_idx(cx, idx_right, &starts);
6869

6970
// Source and destination must be different
7071
if path_to_local(base_left) != path_to_local(base_right);
7172
then {
72-
Some((IndexExpr { base: base_left, idx: start_left, idx_offset: offset_left },
73+
Some((ty, IndexExpr { base: base_left, idx: start_left, idx_offset: offset_left },
7374
IndexExpr { base: base_right, idx: start_right, idx_offset: offset_right }))
7475
} else {
7576
None
7677
}
7778
}
7879
})
7980
})
80-
.map(|o| o.map(|(dst, src)| build_manual_memcpy_suggestion(cx, start, end, limits, &dst, &src)))
81+
.map(|o| o.map(|(ty, dst, src)| build_manual_memcpy_suggestion(cx, start, end, limits, ty, &dst, &src)))
8182
.collect::<Option<Vec<_>>>()
8283
.filter(|v| !v.is_empty())
8384
.map(|v| v.join("\n "));
@@ -104,6 +105,7 @@ fn build_manual_memcpy_suggestion<'tcx>(
104105
start: &Expr<'_>,
105106
end: &Expr<'_>,
106107
limits: ast::RangeLimits,
108+
elem_ty: Ty<'tcx>,
107109
dst: &IndexExpr<'_>,
108110
src: &IndexExpr<'_>,
109111
) -> String {
@@ -186,9 +188,16 @@ fn build_manual_memcpy_suggestion<'tcx>(
186188
.into()
187189
};
188190

191+
let method_str = if is_copy(cx, elem_ty) {
192+
"copy_from_slice"
193+
} else {
194+
"clone_from_slice"
195+
};
196+
189197
format!(
190-
"{}.clone_from_slice(&{}[{}..{}]);",
198+
"{}.{}(&{}[{}..{}]);",
191199
dst,
200+
method_str,
192201
src_base_str,
193202
src_offset.maybe_par(),
194203
src_limit.maybe_par()
@@ -323,12 +332,12 @@ struct Start<'hir> {
323332
kind: StartKind<'hir>,
324333
}
325334

326-
fn is_slice_like<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'_>) -> bool {
335+
fn get_slice_like_element_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
327336
match ty.kind() {
328-
ty::Adt(adt, _) => cx.tcx.is_diagnostic_item(sym::Vec, adt.did),
329-
ty::Ref(_, subty, _) => is_slice_like(cx, subty),
330-
ty::Slice(..) | ty::Array(..) => true,
331-
_ => false,
337+
ty::Adt(adt, subs) if cx.tcx.is_diagnostic_item(sym::Vec, adt.did) => Some(subs.type_at(0)),
338+
ty::Ref(_, subty, _) => get_slice_like_element_ty(cx, subty),
339+
ty::Slice(ty) | ty::Array(ty, _) => Some(ty),
340+
_ => None,
332341
}
333342
}
334343

‎tests/ui/manual_memcpy/with_loop_counters.stderr

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | / for i in 3..src.len() {
55
LL | | dst[i] = src[count];
66
LL | | count += 1;
77
LL | | }
8-
| |_____^ help: try replacing the loop by: `dst[3..src.len()].clone_from_slice(&src[..(src.len() - 3)]);`
8+
| |_____^ help: try replacing the loop by: `dst[3..src.len()].copy_from_slice(&src[..(src.len() - 3)]);`
99
|
1010
= note: `-D clippy::manual-memcpy` implied by `-D warnings`
1111

@@ -16,7 +16,7 @@ LL | / for i in 3..src.len() {
1616
LL | | dst[count] = src[i];
1717
LL | | count += 1;
1818
LL | | }
19-
| |_____^ help: try replacing the loop by: `dst[..(src.len() - 3)].clone_from_slice(&src[3..]);`
19+
| |_____^ help: try replacing the loop by: `dst[..(src.len() - 3)].copy_from_slice(&src[3..]);`
2020

2121
error: it looks like you're manually copying between slices
2222
--> $DIR/with_loop_counters.rs:17:5
@@ -25,7 +25,7 @@ LL | / for i in 0..src.len() {
2525
LL | | dst[count] = src[i];
2626
LL | | count += 1;
2727
LL | | }
28-
| |_____^ help: try replacing the loop by: `dst[3..(src.len() + 3)].clone_from_slice(&src[..]);`
28+
| |_____^ help: try replacing the loop by: `dst[3..(src.len() + 3)].copy_from_slice(&src[..]);`
2929

3030
error: it looks like you're manually copying between slices
3131
--> $DIR/with_loop_counters.rs:23:5
@@ -34,7 +34,7 @@ LL | / for i in 0..src.len() {
3434
LL | | dst[i] = src[count];
3535
LL | | count += 1;
3636
LL | | }
37-
| |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[3..(src.len() + 3)]);`
37+
| |_____^ help: try replacing the loop by: `dst[..src.len()].copy_from_slice(&src[3..(src.len() + 3)]);`
3838

3939
error: it looks like you're manually copying between slices
4040
--> $DIR/with_loop_counters.rs:29:5
@@ -43,7 +43,7 @@ LL | / for i in 3..(3 + src.len()) {
4343
LL | | dst[i] = src[count];
4444
LL | | count += 1;
4545
LL | | }
46-
| |_____^ help: try replacing the loop by: `dst[3..(3 + src.len())].clone_from_slice(&src[..(3 + src.len() - 3)]);`
46+
| |_____^ help: try replacing the loop by: `dst[3..(3 + src.len())].copy_from_slice(&src[..(3 + src.len() - 3)]);`
4747

4848
error: it looks like you're manually copying between slices
4949
--> $DIR/with_loop_counters.rs:35:5
@@ -52,7 +52,7 @@ LL | / for i in 5..src.len() {
5252
LL | | dst[i] = src[count - 2];
5353
LL | | count += 1;
5454
LL | | }
55-
| |_____^ help: try replacing the loop by: `dst[5..src.len()].clone_from_slice(&src[(3 - 2)..((src.len() - 2) + 3 - 5)]);`
55+
| |_____^ help: try replacing the loop by: `dst[5..src.len()].copy_from_slice(&src[(3 - 2)..((src.len() - 2) + 3 - 5)]);`
5656

5757
error: it looks like you're manually copying between slices
5858
--> $DIR/with_loop_counters.rs:41:5
@@ -61,7 +61,7 @@ LL | / for i in 0..dst.len() {
6161
LL | | dst[i] = src[count];
6262
LL | | count += 1;
6363
LL | | }
64-
| |_____^ help: try replacing the loop by: `dst.clone_from_slice(&src[2..(dst.len() + 2)]);`
64+
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src[2..(dst.len() + 2)]);`
6565

6666
error: it looks like you're manually copying between slices
6767
--> $DIR/with_loop_counters.rs:47:5
@@ -70,7 +70,7 @@ LL | / for i in 3..10 {
7070
LL | | dst[i] = src[count];
7171
LL | | count += 1;
7272
LL | | }
73-
| |_____^ help: try replacing the loop by: `dst[3..10].clone_from_slice(&src[5..(10 + 5 - 3)]);`
73+
| |_____^ help: try replacing the loop by: `dst[3..10].copy_from_slice(&src[5..(10 + 5 - 3)]);`
7474

7575
error: it looks like you're manually copying between slices
7676
--> $DIR/with_loop_counters.rs:54:5
@@ -85,8 +85,8 @@ LL | | }
8585
|
8686
help: try replacing the loop by
8787
|
88-
LL ~ dst[3..(src.len() + 3)].clone_from_slice(&src[..]);
89-
LL + dst2[30..(src.len() + 30)].clone_from_slice(&src[..]);
88+
LL ~ dst[3..(src.len() + 3)].copy_from_slice(&src[..]);
89+
LL + dst2[30..(src.len() + 30)].copy_from_slice(&src[..]);
9090
|
9191

9292
error: it looks like you're manually copying between slices
@@ -96,7 +96,7 @@ LL | / for i in 0..1 << 1 {
9696
LL | | dst[count] = src[i + 2];
9797
LL | | count += 1;
9898
LL | | }
99-
| |_____^ help: try replacing the loop by: `dst[(0 << 1)..((1 << 1) + (0 << 1))].clone_from_slice(&src[2..((1 << 1) + 2)]);`
99+
| |_____^ help: try replacing the loop by: `dst[(0 << 1)..((1 << 1) + (0 << 1))].copy_from_slice(&src[2..((1 << 1) + 2)]);`
100100

101101
error: it looks like you're manually copying between slices
102102
--> $DIR/with_loop_counters.rs:71:5
@@ -105,7 +105,7 @@ LL | / for i in 3..src.len() {
105105
LL | | dst[i] = src[count];
106106
LL | | count += 1
107107
LL | | }
108-
| |_____^ help: try replacing the loop by: `dst[3..src.len()].clone_from_slice(&src[..(src.len() - 3)]);`
108+
| |_____^ help: try replacing the loop by: `dst[3..src.len()].copy_from_slice(&src[..(src.len() - 3)]);`
109109

110110
error: aborting due to 11 previous errors
111111

‎tests/ui/manual_memcpy/without_loop_counters.stderr

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ error: it looks like you're manually copying between slices
44
LL | / for i in 0..src.len() {
55
LL | | dst[i] = src[i];
66
LL | | }
7-
| |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..]);`
7+
| |_____^ help: try replacing the loop by: `dst[..src.len()].copy_from_slice(&src[..]);`
88
|
99
= note: `-D clippy::manual-memcpy` implied by `-D warnings`
1010

@@ -14,31 +14,31 @@ error: it looks like you're manually copying between slices
1414
LL | / for i in 0..src.len() {
1515
LL | | dst[i + 10] = src[i];
1616
LL | | }
17-
| |_____^ help: try replacing the loop by: `dst[10..(src.len() + 10)].clone_from_slice(&src[..]);`
17+
| |_____^ help: try replacing the loop by: `dst[10..(src.len() + 10)].copy_from_slice(&src[..]);`
1818

1919
error: it looks like you're manually copying between slices
2020
--> $DIR/without_loop_counters.rs:17:5
2121
|
2222
LL | / for i in 0..src.len() {
2323
LL | | dst[i] = src[i + 10];
2424
LL | | }
25-
| |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[10..(src.len() + 10)]);`
25+
| |_____^ help: try replacing the loop by: `dst[..src.len()].copy_from_slice(&src[10..(src.len() + 10)]);`
2626

2727
error: it looks like you're manually copying between slices
2828
--> $DIR/without_loop_counters.rs:22:5
2929
|
3030
LL | / for i in 11..src.len() {
3131
LL | | dst[i] = src[i - 10];
3232
LL | | }
33-
| |_____^ help: try replacing the loop by: `dst[11..src.len()].clone_from_slice(&src[(11 - 10)..(src.len() - 10)]);`
33+
| |_____^ help: try replacing the loop by: `dst[11..src.len()].copy_from_slice(&src[(11 - 10)..(src.len() - 10)]);`
3434

3535
error: it looks like you're manually copying between slices
3636
--> $DIR/without_loop_counters.rs:27:5
3737
|
3838
LL | / for i in 0..dst.len() {
3939
LL | | dst[i] = src[i];
4040
LL | | }
41-
| |_____^ help: try replacing the loop by: `dst.clone_from_slice(&src[..dst.len()]);`
41+
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src[..dst.len()]);`
4242

4343
error: it looks like you're manually copying between slices
4444
--> $DIR/without_loop_counters.rs:40:5
@@ -51,8 +51,8 @@ LL | | }
5151
|
5252
help: try replacing the loop by
5353
|
54-
LL ~ dst[10..256].clone_from_slice(&src[(10 - 5)..(256 - 5)]);
55-
LL + dst2[(10 + 500)..(256 + 500)].clone_from_slice(&src[10..256]);
54+
LL ~ dst[10..256].copy_from_slice(&src[(10 - 5)..(256 - 5)]);
55+
LL + dst2[(10 + 500)..(256 + 500)].copy_from_slice(&src[10..256]);
5656
|
5757

5858
error: it looks like you're manually copying between slices
@@ -61,47 +61,47 @@ error: it looks like you're manually copying between slices
6161
LL | / for i in 10..LOOP_OFFSET {
6262
LL | | dst[i + LOOP_OFFSET] = src[i - some_var];
6363
LL | | }
64-
| |_____^ help: try replacing the loop by: `dst[(10 + LOOP_OFFSET)..(LOOP_OFFSET + LOOP_OFFSET)].clone_from_slice(&src[(10 - some_var)..(LOOP_OFFSET - some_var)]);`
64+
| |_____^ help: try replacing the loop by: `dst[(10 + LOOP_OFFSET)..(LOOP_OFFSET + LOOP_OFFSET)].copy_from_slice(&src[(10 - some_var)..(LOOP_OFFSET - some_var)]);`
6565

6666
error: it looks like you're manually copying between slices
6767
--> $DIR/without_loop_counters.rs:65:5
6868
|
6969
LL | / for i in 0..src_vec.len() {
7070
LL | | dst_vec[i] = src_vec[i];
7171
LL | | }
72-
| |_____^ help: try replacing the loop by: `dst_vec[..src_vec.len()].clone_from_slice(&src_vec[..]);`
72+
| |_____^ help: try replacing the loop by: `dst_vec[..src_vec.len()].copy_from_slice(&src_vec[..]);`
7373

7474
error: it looks like you're manually copying between slices
7575
--> $DIR/without_loop_counters.rs:94:5
7676
|
7777
LL | / for i in from..from + src.len() {
7878
LL | | dst[i] = src[i - from];
7979
LL | | }
80-
| |_____^ help: try replacing the loop by: `dst[from..(from + src.len())].clone_from_slice(&src[..(from + src.len() - from)]);`
80+
| |_____^ help: try replacing the loop by: `dst[from..(from + src.len())].copy_from_slice(&src[..(from + src.len() - from)]);`
8181

8282
error: it looks like you're manually copying between slices
8383
--> $DIR/without_loop_counters.rs:98:5
8484
|
8585
LL | / for i in from..from + 3 {
8686
LL | | dst[i] = src[i - from];
8787
LL | | }
88-
| |_____^ help: try replacing the loop by: `dst[from..(from + 3)].clone_from_slice(&src[..(from + 3 - from)]);`
88+
| |_____^ help: try replacing the loop by: `dst[from..(from + 3)].copy_from_slice(&src[..(from + 3 - from)]);`
8989

9090
error: it looks like you're manually copying between slices
9191
--> $DIR/without_loop_counters.rs:103:5
9292
|
9393
LL | / for i in 0..5 {
9494
LL | | dst[i - 0] = src[i];
9595
LL | | }
96-
| |_____^ help: try replacing the loop by: `dst[..5].clone_from_slice(&src[..5]);`
96+
| |_____^ help: try replacing the loop by: `dst[..5].copy_from_slice(&src[..5]);`
9797

9898
error: it looks like you're manually copying between slices
9999
--> $DIR/without_loop_counters.rs:108:5
100100
|
101101
LL | / for i in 0..0 {
102102
LL | | dst[i] = src[i];
103103
LL | | }
104-
| |_____^ help: try replacing the loop by: `dst[..0].clone_from_slice(&src[..0]);`
104+
| |_____^ help: try replacing the loop by: `dst[..0].copy_from_slice(&src[..0]);`
105105

106106
error: it looks like you're manually copying between slices
107107
--> $DIR/without_loop_counters.rs:120:5

0 commit comments

Comments
 (0)