Suggest cloning captured binding in move closure

```
error[E0507]: cannot move out of `bar`, a captured variable in an `FnMut` closure
  --> $DIR/borrowck-move-by-capture.rs:9:29
   |
LL |     let bar: Box<_> = Box::new(3);
   |         --- captured outer variable
LL |     let _g = to_fn_mut(|| {
   |                        -- captured by this `FnMut` closure
LL |         let _h = to_fn_once(move || -> isize { *bar });
   |                             ^^^^^^^^^^^^^^^^   ----
   |                             |                  |
   |                             |                  variable moved due to use in closure
   |                             |                  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
   |
LL ~         let value = bar.clone();
LL ~         let _h = to_fn_once(move || -> isize { value });
   |
```

Author: estebank

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

@@ -1241,7 +1241,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         }
     }
 
-    fn implements_clone(&self, ty: Ty<'tcx>) -> bool {
+    pub(crate) 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)

compiler/rustc_borrowck/src/diagnostics/move_errors.rs

@@ -2,10 +2,13 @@
 #![allow(rustc::untranslatable_diagnostic)]
 
 use rustc_errors::{Applicability, Diag};
+use rustc_hir::intravisit::Visitor;
+use rustc_hir::{CaptureBy, ExprKind, HirId, Node};
 use rustc_middle::mir::*;
 use rustc_middle::ty::{self, Ty};
 use rustc_mir_dataflow::move_paths::{LookupResult, MovePathIndex};
 use rustc_span::{BytePos, ExpnKind, MacroKind, Span};
+use rustc_trait_selection::traits::error_reporting::FindExprBySpan;
 
 use crate::diagnostics::CapturedMessageOpt;
 use crate::diagnostics::{DescribePlaceOpt, UseSpans};
@@ -303,17 +306,133 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
         self.cannot_move_out_of(span, &description)
     }
 
+    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;
+        expr_finder.visit_expr(tcx.hir().body(body_id).value);
+        let Some(closure_expr) = expr_finder.result else { return };
+        let ExprKind::Closure(closure) = closure_expr.kind else { return };
+        // We'll only suggest cloning the binding if it's a `move` closure.
+        let CaptureBy::Value { .. } = closure.capture_clause else { return };
+        // Find the expression within the closure where the binding is consumed.
+        let mut suggested = false;
+        let use_span = use_spans.var_or_use();
+        let mut expr_finder = FindExprBySpan::new(use_span, tcx);
+        expr_finder.include_closures = true;
+        expr_finder.visit_expr(tcx.hir().body(body_id).value);
+        let Some(use_expr) = expr_finder.result else { return };
+        let parent = tcx.parent_hir_node(use_expr.hir_id);
+        if let Node::Expr(expr) = parent
+            && let ExprKind::Assign(lhs, ..) = expr.kind
+            && lhs.hir_id == use_expr.hir_id
+        {
+            // Cloning the value being assigned makes no sense:
+            //
+            // error[E0507]: cannot move out of `var`, a captured variable in an `FnMut` closure
+            //   --> $DIR/option-content-move2.rs:11:9
+            //    |
+            // LL |     let mut var = None;
+            //    |         ------- captured outer variable
+            // LL |     func(|| {
+            //    |          -- captured by this `FnMut` closure
+            // LL |         // Shouldn't suggest `move ||.as_ref()` here
+            // LL |         move || {
+            //    |         ^^^^^^^ `var` is moved here
+            // LL |
+            // LL |             var = Some(NotCopyable);
+            //    |             ---
+            //    |             |
+            //    |             variable moved due to use in closure
+            //    |             move occurs because `var` has type `Option<NotCopyable>`, which does not implement the `Copy` trait
+            //    |
+            return;
+        }
+
+        // Search for an appropriate place for the structured `.clone()` suggestion to be applied.
+        // If we encounter a statement before the borrow error, we insert a statement there.
+        for (_, node) in tcx.hir().parent_iter(closure_expr.hir_id) {
+            if let Node::Stmt(stmt) = node {
+                let padding = tcx
+                    .sess
+                    .source_map()
+                    .indentation_before(stmt.span)
+                    .unwrap_or_else(|| "    ".to_string());
+                err.multipart_suggestion_verbose(
+                    "clone the value before moving it into the closure",
+                    vec![
+                        (
+                            stmt.span.shrink_to_lo(),
+                            format!("let value = {upvar_name}.clone();\n{padding}"),
+                        ),
+                        (use_span, "value".to_string()),
+                    ],
+                    Applicability::MachineApplicable,
+                );
+                suggested = true;
+                break;
+            } else if let Node::Expr(expr) = node
+                && let ExprKind::Closure(_) = expr.kind
+            {
+                // We want to suggest cloning only on the first closure, not
+                // subsequent ones (like `ui/suggestions/option-content-move2.rs`).
+                break;
+            }
+        }
+        if !suggested {
+            // If we couldn't find a statement for us to insert a new `.clone()` statement before,
+            // we have a bare expression, so we suggest the creation of a new block inline to go
+            // from `move || val` to `{ let value = val.clone(); move || value }`.
+            let padding = tcx
+                .sess
+                .source_map()
+                .indentation_before(closure_expr.span)
+                .unwrap_or_else(|| "    ".to_string());
+            err.multipart_suggestion_verbose(
+                "clone the value before moving it into the closure",
+                vec![
+                    (
+                        closure_expr.span.shrink_to_lo(),
+                        format!("{{\n{padding}let value = {upvar_name}.clone();\n{padding}"),
+                    ),
+                    (use_spans.var_or_use(), "value".to_string()),
+                    (closure_expr.span.shrink_to_hi(), format!("\n{padding}}}")),
+                ],
+                Applicability::MachineApplicable,
+            );
+        }
+    }
+
     fn report_cannot_move_from_borrowed_content(
         &mut self,
         move_place: Place<'tcx>,
         deref_target_place: Place<'tcx>,
         span: Span,
         use_spans: Option<UseSpans<'tcx>>,
     ) -> Diag<'tcx> {
