about summary refs log tree commit diff
diff options
context:
space:
mode:
authorEsteban Küber <esteban@kuber.com.ar>2023-11-19 23:53:31 +0000
committerEsteban Küber <esteban@kuber.com.ar>2023-12-04 21:54:32 +0000
commit03c88aaf218f3798e1b9ed98f18f57741d368132 (patch)
tree4cdfc976ddc867db83d2db152fd3179f5dacb3a2
parent0e2dac8375950a12812ec65868e42b43ed214ef9 (diff)
downloadrust-03c88aaf218f3798e1b9ed98f18f57741d368132.tar.gz
rust-03c88aaf218f3798e1b9ed98f18f57741d368132.zip
Tweak `.clone()` suggestion to work in more cases
When going through auto-deref, the `<T as Clone>` impl sometimes needs
to be specified for rustc to actually clone the value and not the
reference.

```
error[E0507]: cannot move out of dereference of `S`
  --> $DIR/needs-clone-through-deref.rs:15:18
   |
LL |         for _ in self.clone().into_iter() {}
   |                  ^^^^^^^^^^^^ ----------- value moved due to this method call
   |                  |
   |                  move occurs because value has type `Vec<usize>`, which does not implement the `Copy` trait
   |
note: `into_iter` takes ownership of the receiver `self`, which moves value
  --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
help: you can `clone` the value and consume it, but this might not be your desired behavior
   |
LL |         for _ in <Vec<usize> as Clone>::clone(&self.clone()).into_iter() {}
   |                  ++++++++++++++++++++++++++++++            +
```

