Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify manual_memcpy suggestion in some cases #3323

Merged
merged 1 commit into from
Oct 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -950,8 +950,20 @@ fn detect_manual_memcpy<'a, 'tcx>(
("0", _, x, false) | (x, false, "0", false) => x.into(),
("0", _, x, true) | (x, false, "0", true) => format!("-{}", x),
(x, false, y, false) => format!("({} + {})", x, y),
(x, false, y, true) => format!("({} - {})", x, y),
(x, true, y, false) => format!("({} - {})", y, x),
(x, false, y, true) => {
if x == y {
"0".into()
} else {
format!("({} - {})", x, y)
}
},
(x, true, y, false) => {
if x == y {
"0".into()
} else {
format!("({} - {})", y, x)
}
},
(x, true, y, true) => format!("-({} + {})", x, y),
}
};
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/for_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,19 @@ pub fn manual_copy(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) {
for i in 0..10 {
dst_vec[i] = src[i];
}

// Simplify suggestion (issue #3004)
let src = [0, 1, 2, 3, 4];
let mut dst = [0, 0, 0, 0, 0, 0];
let from = 1;

for i in from..from + src.len() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add a test for values other than src.len(). I think your PR will also improve the suggestion in that case

dst[i] = src[i - from];
}

for i in from..from + 3 {
dst[i] = src[i - from];
}
}

#[warn(clippy::needless_range_loop)]
Expand Down
26 changes: 19 additions & 7 deletions tests/ui/for_loop.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -482,22 +482,34 @@ error: it looks like you're manually copying between slices
| ^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst_vec[..src_vec.len()].clone_from_slice(&src_vec[..])`

error: it looks like you're manually copying between slices
--> $DIR/for_loop.rs:557:14
--> $DIR/for_loop.rs:559:14
|
557 | for i in 0..src.len() {
559 | for i in from..from + src.len() {
| ^^^^^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..from + src.len()].clone_from_slice(&src[0..(from + src.len() - from)])`

error: it looks like you're manually copying between slices
--> $DIR/for_loop.rs:563:14
|
563 | for i in from..from + 3 {
| ^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..from + 3].clone_from_slice(&src[0..(from + 3 - from)])`

error: it looks like you're manually copying between slices
--> $DIR/for_loop.rs:570:14
|
570 | for i in 0..src.len() {
| ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])`

error: the variable `count` is used as a loop counter. Consider using `for (count, item) in text.chars().enumerate()` or similar iterators
--> $DIR/for_loop.rs:618:19
--> $DIR/for_loop.rs:631:19
|
618 | for ch in text.chars() {
631 | for ch in text.chars() {
| ^^^^^^^^^^^^

error: the variable `count` is used as a loop counter. Consider using `for (count, item) in text.chars().enumerate()` or similar iterators
--> $DIR/for_loop.rs:629:19
--> $DIR/for_loop.rs:642:19
|
629 | for ch in text.chars() {
642 | for ch in text.chars() {
| ^^^^^^^^^^^^

error: aborting due to 61 previous errors
error: aborting due to 63 previous errors

Empty file added tests/ui/for_loop.stdout
Empty file.