+        let tcx = self.infcx.tcx;
         // Inspect the type of the content behind the
         // borrow to provide feedback about why this
         // was a move rather than a copy.
-        let ty = deref_target_place.ty(self.body, self.infcx.tcx).ty;
+        let ty = deref_target_place.ty(self.body, tcx).ty;
         let upvar_field = self
             .prefixes(move_place.as_ref(), PrefixSet::All)
             .find_map(|p| self.is_upvar_field_projection(p));
@@ -363,8 +482,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
 
                 let upvar = &self.upvars[upvar_field.unwrap().index()];
                 let upvar_hir_id = upvar.get_root_variable();
-                let upvar_name = upvar.to_string(self.infcx.tcx);
-                let upvar_span = self.infcx.tcx.hir().span(upvar_hir_id);
+                let upvar_name = upvar.to_string(tcx);
+                let upvar_span = tcx.hir().span(upvar_hir_id);
 
                 let place_name = self.describe_any_place(move_place.as_ref());
 
@@ -380,12 +499,21 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                     closure_kind_ty, closure_kind, place_description,
                 );
 
-                self.cannot_move_out_of(span, &place_description)
+                let closure_span = tcx.def_span(def_id);
+                let mut err = self
+                    .cannot_move_out_of(span, &place_description)
                     .with_span_label(upvar_span, "captured outer variable")
                     .with_span_label(
-                        self.infcx.tcx.def_span(def_id),
+                        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);
@@ -415,7 +543,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                     ),
                     (_, _, _) => self.cannot_move_out_of(
                         span,
-                        &source.describe_for_unnamed_place(self.infcx.tcx),
+                        &source.describe_for_unnamed_place(tcx),
                     ),
                 }
             }

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

@@ -50,12 +50,13 @@ pub struct FindExprBySpan<'hir> {
     pub span: Span,
     pub result: Option<&'hir hir::Expr<'hir>>,
     pub ty_result: Option<&'hir hir::Ty<'hir>>,
+    pub include_closures: bool,
     pub tcx: TyCtxt<'hir>,
 }
 
 impl<'hir> FindExprBySpan<'hir> {
     pub fn new(span: Span, tcx: TyCtxt<'hir>) -> Self {
-        Self { span, result: None, ty_result: None, tcx }
+        Self { span, result: None, ty_result: None, tcx, include_closures: false }
     }
 }
 
