about summary refs log tree commit diff
diff options
context:
space:
mode:
authorEsteban Küber <esteban@kuber.com.ar>2024-03-16 18:19:17 +0000
committerEsteban Küber <esteban@kuber.com.ar>2024-04-11 16:41:42 +0000
commitd578ac9e476a376246320db1d934972601c5b0f5 (patch)
treea46e770f1bc3f01267ff1c4575b019fa03a419b6
parent4ca876b7a43c52a51cfcaf865a29a3ff27059b26 (diff)
downloadrust-d578ac9e476a376246320db1d934972601c5b0f5.tar.gz
rust-d578ac9e476a376246320db1d934972601c5b0f5.zip
Account for move error in the spread operator on struct literals
We attempt to suggest an appropriate clone for move errors on expressions
like `S { ..s }` where a field isn't `Copy`. If we can't suggest, we still don't
emit the incorrect suggestion of `S { ..s }.clone()`.

```
error[E0509]: cannot move out of type `S<K>`, which implements the `Drop` trait
  --> $DIR/borrowck-struct-update-with-dtor.rs:28:19
   |
LL |         let _s2 = S { a: 2, ..s0 };
   |                   ^^^^^^^^^^^^^^^^
   |                   |
   |                   cannot move out of here
   |                   move occurs because `s0.c` has type `K`, which does not implement the `Copy` trait
   |
help: clone the value from the field instead of using the spread operator syntax
   |
LL |         let _s2 = S { a: 2, c: s0.c.clone(), ..s0 };
   |                           +++++++++++++++++
```
```
error[E0509]: cannot move out of type `S<()>`, which implements the `Drop` trait
  --> $DIR/borrowck-struct-update-with-dtor.rs:20:19
   |
LL |         let _s2 = S { a: 2, ..s0 };
   |                   ^^^^^^^^^^^^^^^^
   |                   |
   |                   cannot move out of here
   |                   move occurs because `s0.b` has type `B`, which does not implement the `Copy` trait
   |
note: `B` doesn't implement `Copy` or `Clone`
  --> $DIR/borrowck-struct-update-with-dtor.rs:4:1
   |
LL | struct B;
   | ^^^^^^^^
help: if `B` implemented `Clone`, you could clone the value from the field instead of using the spread operator syntax
   |
LL |         let _s2 = S { a: 2, b: s0.b.clone(), ..s0 };
   |                           +++++++++++++++++
```
-rw-r--r--compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs118
-rw-r--r--tests/ui/borrowck/borrowck-struct-update-with-dtor.rs61
-rw-r--r--tests/ui/borrowck/borrowck-struct-update-with-dtor.stderr197
-rw-r--r--tests/ui/functional-struct-update/functional-struct-update-noncopyable.stderr6
4 files changed, 343 insertions, 39 deletions
diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
index d2d4345fed9..ee50e82d80d 100644
--- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
@@ -999,6 +999,93 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         can_suggest_clone
     }
 