CC #109429.
-rw-r--r--compiler/rustc_borrowck/src/diagnostics/mod.rs20
-rw-r--r--tests/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.stderr4
-rw-r--r--tests/ui/borrowck/clone-span-on-try-operator.fixed2
-rw-r--r--tests/ui/borrowck/clone-span-on-try-operator.stderr4
-rw-r--r--tests/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.fixed2
-rw-r--r--tests/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.stderr4
-rw-r--r--tests/ui/moves/needs-clone-through-deref.fixed18
-rw-r--r--tests/ui/moves/needs-clone-through-deref.rs18
-rw-r--r--tests/ui/moves/needs-clone-through-deref.stderr18
-rw-r--r--tests/ui/moves/suggest-clone.fixed2
-rw-r--r--tests/ui/moves/suggest-clone.stderr4
-rw-r--r--tests/ui/suggestions/option-content-move.stderr8
12 files changed, 86 insertions, 18 deletions
diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs
index 4deed98d002..31cb732cfd1 100644
--- a/compiler/rustc_borrowck/src/diagnostics/mod.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs
@@ -1135,11 +1135,25 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                             )
                             && self.infcx.predicate_must_hold_modulo_regions(&o)
                         {
-                            err.span_suggestion_verbose(
-                                move_span.shrink_to_hi(),
+                            let sugg = if moved_place
+                                .iter_projections()
+                                .any(|(_, elem)| matches!(elem, ProjectionElem::Deref))
+                            {
+                                vec![
+                                    // We use the fully-qualified path because `.clone()` can
+                                    // sometimes choose `<&T as Clone>` instead of `<T as Clone>`
+                                    // when going through auto-deref, so this ensures that doesn't
+                                    // happen, causing suggestions for `.clone().clone()`.
+                                    (move_span.shrink_to_lo(), format!("<{ty} as Clone>::clone(&")),
+                                    (move_span.shrink_to_hi(), ")".to_string()),
+                                ]
+                            } else {
+                                vec![(move_span.shrink_to_hi(), ".clone()".to_string())]
+                            };
+                            err.multipart_suggestion_verbose(
                                 "you can `clone` the value and consume it, but this might not be \
                                  your desired behavior",
-                                ".clone()".to_string(),
+                                sugg,
                                 Applicability::MaybeIncorrect,
                             );
                         }
diff --git a/tests/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.stderr b/tests/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.stderr
index 02794a12fad..2b4d3165977 100644
--- a/tests/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.stderr
+++ b/tests/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.stderr
@@ -10,8 +10,8 @@ note: `into_iter` takes ownership of the receiver `self`, which moves value
   --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
 help: you can `clone` the value and consume it, but this might not be your desired behavior
    |
-LL |     let _x = Rc::new(vec![1, 2]).clone().into_iter();
-   |                                 ++++++++
+LL |     let _x = <Vec<i32> as Clone>::clone(&Rc::new(vec![1, 2])).into_iter();
+   |              ++++++++++++++++++++++++++++                   +
 
 error: aborting due to 1 previous error
 
diff --git a/tests/ui/borrowck/clone-span-on-try-operator.fixed b/tests/ui/borrowck/clone-span-on-try-operator.fixed
index 52f66e43a93..4fad75b9a3d 100644
--- a/tests/ui/borrowck/clone-span-on-try-operator.fixed
+++ b/tests/ui/borrowck/clone-span-on-try-operator.fixed
@@ -7,5 +7,5 @@ impl Foo {
 }
 fn main() {
     let foo = &Foo;
-    (*foo).clone().foo(); //~ ERROR cannot move out
+    <Foo as Clone>::clone(&(*foo)).foo(); //~ ERROR cannot move out
 }
diff --git a/tests/ui/borrowck/clone-span-on-try-operator.stderr b/tests/ui/borrowck/clone-span-on-try-operator.stderr
index 5a55088d67a..adf84e49a9f 100644
--- a/tests/ui/borrowck/clone-span-on-try-operator.stderr
+++ b/tests/ui/borrowck/clone-span-on-try-operator.stderr
@@ -13,8 +13,8 @@ LL |     fn foo(self) {}
    |            ^^^^
 help: you can `clone` the value and consume it, but this might not be your desired behavior
    |
-LL |     (*foo).clone().foo();
-   |           ++++++++
+LL |     <Foo as Clone>::clone(&(*foo)).foo();
+   |     +++++++++++++++++++++++      +
 
 error: aborting due to 1 previous error
 
diff --git a/tests/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.fixed b/tests/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.fixed
index b0c5376105b..85acafd88f6 100644
--- a/tests/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.fixed
+++ b/tests/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.fixed
@@ -9,7 +9,7 @@ fn call<F>(f: F) where F : Fn() {
 fn main() {
     let y = vec![format!("World")];
     call(|| {
-        y.clone().into_iter();
+        <Vec<String> as Clone>::clone(&y).into_iter();
         //~^ ERROR cannot move out of `y`, a captured variable in an `Fn` closure
     });
 }
diff --git a/tests/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.stderr b/tests/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.stderr
index fd168b43ac5..a2ff70255f5 100644
--- a/tests/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.stderr
+++ b/tests/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.stderr
@@ -14,8 +14,8 @@ note: `into_iter` takes ownership of the receiver `self`, which moves `y`
   --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
 help: you can `clone` the value and consume it, but this might not be your desired behavior
    |
-LL |         y.clone().into_iter();
-   |          ++++++++
+LL |         <Vec<String> as Clone>::clone(&y).into_iter();
+   |         +++++++++++++++++++++++++++++++ +
 
 error: aborting due to 1 previous error
 
diff --git a/tests/ui/moves/needs-clone-through-deref.fixed b/tests/ui/moves/needs-clone-through-deref.fixed
new file mode 100644
index 00000000000..419718175e9
--- /dev/null
+++ b/tests/ui/moves/needs-clone-through-deref.fixed
@@ -0,0 +1,18 @@
+// run-rustfix
+#![allow(dead_code, noop_method_call)]
+use std::ops::Deref;
+struct S(Vec<usize>);
+impl Deref for S {
+    type Target = Vec<usize>;
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl S {
+    fn foo(&self) {
+        // `self.clone()` returns `&S`, not `Vec`
+        for _ in <Vec<usize> as Clone>::clone(&self.clone()).into_iter() {} //~ ERROR cannot move out of dereference of `S`
+    }
+}
+fn main() {}
diff --git a/tests/ui/moves/needs-clone-through-deref.rs b/tests/ui/moves/needs-clone-through-deref.rs
new file mode 100644
index 00000000000..8116008ffe3
--- /dev/null
+++ b/tests/ui/moves/needs-clone-through-deref.rs
@@ -0,0 +1,18 @@
+// run-rustfix
+#![allow(dead_code, noop_method_call)]
+use std::ops::Deref;
+struct S(Vec<usize>);
+impl Deref for S {
+    type Target = Vec<usize>;
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl S {
+    fn foo(&self) {
+        // `self.clone()` returns `&S`, not `Vec`
+        for _ in self.clone().into_iter() {} //~ ERROR cannot move out of dereference of `S`
+    }
+}
+fn main() {}
diff --git a/tests/ui/moves/needs-clone-through-deref.stderr b/tests/ui/moves/needs-clone-through-deref.stderr
new file mode 100644
index 00000000000..b6da6198af7
--- /dev/null
+++ b/tests/ui/moves/needs-clone-through-deref.stderr
@@ -0,0 +1,18 @@
+error[E0507]: cannot move out of dereference of `S`
+  --> $DIR/needs-clone-through-deref.rs:15:18
+   |
+LL |         for _ in self.clone().into_iter() {}
+   |                  ^^^^^^^^^^^^ ----------- value moved due to this method call
+   |                  |
+   |                  move occurs because value has type `Vec<usize>`, which does not implement the `Copy` trait
+   |
+note: `into_iter` takes ownership of the receiver `self`, which moves value
+  --> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
+help: you can `clone` the value and consume it, but this might not be your desired behavior
+   |
+LL |         for _ in <Vec<usize> as Clone>::clone(&self.clone()).into_iter() {}
+   |                  ++++++++++++++++++++++++++++++            +
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0507`.
diff --git a/tests/ui/moves/suggest-clone.fixed b/tests/ui/moves/suggest-clone.fixed
index 204bfdb10b0..0c4a94d77e4 100644
--- a/tests/ui/moves/suggest-clone.fixed
+++ b/tests/ui/moves/suggest-clone.fixed
@@ -7,5 +7,5 @@ impl Foo {
 }
 fn main() {
     let foo = &Foo;
-    foo.clone().foo(); //~ ERROR cannot move out
+    <Foo as Clone>::clone(&foo).foo(); //~ ERROR cannot move out
 }
diff --git a/tests/ui/moves/suggest-clone.stderr b/tests/ui/moves/suggest-clone.stderr
index e0b68f249ee..25e89a58955 100644
--- a/tests/ui/moves/suggest-clone.stderr
+++ b/tests/ui/moves/suggest-clone.stderr
@@ -13,8 +13,8 @@ LL |     fn foo(self) {}
    |            ^^^^
 help: you can `clone` the value and consume it, but this might not be your desired behavior
    |
-LL |     foo.clone().foo();
-   |        ++++++++
+LL |     <Foo as Clone>::clone(&foo).foo();
+   |     +++++++++++++++++++++++   +
 
 error: aborting due to 1 previous error
 
diff --git a/tests/ui/suggestions/option-content-move.stderr b/tests/ui/suggestions/option-content-move.stderr
index 5060606d842..d4b73706fca 100644
--- a/tests/ui/suggestions/option-content-move.stderr
+++ b/tests/ui/suggestions/option-content-move.stderr
@@ -11,8 +11,8 @@ note: `Option::<T>::unwrap` takes ownership of the receiver `self`, which moves
   --> $SRC_DIR/core/src/option.rs:LL:COL
 help: you can `clone` the value and consume it, but this might not be your desired behavior
    |
-LL |                 if selection.1.clone().unwrap().contains(selection.0) {
-   |                               ++++++++
+LL |                 if <Option<String> as Clone>::clone(&selection.1).unwrap().contains(selection.0) {
+   |                    ++++++++++++++++++++++++++++++++++           +
 
 error[E0507]: cannot move out of `selection.1` which is behind a shared reference
   --> $DIR/option-content-move.rs:27:20
@@ -27,8 +27,8 @@ note: `Result::<T, E>::unwrap` takes ownership of the receiver `self`, which mov
   --> $SRC_DIR/core/src/result.rs:LL:COL
 help: you can `clone` the value and consume it, but this might not be your desired behavior
    |
-LL |                 if selection.1.clone().unwrap().contains(selection.0) {
-   |                               ++++++++
+LL |                 if <Result<String, String> as Clone>::clone(&selection.1).unwrap().contains(selection.0) {
+   |                    ++++++++++++++++++++++++++++++++++++++++++           +
 
 error: aborting due to 2 previous errors