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
rust-lang · Apr 14, 2020 · 74e9256 · 74e9256
2 parents aa08c39 + ab3946d
commit 74e9256
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 11 deletions.
diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs
@@ -341,16 +341,21 @@ 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 {
         projection = base;
         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
@@ -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
@@ -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
@@ -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");
    |             ^^^^^^^^^