From 6816f774775076b98477c0d875b184074fd9cd67 Mon Sep 17 00:00:00 2001 From: Owen Sanchez Date: Mon, 15 Oct 2018 21:12:59 -0700 Subject: [PATCH] Simplify manual_memcpy suggestion in some cases --- clippy_lints/src/loops.rs | 16 ++++++++++++++-- tests/ui/for_loop.rs | 13 +++++++++++++ tests/ui/for_loop.stderr | 26 +++++++++++++++++++------- tests/ui/for_loop.stdout | 0 4 files changed, 46 insertions(+), 9 deletions(-) create mode 100644 tests/ui/for_loop.stdout diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 3c4f06077d98..4c505ababf14 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -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), } }; diff --git a/tests/ui/for_loop.rs b/tests/ui/for_loop.rs index 89c452f44df0..f80270d9fe8f 100644 --- a/tests/ui/for_loop.rs +++ b/tests/ui/for_loop.rs @@ -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() { + dst[i] = src[i - from]; + } + + for i in from..from + 3 { + dst[i] = src[i - from]; + } } #[warn(clippy::needless_range_loop)] diff --git a/tests/ui/for_loop.stderr b/tests/ui/for_loop.stderr index 472fa1486096..331763357832 100644 --- a/tests/ui/for_loop.stderr +++ b/tests/ui/for_loop.stderr @@ -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 diff --git a/tests/ui/for_loop.stdout b/tests/ui/for_loop.stdout new file mode 100644 index 000000000000..e69de29bb2d1