@@ -70,9 +71,17 @@ impl<'v> Visitor<'v> for FindExprBySpan<'v> {
         if self.span == ex.span {
             self.result = Some(ex);
         } else {
+            if let hir::ExprKind::Closure(..) = ex.kind
+                && self.include_closures
+                && let closure_header_sp = self.span.with_hi(ex.span.hi())
+                && closure_header_sp == ex.span
+            {
+                self.result = Some(ex);
+            }
             hir::intravisit::walk_expr(self, ex);
         }
     }
+
     fn visit_ty(&mut self, ty: &'v hir::Ty<'v>) {
         if self.span == ty.span {
             self.ty_result = Some(ty);

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

@@ -11,6 +11,12 @@ LL |         let _h = to_fn_once(move || -> isize { *bar });
    |                             |                  variable moved due to use in closure
    |                             |                  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
+   |
+LL ~         let value = bar.clone();
+LL ~         let _h = to_fn_once(move || -> isize { value });
+   |
 
 error: aborting due to 1 previous error
 

tests/ui/suggestions/option-content-move2.rs

@@ -1,8 +1,10 @@
 struct NotCopyable;
+#[derive(Clone)]
+struct NotCopyableButCloneable;
 
 fn func<F: FnMut() -> H, H: FnMut()>(_: F) {}
 
-fn parse() {
+fn foo() {
     let mut var = None;
     func(|| {
         // Shouldn't suggest `move ||.as_ref()` here
@@ -12,5 +14,15 @@ fn parse() {
         }
     });
 }
+fn bar() {
+    let mut var = None;
+    func(|| {
+        // Shouldn't suggest `move ||.as_ref()` nor to `clone()` here
+        move || {
+        //~^ ERROR: cannot move out of `var`
+            var = Some(NotCopyableButCloneable);
+        }
+    });
+}
 
 fn main() {}

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

@@ -1,5 +1,5 @@
 error[E0507]: cannot move out of `var`, a captured variable in an `FnMut` closure
-  --> $DIR/option-content-move2.rs:9:9
+  --> $DIR/option-content-move2.rs:11:9
    |
 LL |     let mut var = None;
    |         ------- captured outer variable
@@ -15,6 +15,23 @@ LL |             var = Some(NotCopyable);
    |             variable moved due to use in closure
    |             move occurs because `var` has type `Option<NotCopyable>`, which does not implement the `Copy` trait
 
-error: aborting due to 1 previous error
+error[E0507]: cannot move out of `var`, a captured variable in an `FnMut` closure
+  --> $DIR/option-content-move2.rs:21:9
+   |
+LL |     let mut var = None;
+   |         ------- captured outer variable
+LL |     func(|| {
+   |          -- captured by this `FnMut` closure
+LL |         // Shouldn't suggest `move ||.as_ref()` nor to `clone()` here
+LL |         move || {
+   |         ^^^^^^^ `var` is moved here
+LL |
+LL |             var = Some(NotCopyableButCloneable);
+   |             ---
+   |             |
+   |             variable moved due to use in closure
+   |             move occurs because `var` has type `Option<NotCopyableButCloneable>`, which does not implement the `Copy` trait
+
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0507`.

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

@@ -1,10 +1,23 @@
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 struct NotCopyable;
+#[derive(Debug, Clone)]
+struct NotCopyableButCloneable;
 
 fn func<F: FnMut() -> H, H: FnMut()>(_: F) {}
 
-fn parse() {
-    let mut var = NotCopyable;
+fn foo() {
+    let var = NotCopyable;
+    func(|| {
+        // Shouldn't suggest `move ||.as_ref()` here
+        move || { //~ ERROR cannot move out of `var`
+            let x = var; //~ ERROR cannot move out of `var`
+            println!("{x:?}");
+        }
+    });
+}
+
+fn bar() {
+    let var = NotCopyableButCloneable;
     func(|| {
         // Shouldn't suggest `move ||.as_ref()` here
         move || { //~ ERROR cannot move out of `var`