Skip to content

Commit 99aceeb

Browse files
rail-rainrail-rain
committed
Use the spans of the entire for loops for suggestions
1 parent 5c71352 commit 99aceeb

File tree

2 files changed

+140
-79
lines changed

2 files changed

+140
-79
lines changed

‎clippy_lints/src/loops.rs

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,17 @@ fn check_for_loop<'tcx>(
779779
detect_same_item_push(cx, pat, arg, body, expr);
780780
}
781781

782+
// this function assumes the given expression is a `for` loop.
783+
fn get_span_of_entire_for_loop(expr: &Expr<'_>) -> Span {
784+
// for some reason this is the only way to get the `Span`
785+
// of the entire `for` loop
786+
if let ExprKind::Match(_, arms, _) = &expr.kind {
787+
arms[0].body.span
788+
} else {
789+
unreachable!()
790+
}
791+
}
792+
782793
fn same_var<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, var: HirId) -> bool {
783794
if_chain! {
784795
if let ExprKind::Path(qpath) = &expr.kind;
@@ -1138,7 +1149,7 @@ fn build_manual_memcpy_suggestion<'tcx>(
11381149
};
11391150

11401151
format!(
1141-
"{}.clone_from_slice(&{}[{}..{}])",
1152+
"{}.clone_from_slice(&{}[{}..{}]);",
11421153
dst,
11431154
src_base_str,
11441155
src_offset.maybe_par(),
@@ -1218,7 +1229,7 @@ fn detect_manual_memcpy<'tcx>(
12181229
span_lint_and_sugg(
12191230
cx,
12201231
MANUAL_MEMCPY,
1221-
expr.span,
1232+
get_span_of_entire_for_loop(expr),
12221233
"it looks like you're manually copying between slices",
12231234
"try replacing the loop by",
12241235
big_sugg,
@@ -1734,13 +1745,7 @@ fn check_for_loop_explicit_counter<'tcx>(
17341745
then {
17351746
let mut applicability = Applicability::MachineApplicable;
17361747

1737-
// for some reason this is the only way to get the `Span`
1738-
// of the entire `for` loop
1739-
let for_span = if let ExprKind::Match(_, arms, _) = &expr.kind {
1740-
arms[0].body.span
1741-
} else {
1742-
unreachable!()
1743-
};
1748+
let for_span = get_span_of_entire_for_loop(expr);
17441749

17451750
span_lint_and_sugg(
17461751
cx,

‎tests/ui/manual_memcpy.stderr

Lines changed: 126 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1,148 +1,204 @@
11
error: it looks like you're manually copying between slices
2-
--> $DIR/manual_memcpy.rs:7:14
2+
--> $DIR/manual_memcpy.rs:7:5
33
|
4-
LL | for i in 0..src.len() {
5-
| ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])`
4+
LL | / for i in 0..src.len() {
5+
LL | | dst[i] = src[i];
6+
LL | | }
7+
| |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..]);`
68
|
79
= note: `-D clippy::manual-memcpy` implied by `-D warnings`
810

911
error: it looks like you're manually copying between slices
10-
--> $DIR/manual_memcpy.rs:12:14
12+
--> $DIR/manual_memcpy.rs:12:5
1113
|
12-
LL | for i in 0..src.len() {
13-
| ^^^^^^^^^^^^ help: try replacing the loop by: `dst[10..(src.len() + 10)].clone_from_slice(&src[..])`
14+
LL | / for i in 0..src.len() {
15+
LL | | dst[i + 10] = src[i];
16+
LL | | }
17+
| |_____^ help: try replacing the loop by: `dst[10..(src.len() + 10)].clone_from_slice(&src[..]);`
1418

1519
error: it looks like you're manually copying between slices
16-
--> $DIR/manual_memcpy.rs:17:14
20+
--> $DIR/manual_memcpy.rs:17:5
1721
|
18-
LL | for i in 0..src.len() {
19-
| ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[10..(src.len() + 10)])`
22+
LL | / for i in 0..src.len() {
23+
LL | | dst[i] = src[i + 10];
24+
LL | | }
25+
| |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[10..(src.len() + 10)]);`
2026

2127
error: it looks like you're manually copying between slices
22-
--> $DIR/manual_memcpy.rs:22:14
28+
--> $DIR/manual_memcpy.rs:22:5
2329
|
24-
LL | for i in 11..src.len() {
25-
| ^^^^^^^^^^^^^ help: try replacing the loop by: `dst[11..src.len()].clone_from_slice(&src[(11 - 10)..(src.len() - 10)])`
30+
LL | / for i in 11..src.len() {
31+
LL | | dst[i] = src[i - 10];
32+
LL | | }
33+
| |_____^ help: try replacing the loop by: `dst[11..src.len()].clone_from_slice(&src[(11 - 10)..(src.len() - 10)]);`
2634

2735
error: it looks like you're manually copying between slices
28-
--> $DIR/manual_memcpy.rs:27:14
36+
--> $DIR/manual_memcpy.rs:27:5
2937
|
30-
LL | for i in 0..dst.len() {
31-
| ^^^^^^^^^^^^ help: try replacing the loop by: `dst.clone_from_slice(&src[..dst.len()])`
38+
LL | / for i in 0..dst.len() {
39+
LL | | dst[i] = src[i];
40+
LL | | }
41+
| |_____^ help: try replacing the loop by: `dst.clone_from_slice(&src[..dst.len()]);`
3242

3343
error: it looks like you're manually copying between slices
34-
--> $DIR/manual_memcpy.rs:40:14
44+
--> $DIR/manual_memcpy.rs:40:5
3545
|
36-
LL | for i in 10..256 {
37-
| ^^^^^^^
46+
LL | / for i in 10..256 {
47+
LL | | dst[i] = src[i - 5];
48+
LL | | dst2[i + 500] = src[i]
49+
LL | | }
50+
| |_____^
3851
|
3952
help: try replacing the loop by
4053
|
41-
LL | for i in dst[10..256].clone_from_slice(&src[(10 - 5)..(256 - 5)])
42-
LL | dst2[(10 + 500)..(256 + 500)].clone_from_slice(&src[10..256]) {
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]);
4356
|
4457

4558
error: it looks like you're manually copying between slices
46-
--> $DIR/manual_memcpy.rs:52:14
59+
--> $DIR/manual_memcpy.rs:52:5
4760
|
48-
LL | for i in 10..LOOP_OFFSET {
49-
| ^^^^^^^^^^^^^^^ 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)])`
61+
LL | / for i in 10..LOOP_OFFSET {
62+
LL | | dst[i + LOOP_OFFSET] = src[i - some_var];
63+
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)]);`
5065

5166
error: it looks like you're manually copying between slices
52-
--> $DIR/manual_memcpy.rs:65:14
67+
--> $DIR/manual_memcpy.rs:65:5
5368
|
54-
LL | for i in 0..src_vec.len() {
55-
| ^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst_vec[..src_vec.len()].clone_from_slice(&src_vec[..])`
69+
LL | / for i in 0..src_vec.len() {
70+
LL | | dst_vec[i] = src_vec[i];
71+
LL | | }
72+
| |_____^ help: try replacing the loop by: `dst_vec[..src_vec.len()].clone_from_slice(&src_vec[..]);`
5673

5774
error: it looks like you're manually copying between slices
58-
--> $DIR/manual_memcpy.rs:94:14
75+
--> $DIR/manual_memcpy.rs:94:5
5976
|
60-
LL | for i in from..from + src.len() {
61-
| ^^^^^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..(from + src.len())].clone_from_slice(&src[..(from + src.len() - from)])`
77+
LL | / for i in from..from + src.len() {
78+
LL | | dst[i] = src[i - from];
79+
LL | | }
80+
| |_____^ help: try replacing the loop by: `dst[from..(from + src.len())].clone_from_slice(&src[..(from + src.len() - from)]);`
6281

6382
error: it looks like you're manually copying between slices
64-
--> $DIR/manual_memcpy.rs:98:14
83+
--> $DIR/manual_memcpy.rs:98:5
6584
|
66-
LL | for i in from..from + 3 {
67-
| ^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..(from + 3)].clone_from_slice(&src[..(from + 3 - from)])`
85+
LL | / for i in from..from + 3 {
86+
LL | | dst[i] = src[i - from];
87+
LL | | }
88+
| |_____^ help: try replacing the loop by: `dst[from..(from + 3)].clone_from_slice(&src[..(from + 3 - from)]);`
6889

6990
error: it looks like you're manually copying between slices
70-
--> $DIR/manual_memcpy.rs:103:14
91+
--> $DIR/manual_memcpy.rs:103:5
7192
|
72-
LL | for i in 0..5 {
73-
| ^^^^ help: try replacing the loop by: `dst[..5].clone_from_slice(&src[..5])`
93+
LL | / for i in 0..5 {
94+
LL | | dst[i - 0] = src[i];
95+
LL | | }
96+
| |_____^ help: try replacing the loop by: `dst[..5].clone_from_slice(&src[..5]);`
7497

7598
error: it looks like you're manually copying between slices
76-
--> $DIR/manual_memcpy.rs:108:14
99+
--> $DIR/manual_memcpy.rs:108:5
77100
|
78-
LL | for i in 0..0 {
79-
| ^^^^ help: try replacing the loop by: `dst[..0].clone_from_slice(&src[..0])`
101+
LL | / for i in 0..0 {
102+
LL | | dst[i] = src[i];
103+
LL | | }
104+
| |_____^ help: try replacing the loop by: `dst[..0].clone_from_slice(&src[..0]);`
80105

81106
error: it looks like you're manually copying between slices
82-
--> $DIR/manual_memcpy.rs:120:14
107+
--> $DIR/manual_memcpy.rs:120:5
83108
|
84-
LL | for i in 3..src.len() {
85-
| ^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..src.len()].clone_from_slice(&src[..(src.len() - 3)])`
109+
LL | / for i in 3..src.len() {
110+
LL | | dst[i] = src[count];
111+
LL | | count += 1;
112+
LL | | }
113+
| |_____^ help: try replacing the loop by: `dst[3..src.len()].clone_from_slice(&src[..(src.len() - 3)]);`
86114

87115
error: it looks like you're manually copying between slices
88-
--> $DIR/manual_memcpy.rs:126:14
116+
--> $DIR/manual_memcpy.rs:126:5
89117
|
90-
LL | for i in 3..src.len() {
91-
| ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..(src.len() - 3)].clone_from_slice(&src[3..])`
118+
LL | / for i in 3..src.len() {
119+
LL | | dst[count] = src[i];
120+
LL | | count += 1;
121+
LL | | }
122+
| |_____^ help: try replacing the loop by: `dst[..(src.len() - 3)].clone_from_slice(&src[3..]);`
92123

93124
error: it looks like you're manually copying between slices
94-
--> $DIR/manual_memcpy.rs:132:14
125+
--> $DIR/manual_memcpy.rs:132:5
95126
|
96-
LL | for i in 0..src.len() {
97-
| ^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..(src.len() + 3)].clone_from_slice(&src[..])`
127+
LL | / for i in 0..src.len() {
128+
LL | | dst[count] = src[i];
129+
LL | | count += 1;
130+
LL | | }
131+
| |_____^ help: try replacing the loop by: `dst[3..(src.len() + 3)].clone_from_slice(&src[..]);`
98132

99133
error: it looks like you're manually copying between slices
100-
--> $DIR/manual_memcpy.rs:138:14
134+
--> $DIR/manual_memcpy.rs:138:5
101135
|
102-
LL | for i in 0..src.len() {
103-
| ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[3..(src.len() + 3)])`
136+
LL | / for i in 0..src.len() {
137+
LL | | dst[i] = src[count];
138+
LL | | count += 1;
139+
LL | | }
140+
| |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[3..(src.len() + 3)]);`
104141

105142
error: it looks like you're manually copying between slices
106-
--> $DIR/manual_memcpy.rs:144:14
143+
--> $DIR/manual_memcpy.rs:144:5
107144
|
108-
LL | for i in 3..(3 + src.len()) {
109-
| ^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..((3 + src.len()))].clone_from_slice(&src[..((3 + src.len()) - 3)])`
145+
LL | / for i in 3..(3 + src.len()) {
146+
LL | | dst[i] = src[count];
147+
LL | | count += 1;
148+
LL | | }
149+
| |_____^ help: try replacing the loop by: `dst[3..((3 + src.len()))].clone_from_slice(&src[..((3 + src.len()) - 3)]);`
110150

111151
error: it looks like you're manually copying between slices
112-
--> $DIR/manual_memcpy.rs:150:14
152+
--> $DIR/manual_memcpy.rs:150:5
113153
|
114-
LL | for i in 5..src.len() {
115-
| ^^^^^^^^^^^^ help: try replacing the loop by: `dst[5..src.len()].clone_from_slice(&src[(3 - 2)..((src.len() - 2) + 3 - 5)])`
154+
LL | / for i in 5..src.len() {
155+
LL | | dst[i] = src[count - 2];
156+
LL | | count += 1;
157+
LL | | }
158+
| |_____^ help: try replacing the loop by: `dst[5..src.len()].clone_from_slice(&src[(3 - 2)..((src.len() - 2) + 3 - 5)]);`
116159

117160
error: it looks like you're manually copying between slices
118-
--> $DIR/manual_memcpy.rs:156:14
161+
--> $DIR/manual_memcpy.rs:156:5
119162
|
120-
LL | for i in 3..10 {
121-
| ^^^^^ help: try replacing the loop by: `dst[3..10].clone_from_slice(&src[5..(10 + 5 - 3)])`
163+
LL | / for i in 3..10 {
164+
LL | | dst[i] = src[count];
165+
LL | | count += 1;
166+
LL | | }
167+
| |_____^ help: try replacing the loop by: `dst[3..10].clone_from_slice(&src[5..(10 + 5 - 3)]);`
122168

123169
error: it looks like you're manually copying between slices
124-
--> $DIR/manual_memcpy.rs:163:14
170+
--> $DIR/manual_memcpy.rs:163:5
125171
|
126-
LL | for i in 0..src.len() {
127-
| ^^^^^^^^^^^^
172+
LL | / for i in 0..src.len() {
173+
LL | | dst[count] = src[i];
174+
LL | | dst2[count2] = src[i];
175+
LL | | count += 1;
176+
LL | | count2 += 1;
177+
LL | | }
178+
| |_____^
128179
|
129180
help: try replacing the loop by
130181
|
131-
LL | for i in dst[3..(src.len() + 3)].clone_from_slice(&src[..])
132-
LL | dst2[30..(src.len() + 30)].clone_from_slice(&src[..]) {
182+
LL | dst[3..(src.len() + 3)].clone_from_slice(&src[..]);
183+
LL | dst2[30..(src.len() + 30)].clone_from_slice(&src[..]);
133184
|
134185

135186
error: it looks like you're manually copying between slices
136-
--> $DIR/manual_memcpy.rs:173:14
187+
--> $DIR/manual_memcpy.rs:173:5
137188
|
138-
LL | for i in 0..1 << 1 {
139-
| ^^^^^^^^^ help: try replacing the loop by: `dst[(0 << 1)..((1 << 1) + (0 << 1))].clone_from_slice(&src[2..((1 << 1) + 2)])`
189+
LL | / for i in 0..1 << 1 {
190+
LL | | dst[count] = src[i + 2];
191+
LL | | count += 1;
192+
LL | | }
193+
| |_____^ help: try replacing the loop by: `dst[(0 << 1)..((1 << 1) + (0 << 1))].clone_from_slice(&src[2..((1 << 1) + 2)]);`
140194

141195
error: it looks like you're manually copying between slices
142-
--> $DIR/manual_memcpy.rs:181:14
196+
--> $DIR/manual_memcpy.rs:181:5
143197
|
144-
LL | for i in 0..src.len() {
145-
| ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])`
198+
LL | / for i in 0..src.len() {
199+
LL | | dst[i] = src[i].clone();
200+
LL | | }
201+
| |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..]);`
146202

147203
error: aborting due to 22 previous errors
148204

0 commit comments

Comments
 (0)