Rollup merge of #124595 - estebank:issue-104232, r=davidtwco

Suggest cloning `Arc` moved into closure

```
error[E0382]: borrow of moved value: `x`
  --> $DIR/moves-based-on-type-capture-clause-bad.rs:9:20
   |
LL |     let x = "Hello world!".to_string();
   |         - move occurs because `x` has type `String`, which does not implement the `Copy` trait
LL |     thread::spawn(move || {
   |                   ------- value moved into closure here
LL |         println!("{}", x);
   |                        - variable moved due to use in closure
LL |     });
LL |     println!("{}", x);
   |                    ^ value borrowed here after move
   |
   = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider cloning the value before moving it into the closure
   |
LL ~     let value = x.clone();
LL ~     thread::spawn(move || {
LL ~         println!("{}", value);
   |
```

Fix rust-lang/rust#104232.

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

@@ -518,11 +518,11 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 } = move_spans
                     && can_suggest_clone
                 {
-                    self.suggest_cloning(err, ty, expr, Some(move_spans));
+                    self.suggest_cloning(err, place.as_ref(), ty, expr, Some(move_spans));
                 } else if self.suggest_hoisting_call_outside_loop(err, expr) && can_suggest_clone {
                     // The place where the type moves would be misleading to suggest clone.
                     // #121466
-                    self.suggest_cloning(err, ty, expr, Some(move_spans));
+                    self.suggest_cloning(err, place.as_ref(), ty, expr, Some(move_spans));
                 }
             }
 
@@ -1224,6 +1224,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
     pub(crate) fn suggest_cloning(
         &self,
         err: &mut Diag<'_>,
+        place: PlaceRef<'tcx>,
         ty: Ty<'tcx>,
         expr: &'tcx hir::Expr<'tcx>,
         use_spans: Option<UseSpans<'tcx>>,
@@ -1238,7 +1239,13 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
         }
 
         if self.implements_clone(ty) {
-            self.suggest_cloning_inner(err, ty, expr);
+            if self.in_move_closure(expr) {
+                if let Some(name) = self.describe_place(place) {
+                    self.suggest_clone_of_captured_var_in_move_closure(err, &name, use_spans);
+                }
+            } else {
+                self.suggest_cloning_inner(err, ty, expr);
+            }
         } else if let ty::Adt(def, args) = ty.kind()
             && def.did().as_local().is_some()
             && def.variants().iter().all(|variant| {
@@ -1505,7 +1512,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
             if let hir::ExprKind::AddrOf(_, _, borrowed_expr) = expr.kind
                 && let Some(ty) = typeck_results.expr_ty_opt(borrowed_expr)
             {
-                self.suggest_cloning(&mut err, ty, borrowed_expr, Some(move_spans));
+                self.suggest_cloning(&mut err, place.as_ref(), ty, borrowed_expr, Some(move_spans));
             } else if typeck_results.expr_adjustments(expr).first().is_some_and(|adj| {
                 matches!(
                     adj.kind,
@@ -1518,7 +1525,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 )
             }) && let Some(ty) = typeck_results.expr_ty_opt(expr)
             {
-                self.suggest_cloning(&mut err, ty, expr, Some(move_spans));
+                self.suggest_cloning(&mut err, place.as_ref(), ty, expr, Some(move_spans));
             }
         }
         self.buffer_error(err);

compiler/rustc_borrowck/src/diagnostics/move_errors.rs

@@ -325,25 +325,17 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
         self.cannot_move_out_of(span, &description)
     }
 
