about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2020-04-14 05:59:26 +0000
committerbors <bors@rust-lang.org>2020-04-14 05:59:26 +0000
commit74e92566d5cbe670a74a4ce340973716760479db (patch)
tree72e3a57192c874002f121eee27f4c8e69e9f4e91
parentaa08c39b117ddaa62a19421be726f341bb95a1f6 (diff)
parentab3946d7e9d83e8d177073f27d8ec704b399d687 (diff)
downloadrust-74e92566d5cbe670a74a4ce340973716760479db.tar.gz
rust-74e92566d5cbe670a74a4ce340973716760479db.zip
Auto merge of #5453 - rabisg0:fix/redundant_clone, r=phansch
Fixes #5405: redundant clone false positive with arrays

Check whether slice elements implement Copy before suggesting to drop
the clone method

changelog: add a check for slice indexing on redundant_clone lint
-rw-r--r--clippy_lints/src/redundant_clone.rs7
-rw-r--r--tests/ui/redundant_clone.fixed10
-rw-r--r--tests/ui/redundant_clone.rs10
-rw-r--r--tests/ui/redundant_clone.stderr20
4 files changed, 36 insertions, 11 deletions
diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs
index c315b575ef5..aedbafd408b 100644
--- a/clippy_lints/src/redundant_clone.rs
+++ b/clippy_lints/src/redundant_clone.rs
@@ -341,6 +341,9 @@ fn base_local_and_movability<'tcx>(
     let mut deref = false;
     // Accessing a field of an ADT that has `Drop`. Moving the field out will cause E0509.
     let mut field = false;
+    // If projection is a slice index then clone can be removed only if the
+    // underlying type implements Copy
+    let mut slice = false;
 
     let PlaceRef { local, mut projection } = place.as_ref();
     while let [base @ .., elem] = projection {
@@ -348,9 +351,11 @@ fn base_local_and_movability<'tcx>(
         deref |= matches!(elem, mir::ProjectionElem::Deref);
         field |= matches!(elem, mir::ProjectionElem::Field(..))
             && has_drop(cx, mir::Place::ty_from(local, projection, &mir.local_decls, cx.tcx).ty);
+        slice |= matches!(elem, mir::ProjectionElem::Index(..))
+            && !is_copy(cx, mir::Place::ty_from(local, projection, &mir.local_decls, cx.tcx).ty);
     }
 
-    Some((local, deref || field))
+    Some((local, deref || field || slice))
 }
 
 struct LocalUseVisitor {
diff --git a/tests/ui/redundant_clone.fixed b/tests/ui/redundant_clone.fixed
index 54815603c6d..764c10a6d39 100644
--- a/tests/ui/redundant_clone.fixed
+++ b/tests/ui/redundant_clone.fixed
@@ -51,6 +51,7 @@ fn main() {
     cannot_move_from_type_with_drop();
     borrower_propagation();
     not_consumed();
+    issue_5405();
 }
 
 #[derive(Clone)]
@@ -160,3 +161,12 @@ fn not_consumed() {
         println!("{}", x);
     }
 }
+
+#[allow(clippy::clone_on_copy)]
+fn issue_5405() {
+    let a: [String; 1] = [String::from("foo")];
+    let _b: String = a[0].clone();
+
+    let c: [usize; 2] = [2, 3];
+    let _d: usize = c[1].clone();
+}
diff --git a/tests/ui/redundant_clone.rs b/tests/ui/redundant_clone.rs
index a9b31183adc..839747b131d 100644
--- a/tests/ui/redundant_clone.rs
+++ b/tests/ui/redundant_clone.rs
@@ -51,6 +51,7 @@ fn main() {
     cannot_move_from_type_with_drop();
     borrower_propagation();
     not_consumed();
+    issue_5405();
 }
 
 #[derive(Clone)]
@@ -160,3 +161,12 @@ fn not_consumed() {
         println!("{}", x);
     }
 }
+
+#[allow(clippy::clone_on_copy)]
+fn issue_5405() {
+    let a: [String; 1] = [String::from("foo")];
+    let _b: String = a[0].clone();
+
+    let c: [usize; 2] = [2, 3];
+    let _d: usize = c[1].clone();
+}
diff --git a/tests/ui/redundant_clone.stderr b/tests/ui/redundant_clone.stderr
index 9c27812b9cd..eced198283c 100644
--- a/tests/ui/redundant_clone.stderr
+++ b/tests/ui/redundant_clone.stderr
@@ -108,61 +108,61 @@ LL |     let _t = tup.0.clone();
    |              ^^^^^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:60:22
+  --> $DIR/redundant_clone.rs:61:22
    |
 LL |         (a.clone(), a.clone())
    |                      ^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:60:21
+  --> $DIR/redundant_clone.rs:61:21
    |
 LL |         (a.clone(), a.clone())
    |                     ^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:120:15
+  --> $DIR/redundant_clone.rs:121:15
    |
 LL |     let _s = s.clone();
    |               ^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:120:14
+  --> $DIR/redundant_clone.rs:121:14
    |
 LL |     let _s = s.clone();
    |              ^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:121:15
+  --> $DIR/redundant_clone.rs:122:15
    |
 LL |     let _t = t.clone();
    |               ^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:121:14
+  --> $DIR/redundant_clone.rs:122:14
    |
 LL |     let _t = t.clone();
    |              ^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:131:19
+  --> $DIR/redundant_clone.rs:132:19
    |
 LL |         let _f = f.clone();
    |                   ^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:131:18
+  --> $DIR/redundant_clone.rs:132:18
    |
 LL |         let _f = f.clone();
    |                  ^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:143:14
+  --> $DIR/redundant_clone.rs:144:14
    |
 LL |     let y = x.clone().join("matthias");
    |              ^^^^^^^^ help: remove this
    |
 note: cloned value is neither consumed nor mutated
-  --> $DIR/redundant_clone.rs:143:13
+  --> $DIR/redundant_clone.rs:144:13
    |
 LL |     let y = x.clone().join("matthias");
    |             ^^^^^^^^^