Rollup merge of #144200 - estebank:dont-point-at-closure, r=lcnr

Tweak output for non-`Clone` values moved into closures

When we encounter a non-`Clone` value being moved into a closure, try to find the corresponding type of the binding being moved, if it is a `let`-binding or a function parameter. If any of those cases, we point at them with the note explaining that the type is not `Copy`, instead of giving that label to the place where it is captured. When it is a `let`-binding with no explicit type, we point at the initializer (if it fits in a single line).

```
error[E0507]: cannot move out of `foo`, a captured variable in an `Fn` closure
  --> f111.rs:14:25
   |
13 | fn do_stuff(foo: Option<Foo>) {
   |             ---  ----------- move occurs because `foo` has type `Option<Foo>`, which does not implement the `Copy` trait
   |             |
   |             captured outer variable
14 |     require_fn_trait(|| async {
   |                      -- ^^^^^ `foo` is moved here
   |                      |
   |                      captured by this `Fn` closure
15 |         if foo.map_or(false, |f| f.foo()) {
   |            --- variable moved due to use in coroutine
```

instead of

```
error[E0507]: cannot move out of `foo`, a captured variable in an `Fn` closure
  --> f111.rs:14:25
   |
13 | fn do_stuff(foo: Option<Foo>) {
   |             --- captured outer variable
14 |     require_fn_trait(|| async {
   |                      -- ^^^^^ `foo` is moved here
   |                      |
   |                      captured by this `Fn` closure
15 |         if foo.map_or(false, |f| f.foo()) {
   |            ---
   |            |
   |            variable moved due to use in coroutine
   |            move occurs because `foo` has type `Option<Foo>`, which does not implement the `Copy` trait
```

Author: matthiaskrgr

compiler/rustc_borrowck/src/diagnostics/move_errors.rs