+    fn suggest_cloning_on_spread_operator(
+        &self,
+        err: &mut Diag<'_>,
+        ty: Ty<'tcx>,
+        expr: &'cx hir::Expr<'cx>,
+    ) {
+        let typeck_results = self.infcx.tcx.typeck(self.mir_def_id());
+        let hir::ExprKind::Struct(struct_qpath, fields, Some(base)) = expr.kind else { return };
+        let hir::QPath::Resolved(_, path) = struct_qpath else { return };
+        let hir::def::Res::Def(_, def_id) = path.res else { return };
+        let Some(expr_ty) = typeck_results.node_type_opt(expr.hir_id) else { return };
+        let ty::Adt(def, args) = expr_ty.kind() else { return };
+        let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = base.kind else { return };
+        let (hir::def::Res::Local(_)
+        | hir::def::Res::Def(
+            DefKind::Const | DefKind::ConstParam | DefKind::Static { .. } | DefKind::AssocConst,
+            _,
+        )) = path.res
+        else {
+            return;
+        };
+        let Ok(base_str) = self.infcx.tcx.sess.source_map().span_to_snippet(base.span) else {
+            return;
+        };
+
+        // 1. look for the fields of type `ty`.
+        // 2. check if they are clone and add them to suggestion
+        // 3. check if there are any values left to `..` and remove it if not
+        // 4. emit suggestion to clone the field directly as `bar: base.bar.clone()`
+
+        let mut final_field_count = fields.len();
+        let Some(variant) = def.variants().iter().find(|variant| variant.def_id == def_id) else {
+            // When we have an enum, look for the variant that corresponds to the variant the user
+            // wrote.
+            return;
+        };
+        let mut sugg = vec![];
+        for field in &variant.fields {
+            // In practice unless there are more than one field with the same type, we'll be
+            // suggesting a single field at a type, because we don't aggregate multiple borrow
+            // checker errors involving the spread operator into a single one.
+            let field_ty = field.ty(self.infcx.tcx, args);
+            let ident = field.ident(self.infcx.tcx);
+            if field_ty == ty && fields.iter().all(|field| field.ident.name != ident.name) {
+                // Suggest adding field and cloning it.
+                sugg.push(format!("{ident}: {base_str}.{ident}.clone()"));
+                final_field_count += 1;
+            }
+        }
+        let (span, sugg) = match fields {
+            [.., last] => (
+                if final_field_count == variant.fields.len() {
+                    // We'll remove the `..base` as there aren't any fields left.
+                    last.span.shrink_to_hi().with_hi(base.span.hi())
+                } else {
+                    last.span.shrink_to_hi()
+                },
+                format!(", {}", sugg.join(", ")),
+            ),
+            // Account for no fields in suggestion span.
+            [] => (
+                expr.span.with_lo(struct_qpath.span().hi()),
+                if final_field_count == variant.fields.len() {
+                    // We'll remove the `..base` as there aren't any fields left.
+                    format!(" {{ {} }}", sugg.join(", "))
+                } else {
+                    format!(" {{ {}, ..{base_str} }}", sugg.join(", "))
+                },
+            ),
+        };
+        let prefix = if !self.implements_clone(ty) {
+            let msg = format!("`{ty}` doesn't implement `Copy` or `Clone`");
+            if let ty::Adt(def, _) = ty.kind() {
+                err.span_note(self.infcx.tcx.def_span(def.did()), msg);
+            } else {
+                err.note(msg);
+            }
+            format!("if `{ty}` implemented `Clone`, you could ")
+        } else {
+            String::new()
+        };
+        let msg = format!(
+            "{prefix}clone the value from the field instead of using the spread operator syntax",
+        );
+        err.span_suggestion_verbose(span, msg, sugg, Applicability::MachineApplicable);
+    }
+
     pub(crate) fn suggest_cloning(
         &self,
         err: &mut Diag<'_>,
@@ -1006,6 +1093,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         mut expr: &'cx hir::Expr<'cx>,
         mut other_expr: Option<&'cx hir::Expr<'cx>>,
     ) {
+        if let hir::ExprKind::Struct(_, _, Some(_)) = expr.kind {
+            // We have `S { foo: val, ..base }`. In `check_aggregate_rvalue` we have a single
+            // `Location` that covers both the `S { ... }` literal, all of its fields and the
+            // `base`. If the move happens because of `S { foo: val, bar: base.bar }` the `expr`
+            //  will already be correct. Instead, we see if we can suggest writing.
+            self.suggest_cloning_on_spread_operator(err, ty, expr);
+            return;
+        }
+
         if let Some(some_other_expr) = other_expr
             && let Some(parent_binop) =
                 self.infcx.tcx.hir().parent_iter(expr.hir_id).find_map(|n| {
@@ -1087,11 +1183,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                             } else {
                                 false
                             })
-                        && let Some(clone_trait_def) = self.infcx.tcx.lang_items().clone_trait()
-                        && self
-                            .infcx
-                            .type_implements_trait(clone_trait_def, [call_ty], self.param_env)
-                            .must_apply_modulo_regions()
+                        && self.implements_clone(call_ty)
                         && self.suggest_cloning_inner(err, call_ty, parent)
                     {
                         return;
@@ -1101,16 +1193,20 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             }
         }
         let ty = ty.peel_refs();