-    fn suggest_clone_of_captured_var_in_move_closure(
+    pub(in crate::diagnostics) fn suggest_clone_of_captured_var_in_move_closure(
         &self,
         err: &mut Diag<'_>,
-        upvar_hir_id: HirId,
         upvar_name: &str,
         use_spans: Option<UseSpans<'tcx>>,
     ) {
         let tcx = self.infcx.tcx;
-        let typeck_results = tcx.typeck(self.mir_def_id());
         let Some(use_spans) = use_spans else { return };
         // We only care about the case where a closure captured a binding.
         let UseSpans::ClosureUse { args_span, .. } = use_spans else { return };
         let Some(body_id) = tcx.hir_node(self.mir_hir_id()).body_id() else { return };
-        // Fetch the type of the expression corresponding to the closure-captured binding.
-        let Some(captured_ty) = typeck_results.node_type_opt(upvar_hir_id) else { return };
-        if !self.implements_clone(captured_ty) {
-            // We only suggest cloning the captured binding if the type can actually be cloned.
-            return;
-        };
         // Find the closure that captured the binding.
         let mut expr_finder = FindExprBySpan::new(args_span, tcx);
         expr_finder.include_closures = true;
@@ -396,7 +388,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                     .indentation_before(stmt.span)
                     .unwrap_or_else(|| "    ".to_string());
                 err.multipart_suggestion_verbose(
-                    "clone the value before moving it into the closure",
+                    "consider cloning the value before moving it into the closure",
                     vec![
                         (
                             stmt.span.shrink_to_lo(),
@@ -426,7 +418,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 .indentation_before(closure_expr.span)
                 .unwrap_or_else(|| "    ".to_string());
             err.multipart_suggestion_verbose(
-                "clone the value before moving it into the closure",
+                "consider cloning the value before moving it into the closure",
                 vec![
                     (
                         closure_expr.span.shrink_to_lo(),
@@ -523,20 +515,12 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 );
 
                 let closure_span = tcx.def_span(def_id);
-                let mut err = self
+                self.cannot_move_out_of(span, &place_description)
-                    .cannot_move_out_of(span, &place_description)
                     .with_span_label(upvar_span, "captured outer variable")
                     .with_span_label(
                         closure_span,
                         format!("captured by this `{closure_kind}` closure"),
-                    );
+                    )
-                self.suggest_clone_of_captured_var_in_move_closure(
-                    &mut err,
-                    upvar_hir_id,
-                    &upvar_name,
-                    use_spans,
-                );
-                err
             }
             _ => {
                 let source = self.borrowed_content_source(deref_base);
@@ -597,7 +581,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                     };
 
                     if let Some(expr) = self.find_expr(span) {
-                        self.suggest_cloning(err, place_ty, expr, None);
+                        self.suggest_cloning(err, move_from.as_ref(), place_ty, expr, None);
                     }
 
                     err.subdiagnostic(crate::session_diagnostics::TypeNoCopy::Label {
@@ -629,7 +613,13 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 };
 
                 if let Some(expr) = self.find_expr(use_span) {
-                    self.suggest_cloning(err, place_ty, expr, Some(use_spans));
+                    self.suggest_cloning(
+                        err,
+                        original_path.as_ref(),
+                        place_ty,
+                        expr,
+                        Some(use_spans),
+                    );
                 }
 
                 err.subdiagnostic(crate::session_diagnostics::TypeNoCopy::Label {
@@ -832,7 +822,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 let place_desc = self.local_name(*local).map(|sym| format!("`{sym}`"));
 
                 if let Some(expr) = self.find_expr(binding_span) {
-                    self.suggest_cloning(err, bind_to.ty, expr, None);
+                    let local_place: PlaceRef<'tcx> = (*local).into();
+                    self.suggest_cloning(err, local_place, bind_to.ty, expr, None);
                 }
 
                 err.subdiagnostic(crate::session_diagnostics::TypeNoCopy::Label {

tests/ui/borrowck/borrowck-move-by-capture.stderr

@@ -12,7 +12,7 @@ LL |         let _h = to_fn_once(move || -> isize { *bar });
    |                             |                  move occurs because `bar` has type `Box<isize>`, which does not implement the `Copy` trait
    |                             `bar` is moved here
    |
-help: clone the value before moving it into the closure
+help: consider cloning the value before moving it into the closure
    |
 LL ~         let value = bar.clone();
 LL ~         let _h = to_fn_once(move || -> isize { value });

tests/ui/borrowck/borrowck-move-moved-value-into-closure.stderr

@@ -12,6 +12,12 @@ LL |     call_f(move|| { *t + 1 });
    |            ^^^^^^   -- use occurs due to use in closure
    |            |
    |            value used here after move
+   |
+help: consider cloning the value before moving it into the closure
+   |
+LL ~     let value = t.clone();
+LL ~     call_f(move|| { value + 1 });
+   |
 
 error: aborting due to 1 previous error
 

tests/ui/moves/moves-based-on-type-capture-clause-bad.fixed

@@ -0,0 +1,11 @@
+//@ run-rustfix
+use std::thread;
+
+fn main() {
+    let x = "Hello world!".to_string();
+    let value = x.clone();
+    thread::spawn(move || {
+        println!("{}", value);
+    });
+    println!("{}", x); //~ ERROR borrow of moved value
+}

tests/ui/moves/moves-based-on-type-capture-clause-bad.rs

@@ -1,3 +1,4 @@
+//@ run-rustfix
 use std::thread;
 
 fn main() {

tests/ui/moves/moves-based-on-type-capture-clause-bad.stderr

@@ -1,5 +1,5 @@
 error[E0382]: borrow of moved value: `x`
-  --> $DIR/moves-based-on-type-capture-clause-bad.rs:8:20
+  --> $DIR/moves-based-on-type-capture-clause-bad.rs:9:20
    |
 LL |     let x = "Hello world!".to_string();
    |         - move occurs because `x` has type `String`, which does not implement the `Copy` trait
@@ -12,6 +12,12 @@ LL |     println!("{}", x);
    |                    ^ value borrowed here after move
    |
    = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
+help: consider cloning the value before moving it into the closure
+   |
+LL ~     let value = x.clone();
+LL ~     thread::spawn(move || {
+LL ~         println!("{}", value);
+   |
 
 error: aborting due to 1 previous error
 

tests/ui/moves/no-capture-arc.stderr

@@ -13,6 +13,12 @@ LL |     assert_eq!((*arc_v)[2], 3);
    |                  ^^^^^ value borrowed here after move
    |
    = note: borrow occurs due to deref coercion to `Vec<i32>`
+help: consider cloning the value before moving it into the closure
+   |
+LL ~     let value = arc_v.clone();
+LL ~     thread::spawn(move|| {
+LL ~         assert_eq!((*value)[3], 4);
+   |
 
 error: aborting due to 1 previous error
 

tests/ui/moves/no-reuse-move-arc.fixed

@@ -0,0 +1,17 @@
+//@ run-rustfix
+use std::sync::Arc;
+use std::thread;
+
+fn main() {
+    let v = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
+    let arc_v = Arc::new(v);
+
+    let value = arc_v.clone();
+    thread::spawn(move|| {
+        assert_eq!((*value)[3], 4);
+    });
+
+    assert_eq!((*arc_v)[2], 3); //~ ERROR borrow of moved value: `arc_v`
+
+    println!("{:?}", *arc_v);
+}

tests/ui/moves/no-reuse-move-arc.rs

@@ -1,3 +1,4 @@
+//@ run-rustfix
 use std::sync::Arc;
 use std::thread;
 

tests/ui/moves/no-reuse-move-arc.stderr

@@ -1,5 +1,5 @@
 error[E0382]: borrow of moved value: `arc_v`
-  --> $DIR/no-reuse-move-arc.rs:12:18
+  --> $DIR/no-reuse-move-arc.rs:13:18
    |
 LL |     let arc_v = Arc::new(v);
    |         ----- move occurs because `arc_v` has type `Arc<Vec<i32>>`, which does not implement the `Copy` trait
@@ -13,6 +13,12 @@ LL |     assert_eq!((*arc_v)[2], 3);
    |                  ^^^^^ value borrowed here after move
    |
    = note: borrow occurs due to deref coercion to `Vec<i32>`
+help: consider cloning the value before moving it into the closure
+   |
+LL ~     let value = arc_v.clone();
+LL ~     thread::spawn(move|| {
+LL ~         assert_eq!((*value)[3], 4);
+   |
 
 error: aborting due to 1 previous error
 

tests/ui/suggestions/option-content-move3.stderr

@@ -79,7 +79,7 @@ LL |             let x = var;
    |                     variable moved due to use in closure
    |                     move occurs because `var` has type `NotCopyableButCloneable`, which does not implement the `Copy` trait
    |
-help: clone the value before moving it into the closure
+help: consider cloning the value before moving it into the closure
    |
 LL ~         {
 LL +         let value = var.clone();