@@ -115,10 +115,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
     fn append_to_grouped_errors(
         &self,
         grouped_errors: &mut Vec<GroupedMoveError<'tcx>>,
-        error: MoveError<'tcx>,
+        MoveError { place: original_path, location, kind }: MoveError<'tcx>,
     ) {
-        let MoveError { place: original_path, location, kind } = error;
-
         // Note: that the only time we assign a place isn't a temporary
         // to a user variable is when initializing it.
         // If that ever stops being the case, then the ever initialized
@@ -251,54 +249,47 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
     }
 
     fn report(&mut self, error: GroupedMoveError<'tcx>) {
-        let (mut err, err_span) = {
-            let (span, use_spans, original_path, kind) = match error {
-                GroupedMoveError::MovesFromPlace { span, original_path, ref kind, .. }
-                | GroupedMoveError::MovesFromValue { span, original_path, ref kind, .. } => {
-                    (span, None, original_path, kind)
-                }
-                GroupedMoveError::OtherIllegalMove { use_spans, original_path, ref kind } => {
-                    (use_spans.args_or_use(), Some(use_spans), original_path, kind)
-                }
-            };
-            debug!(
-                "report: original_path={:?} span={:?}, kind={:?} \
-                   original_path.is_upvar_field_projection={:?}",
-                original_path,
-                span,
-                kind,
-                self.is_upvar_field_projection(original_path.as_ref())
-            );
-            if self.has_ambiguous_copy(original_path.ty(self.body, self.infcx.tcx).ty) {
-                // If the type may implement Copy, skip the error.
-                // It's an error with the Copy implementation (e.g. duplicate Copy) rather than borrow check
-                self.dcx().span_delayed_bug(
+        let (span, use_spans, original_path, kind) = match error {
+            GroupedMoveError::MovesFromPlace { span, original_path, ref kind, .. }
+            | GroupedMoveError::MovesFromValue { span, original_path, ref kind, .. } => {
+                (span, None, original_path, kind)
+            }
+            GroupedMoveError::OtherIllegalMove { use_spans, original_path, ref kind } => {
+                (use_spans.args_or_use(), Some(use_spans), original_path, kind)
+            }
+        };
+        debug!(
+            "report: original_path={:?} span={:?}, kind={:?} \
+             original_path.is_upvar_field_projection={:?}",
+            original_path,
+            span,
+            kind,
+            self.is_upvar_field_projection(original_path.as_ref())
+        );
+        if self.has_ambiguous_copy(original_path.ty(self.body, self.infcx.tcx).ty) {
+            // If the type may implement Copy, skip the error.
+            // It's an error with the Copy implementation (e.g. duplicate Copy) rather than borrow check
+            self.dcx()
+                .span_delayed_bug(span, "Type may implement copy, but there is no other error.");
+            return;
+        }
+        let mut err = match kind {
+            &IllegalMoveOriginKind::BorrowedContent { target_place } => self
+                .report_cannot_move_from_borrowed_content(
+                    original_path,
+                    target_place,
                     span,
-                    "Type may implement copy, but there is no other error.",
-                );
-                return;
+                    use_spans,
+                ),
+            &IllegalMoveOriginKind::InteriorOfTypeWithDestructor { container_ty: ty } => {
+                self.cannot_move_out_of_interior_of_drop(span, ty)
+            }
+            &IllegalMoveOriginKind::InteriorOfSliceOrArray { ty, is_index } => {
+                self.cannot_move_out_of_interior_noncopy(span, ty, Some(is_index))
             }
-            (
-                match kind {
-                    &IllegalMoveOriginKind::BorrowedContent { target_place } => self
-                        .report_cannot_move_from_borrowed_content(
-                            original_path,
-                            target_place,
-                            span,
-                            use_spans,
-                        ),
-                    &IllegalMoveOriginKind::InteriorOfTypeWithDestructor { container_ty: ty } => {
-                        self.cannot_move_out_of_interior_of_drop(span, ty)
-                    }
-                    &IllegalMoveOriginKind::InteriorOfSliceOrArray { ty, is_index } => {
-                        self.cannot_move_out_of_interior_noncopy(span, ty, Some(is_index))
-                    }
-                },
-                span,
-            )
         };
 
-        self.add_move_hints(error, &mut err, err_span);
+        self.add_move_hints(error, &mut err, span);
         self.buffer_error(err);
     }
 
@@ -483,7 +474,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 self.cannot_move_out_of_interior_noncopy(span, ty, None)
             }
             ty::Closure(def_id, closure_args)
-                if def_id.as_local() == Some(self.mir_def_id()) && upvar_field.is_some() =>
+                if def_id.as_local() == Some(self.mir_def_id())
+                    && let Some(upvar_field) = upvar_field =>
             {
                 let closure_kind_ty = closure_args.as_closure().kind_ty();
                 let closure_kind = match closure_kind_ty.to_opt_closure_kind() {
@@ -496,7 +488,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 let capture_description =
                     format!("captured variable in an `{closure_kind}` closure");
 
-                let upvar = &self.upvars[upvar_field.unwrap().index()];
+                let upvar = &self.upvars[upvar_field.index()];
                 let upvar_hir_id = upvar.get_root_variable();
                 let upvar_name = upvar.to_string(tcx);
                 let upvar_span = tcx.hir_span(upvar_hir_id);
@@ -606,7 +598,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
             }
             // No binding. Nothing to suggest.
             GroupedMoveError::OtherIllegalMove { ref original_path, use_spans, .. } => {
-                let use_span = use_spans.var_or_use();
+                let mut use_span = use_spans.var_or_use();
                 let place_ty = original_path.ty(self.body, self.infcx.tcx).ty;
                 let place_desc = match self.describe_place(original_path.as_ref()) {
                     Some(desc) => format!("`{desc}`"),
@@ -623,19 +615,59 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                     );
                 }
 
+                if let Some(upvar_field) = self
+                    .prefixes(original_path.as_ref(), PrefixSet::All)
+                    .find_map(|p| self.is_upvar_field_projection(p))
+                {
+                    // Look for the introduction of the original binding being moved.
+                    let upvar = &self.upvars[upvar_field.index()];
+                    let upvar_hir_id = upvar.get_root_variable();
+                    use_span = match self.infcx.tcx.parent_hir_node(upvar_hir_id) {
+                        hir::Node::Param(param) => {
+                            // Instead of pointing at the path where we access the value within a
+                            // closure, we point at the type of the outer `fn` argument.
+                            param.ty_span
+                        }
+                        hir::Node::LetStmt(stmt) => match (stmt.ty, stmt.init) {
+                            // We point at the type of the outer let-binding.
+                            (Some(ty), _) => ty.span,
+                            // We point at the initializer of the outer let-binding, but only if it
+                            // isn't something that spans multiple lines, like a closure, as the
+                            // ASCII art gets messy.
+                            (None, Some(init))
+                                if !self.infcx.tcx.sess.source_map().is_multiline(init.span) =>
+                            {
+                                init.span
+                            }
+                            _ => use_span,
+                        },
+                        _ => use_span,
+                    };
+                }
+
                 err.subdiagnostic(crate::session_diagnostics::TypeNoCopy::Label {
                     is_partial_move: false,
                     ty: place_ty,
                     place: &place_desc,
                     span: use_span,
                 });
 
+                let mut pointed_at_span = false;
                 use_spans.args_subdiag(err, |args_span| {
+                    if args_span == span || args_span == use_span {
+                        pointed_at_span = true;
+                    }
                     crate::session_diagnostics::CaptureArgLabel::MoveOutPlace {
-                        place: place_desc,
+                        place: place_desc.clone(),
                         args_span,
                     }
                 });
+                if !pointed_at_span && use_span != span {
+                    err.subdiagnostic(crate::session_diagnostics::CaptureArgLabel::MoveOutPlace {
+                        place: place_desc,
+                        args_span: span,
+                    });
+                }
 
                 self.add_note_for_packed_struct_derive(err, original_path.local);
             }

tests/ui/async-await/async-closures/closure-shim-borrowck-error.stderr

@@ -1,14 +1,13 @@
 error[E0507]: cannot move out of `x` which is behind a mutable reference
   --> $DIR/closure-shim-borrowck-error.rs:11:18
    |
+LL | fn hello(x: Ty) {
+   |             -- move occurs because `x` has type `Ty`, which does not implement the `Copy` trait
 LL |     needs_fn_mut(async || {
    |                  ^^^^^^^^ `x` is moved here
 LL |
 LL |         x.hello();
-   |         -
-   |         |
-   |         variable moved due to use in coroutine
-   |         move occurs because `x` has type `Ty`, which does not implement the `Copy` trait
+   |         - variable moved due to use in coroutine
    |
 note: if `Ty` implemented `Clone`, you could clone the value
   --> $DIR/closure-shim-borrowck-error.rs:17:1

tests/ui/async-await/async-closures/move-out-of-ref.stderr

@@ -1,8 +1,11 @@
 error[E0507]: cannot move out of `*x` which is behind a shared reference
   --> $DIR/move-out-of-ref.rs:9:9
    |
+LL | fn hello(x: &Ty) {
+   |             --- move occurs because `*x` has type `Ty`, which does not implement the `Copy` trait
+LL |     let c = async || {
 LL |         *x;
-   |         ^^ move occurs because `*x` has type `Ty`, which does not implement the `Copy` trait
+   |         ^^ `*x` is moved here
    |
 note: if `Ty` implemented `Clone`, you could clone the value
   --> $DIR/move-out-of-ref.rs:5:1

tests/ui/borrowck/borrowck-in-static.stderr

@@ -2,9 +2,11 @@ error[E0507]: cannot move out of `x`, a captured variable in an `Fn` closure
   --> $DIR/borrowck-in-static.rs:5:17
    |
 LL |     let x = Box::new(0);
-   |         - captured outer variable
+   |         -   ----------- move occurs because `x` has type `Box<i32>`, which does not implement the `Copy` trait
+   |         |
+   |         captured outer variable
 LL |     Box::new(|| x)
-   |              -- ^ move occurs because `x` has type `Box<i32>`, which does not implement the `Copy` trait
+   |              -- ^ `x` is moved here
    |              |
    |              captured by this `Fn` closure
    |

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

@@ -2,14 +2,14 @@ error[E0507]: cannot move out of `bar`, a captured variable in an `FnMut` closur
   --> $DIR/borrowck-move-by-capture.rs:9:29
    |
 LL |     let bar: Box<_> = Box::new(3);
-   |         --- captured outer variable
+   |         ---  ------ move occurs because `bar` has type `Box<isize>`, which does not implement the `Copy` trait
+   |         |
+   |         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
+   |                             ^^^^^^^^^^^^^^^^   ---- variable moved due to use in closure
+   |                             |
    |                             `bar` is moved here
    |
 help: consider cloning the value before moving it into the closure

tests/ui/borrowck/issue-103624.stderr

@@ -2,13 +2,16 @@ error[E0507]: cannot move out of `self.b`, as `self` is a captured variable in a
   --> $DIR/issue-103624.rs:16:13
    |
 LL |     async fn foo(&self) {
-   |                  ----- captured outer variable
+   |                  -----
+   |                  |
+   |                  captured outer variable
+   |                  move occurs because `self.b` has type `StructB`, which does not implement the `Copy` trait
 LL |         let bar = self.b.bar().await;
 LL |         spawn_blocking(move || {
    |                        ------- captured by this `Fn` closure
 LL |
 LL |             self.b;
-   |             ^^^^^^ move occurs because `self.b` has type `StructB`, which does not implement the `Copy` trait
+   |             ^^^^^^ `self.b` is moved here
    |
 note: if `StructB` implemented `Clone`, you could clone the value
   --> $DIR/issue-103624.rs:23:1

tests/ui/issues/issue-4335.stderr

@@ -2,9 +2,11 @@ error[E0507]: cannot move out of `*v`, as `v` is a captured variable in an `FnMu
   --> $DIR/issue-4335.rs:6:20
    |
 LL | fn f<'r, T>(v: &'r T) -> Box<dyn FnMut() -> T + 'r> {
-   |             - captured outer variable
+   |             -  ----- move occurs because `*v` has type `T`, which does not implement the `Copy` trait
+   |             |
+   |             captured outer variable
 LL |     id(Box::new(|| *v))
-   |                 -- ^^ move occurs because `*v` has type `T`, which does not implement the `Copy` trait
+   |                 -- ^^ `*v` is moved here
    |                 |
    |                 captured by this `FnMut` closure
    |

tests/ui/moves/moves-based-on-type-move-out-of-closure-env-issue-1965.stderr

@@ -2,9 +2,11 @@ error[E0507]: cannot move out of `i`, a captured variable in an `Fn` closure
   --> $DIR/moves-based-on-type-move-out-of-closure-env-issue-1965.rs:9:28
    |
 LL |     let i = Box::new(3);
-   |         - captured outer variable
+   |         -   ----------- move occurs because `i` has type `Box<usize>`, which does not implement the `Copy` trait
+   |         |
+   |         captured outer variable
 LL |     let _f = to_fn(|| test(i));
-   |                    --      ^ move occurs because `i` has type `Box<usize>`, which does not implement the `Copy` trait
+   |                    --      ^ `i` is moved here
    |                    |
    |                    captured by this `Fn` closure
    |

tests/ui/nll/issue-52663-span-decl-captured-variable.stderr

@@ -2,9 +2,11 @@ error[E0507]: cannot move out of `x.0`, as `x` is a captured variable in an `Fn`
   --> $DIR/issue-52663-span-decl-captured-variable.rs:8:26
    |
 LL |        let x = (vec![22], vec![44]);
-   |            - captured outer variable
+   |            -   -------------------- move occurs because `x.0` has type `Vec<i32>`, which does not implement the `Copy` trait
+   |            |
+   |            captured outer variable
 LL |        expect_fn(|| drop(x.0));
-   |                  --      ^^^ move occurs because `x.0` has type `Vec<i32>`, which does not implement the `Copy` trait
+   |                  --      ^^^ `x.0` is moved here
    |                  |
    |                  captured by this `Fn` closure
    |

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

@@ -2,35 +2,33 @@ error[E0507]: cannot move out of `var`, a captured variable in an `FnMut` closur
   --> $DIR/option-content-move2.rs:11:9
    |
 LL |     let mut var = None;
-   |         ------- captured outer variable
+   |         -------   ---- move occurs because `var` has type `Option<NotCopyable>`, which does not implement the `Copy` trait
+   |         |
+   |         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
+   |             --- variable moved due to use in closure
 
 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
+   |         -------   ---- move occurs because `var` has type `Option<NotCopyableButCloneable>`, which does not implement the `Copy` trait
+   |         |
+   |         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
+   |             --- variable moved due to use in closure
 
 error: aborting due to 2 previous errors