-        if let Some(clone_trait_def) = self.infcx.tcx.lang_items().clone_trait()
-            && self
-                .infcx
-                .type_implements_trait(clone_trait_def, [ty], self.param_env)
-                .must_apply_modulo_regions()
-        {
+        if self.implements_clone(ty) {
             self.suggest_cloning_inner(err, ty, expr);
+            // } else {
+            //     err.note(format!("if `{ty}` implemented `Clone`, you could clone the value"));
         }
     }
 
+    fn implements_clone(&self, ty: Ty<'tcx>) -> bool {
+        let Some(clone_trait_def) = self.infcx.tcx.lang_items().clone_trait() else { return false };
+        self.infcx
+            .type_implements_trait(clone_trait_def, [ty], self.param_env)
+            .must_apply_modulo_regions()
+    }
+
     pub(crate) fn clone_on_reference(&self, expr: &hir::Expr<'_>) -> Option<Span> {
         let typeck_results = self.infcx.tcx.typeck(self.mir_def_id());
         if let hir::ExprKind::MethodCall(segment, rcvr, args, span) = expr.kind
diff --git a/tests/ui/borrowck/borrowck-struct-update-with-dtor.rs b/tests/ui/borrowck/borrowck-struct-update-with-dtor.rs
index 1f6ed6d46aa..f0d067477c6 100644
--- a/tests/ui/borrowck/borrowck-struct-update-with-dtor.rs
+++ b/tests/ui/borrowck/borrowck-struct-update-with-dtor.rs
@@ -2,20 +2,63 @@
 // move, when the struct implements Drop.
 
 struct B;
-struct S { a: isize, b: B }
-impl Drop for S { fn drop(&mut self) { } }
+struct S<K> { a: isize, b: B, c: K }
+impl<K> Drop for S<K> { fn drop(&mut self) { } }
 
-struct T { a: isize, mv: Box<isize> }
+struct T { a: isize, b: Box<isize> }
 impl Drop for T { fn drop(&mut self) { } }
 
-fn f(s0:S) {
-    let _s2 = S{a: 2, ..s0};
-    //~^ ERROR [E0509]
+struct V<K> { a: isize, b: Box<isize>, c: K }
+impl<K> Drop for V<K> { fn drop(&mut self) { } }
+
+#[derive(Clone)]
+struct Clonable;
+
+mod not_all_clone {
+    use super::*;
+    fn a(s0: S<()>) {
+        let _s2 = S { a: 2, ..s0 };
+        //~^ ERROR [E0509]
+    }
+    fn b(s0: S<B>) {
+        let _s2 = S { a: 2, ..s0 };
+        //~^ ERROR [E0509]
+        //~| ERROR [E0509]
+    }
+    fn c<K: Clone>(s0: S<K>) {
+        let _s2 = S { a: 2, ..s0 };
+        //~^ ERROR [E0509]
+        //~| ERROR [E0509]
+    }
 }
+mod all_clone {
+    use super::*;
+    fn a(s0: T) {
+        let _s2 = T { a: 2, ..s0 };
+        //~^ ERROR [E0509]
+    }
+
+    fn b(s0: T) {
+        let _s2 = T { ..s0 };
+        //~^ ERROR [E0509]
+    }
+
+    fn c(s0: T) {
+        let _s2 = T { a: 2, b: s0.b };
+        //~^ ERROR [E0509]
+    }
+
+    fn d<K: Clone>(s0: V<K>) {
+        let _s2 = V { a: 2, ..s0 };
+        //~^ ERROR [E0509]
+        //~| ERROR [E0509]
+    }
 
-fn g(s0:T) {
-    let _s2 = T{a: 2, ..s0};
-    //~^ ERROR [E0509]
+    fn e(s0: V<Clonable>) {
+        let _s2 = V { a: 2, ..s0 };
+        //~^ ERROR [E0509]
+        //~| ERROR [E0509]
+    }
 }
 
 fn main() { }
diff --git a/tests/ui/borrowck/borrowck-struct-update-with-dtor.stderr b/tests/ui/borrowck/borrowck-struct-update-with-dtor.stderr
index 01004fa56c6..d34b07fbcc6 100644
--- a/tests/ui/borrowck/borrowck-struct-update-with-dtor.stderr
+++ b/tests/ui/borrowck/borrowck-struct-update-with-dtor.stderr
@@ -1,26 +1,191 @@
-error[E0509]: cannot move out of type `S`, which implements the `Drop` trait
-  --> $DIR/borrowck-struct-update-with-dtor.rs:12:15
+error[E0509]: cannot move out of type `S<()>`, which implements the `Drop` trait
+  --> $DIR/borrowck-struct-update-with-dtor.rs:20:19
    |
-LL |     let _s2 = S{a: 2, ..s0};
-   |               ^^^^^^^^^^^^^
-   |               |
-   |               cannot move out of here
-   |               move occurs because `s0.b` has type `B`, which does not implement the `Copy` trait
+LL |         let _s2 = S { a: 2, ..s0 };
+   |                   ^^^^^^^^^^^^^^^^
+   |                   |
+   |                   cannot move out of here
+   |                   move occurs because `s0.b` has type `B`, which does not implement the `Copy` trait
+   |
+note: `B` doesn't implement `Copy` or `Clone`
+  --> $DIR/borrowck-struct-update-with-dtor.rs:4:1
+   |
+LL | struct B;
+   | ^^^^^^^^
+help: if `B` implemented `Clone`, you could clone the value from the field instead of using the spread operator syntax
+   |
+LL |         let _s2 = S { a: 2, b: s0.b.clone(), ..s0 };
+   |                           +++++++++++++++++
+
+error[E0509]: cannot move out of type `S<B>`, which implements the `Drop` trait
+  --> $DIR/borrowck-struct-update-with-dtor.rs:24:19
+   |
+LL |         let _s2 = S { a: 2, ..s0 };
+   |                   ^^^^^^^^^^^^^^^^
+   |                   |
+   |                   cannot move out of here
+   |                   move occurs because `s0.b` has type `B`, which does not implement the `Copy` trait
+   |
+note: `B` doesn't implement `Copy` or `Clone`
+  --> $DIR/borrowck-struct-update-with-dtor.rs:4:1
+   |
+LL | struct B;
+   | ^^^^^^^^
+help: if `B` implemented `Clone`, you could clone the value from the field instead of using the spread operator syntax
+   |
+LL |         let _s2 = S { a: 2, b: s0.b.clone(), c: s0.c.clone() };
+   |                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+error[E0509]: cannot move out of type `S<B>`, which implements the `Drop` trait
+  --> $DIR/borrowck-struct-update-with-dtor.rs:24:19
+   |
+LL |         let _s2 = S { a: 2, ..s0 };
+   |                   ^^^^^^^^^^^^^^^^
+   |                   |
+   |                   cannot move out of here
+   |                   move occurs because `s0.c` has type `B`, which does not implement the `Copy` trait
+   |
+note: `B` doesn't implement `Copy` or `Clone`
+  --> $DIR/borrowck-struct-update-with-dtor.rs:4:1
+   |
+LL | struct B;
+   | ^^^^^^^^
+help: if `B` implemented `Clone`, you could clone the value from the field instead of using the spread operator syntax
+   |
+LL |         let _s2 = S { a: 2, b: s0.b.clone(), c: s0.c.clone() };
+   |                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+error[E0509]: cannot move out of type `S<K>`, which implements the `Drop` trait
+  --> $DIR/borrowck-struct-update-with-dtor.rs:29:19
+   |
+LL |         let _s2 = S { a: 2, ..s0 };
+   |                   ^^^^^^^^^^^^^^^^
+   |                   |
+   |                   cannot move out of here
+   |                   move occurs because `s0.b` has type `B`, which does not implement the `Copy` trait
+   |
+note: `B` doesn't implement `Copy` or `Clone`
+  --> $DIR/borrowck-struct-update-with-dtor.rs:4:1
+   |
+LL | struct B;
+   | ^^^^^^^^
+help: if `B` implemented `Clone`, you could clone the value from the field instead of using the spread operator syntax
+   |
+LL |         let _s2 = S { a: 2, b: s0.b.clone(), ..s0 };
+   |                           +++++++++++++++++
+
+error[E0509]: cannot move out of type `S<K>`, which implements the `Drop` trait
+  --> $DIR/borrowck-struct-update-with-dtor.rs:29:19
+   |
+LL |         let _s2 = S { a: 2, ..s0 };
+   |                   ^^^^^^^^^^^^^^^^
+   |                   |
+   |                   cannot move out of here
+   |                   move occurs because `s0.c` has type `K`, which does not implement the `Copy` trait
+   |
+help: clone the value from the field instead of using the spread operator syntax
+   |
+LL |         let _s2 = S { a: 2, c: s0.c.clone(), ..s0 };
+   |                           +++++++++++++++++
+
+error[E0509]: cannot move out of type `T`, which implements the `Drop` trait
+  --> $DIR/borrowck-struct-update-with-dtor.rs:37:19
+   |
+LL |         let _s2 = T { a: 2, ..s0 };
+   |                   ^^^^^^^^^^^^^^^^
+   |                   |
+   |                   cannot move out of here
+   |                   move occurs because `s0.b` has type `Box<isize>`, which does not implement the `Copy` trait
+   |
+help: clone the value from the field instead of using the spread operator syntax
+   |
+LL |         let _s2 = T { a: 2, b: s0.b.clone() };
+   |                           ~~~~~~~~~~~~~~~~~
+
+error[E0509]: cannot move out of type `T`, which implements the `Drop` trait
+  --> $DIR/borrowck-struct-update-with-dtor.rs:42:19
+   |
+LL |         let _s2 = T { ..s0 };
+   |                   ^^^^^^^^^^
+   |                   |
+   |                   cannot move out of here
+   |                   move occurs because `s0.b` has type `Box<isize>`, which does not implement the `Copy` trait
+   |
+help: clone the value from the field instead of using the spread operator syntax
+   |
+LL |         let _s2 = T { b: s0.b.clone(), ..s0 };
+   |                     ~~~~~~~~~~~~~~~~~~~~~~~~~
 
 error[E0509]: cannot move out of type `T`, which implements the `Drop` trait
-  --> $DIR/borrowck-struct-update-with-dtor.rs:17:15
+  --> $DIR/borrowck-struct-update-with-dtor.rs:47:32
    |
-LL |     let _s2 = T{a: 2, ..s0};
-   |               ^^^^^^^^^^^^^
-   |               |
-   |               cannot move out of here
-   |               move occurs because `s0.mv` has type `Box<isize>`, which does not implement the `Copy` trait
+LL |         let _s2 = T { a: 2, b: s0.b };
+   |                                ^^^^
+   |                                |
+   |                                cannot move out of here
+   |                                move occurs because `s0.b` has type `Box<isize>`, which does not implement the `Copy` trait
    |
 help: consider cloning the value if the performance cost is acceptable
    |
-LL |     let _s2 = T{a: 2, ..s0}.clone();
-   |                            ++++++++
+LL |         let _s2 = T { a: 2, b: s0.b.clone() };
+   |                                    ++++++++
+
+error[E0509]: cannot move out of type `V<K>`, which implements the `Drop` trait
+  --> $DIR/borrowck-struct-update-with-dtor.rs:52:19
+   |
+LL |         let _s2 = V { a: 2, ..s0 };
+   |                   ^^^^^^^^^^^^^^^^
+   |                   |
+   |                   cannot move out of here
+   |                   move occurs because `s0.b` has type `Box<isize>`, which does not implement the `Copy` trait
+   |
+help: clone the value from the field instead of using the spread operator syntax
+   |
+LL |         let _s2 = V { a: 2, b: s0.b.clone(), ..s0 };
+   |                           +++++++++++++++++
+
+error[E0509]: cannot move out of type `V<K>`, which implements the `Drop` trait
+  --> $DIR/borrowck-struct-update-with-dtor.rs:52:19
+   |
+LL |         let _s2 = V { a: 2, ..s0 };
+   |                   ^^^^^^^^^^^^^^^^
+   |                   |
+   |                   cannot move out of here
+   |                   move occurs because `s0.c` has type `K`, which does not implement the `Copy` trait
+   |
+help: clone the value from the field instead of using the spread operator syntax
+   |
+LL |         let _s2 = V { a: 2, c: s0.c.clone(), ..s0 };
+   |                           +++++++++++++++++
+
+error[E0509]: cannot move out of type `V<Clonable>`, which implements the `Drop` trait
+  --> $DIR/borrowck-struct-update-with-dtor.rs:58:19
+   |
+LL |         let _s2 = V { a: 2, ..s0 };
+   |                   ^^^^^^^^^^^^^^^^
+   |                   |
+   |                   cannot move out of here
+   |                   move occurs because `s0.b` has type `Box<isize>`, which does not implement the `Copy` trait
+   |
+help: clone the value from the field instead of using the spread operator syntax
+   |
+LL |         let _s2 = V { a: 2, b: s0.b.clone(), ..s0 };
+   |                           +++++++++++++++++
+
+error[E0509]: cannot move out of type `V<Clonable>`, which implements the `Drop` trait
+  --> $DIR/borrowck-struct-update-with-dtor.rs:58:19
+   |
+LL |         let _s2 = V { a: 2, ..s0 };
+   |                   ^^^^^^^^^^^^^^^^
+   |                   |
+   |                   cannot move out of here
+   |                   move occurs because `s0.c` has type `Clonable`, which does not implement the `Copy` trait
+   |
+help: clone the value from the field instead of using the spread operator syntax
+   |
+LL |         let _s2 = V { a: 2, c: s0.c.clone(), ..s0 };
+   |                           +++++++++++++++++
 
-error: aborting due to 2 previous errors
+error: aborting due to 12 previous errors
 
 For more information about this error, try `rustc --explain E0509`.
diff --git a/tests/ui/functional-struct-update/functional-struct-update-noncopyable.stderr b/tests/ui/functional-struct-update/functional-struct-update-noncopyable.stderr
index 4d314f4f7c3..3f07142eeb7 100644
--- a/tests/ui/functional-struct-update/functional-struct-update-noncopyable.stderr
+++ b/tests/ui/functional-struct-update/functional-struct-update-noncopyable.stderr
@@ -7,10 +7,10 @@ LL |     let _b = A { y: Arc::new(3), ..a };
    |              cannot move out of here
    |              move occurs because `a.x` has type `Arc<isize>`, which does not implement the `Copy` trait
    |
-help: clone the value to increment its reference count
+help: clone the value from the field instead of using the spread operator syntax
    |
-LL |     let _b = A { y: Arc::new(3), ..a }.clone();
-   |                                       ++++++++
+LL |     let _b = A { y: Arc::new(3), x: a.x.clone() };
+   |                                ~~~~~~~~~~~~~~~~
 
 error: aborting due to 1 previous error