Point to a move-related span when pointing to closure upvars

src/librustc_middle/ty/mod.rs

@@ -721,7 +721,13 @@ pub enum UpvarCapture<'tcx> {
     /// Upvar is captured by value. This is always true when the
     /// closure is labeled `move`, but can also be true in other cases
     /// depending on inference.
-    ByValue,
+    ///
+    /// If the upvar was inferred to be captured by value (e.g. `move`
+    /// was not used), then the `Span` points to a usage that
+    /// required it. There may be more than one such usage
+    /// (e.g. `|| { a; a; }`), in which case we pick an
+    /// arbitrary one.
+    ByValue(Option<Span>),
 
     /// Upvar is captured by reference.
     ByRef(UpvarBorrow<'tcx>),

src/librustc_mir/borrow_check/diagnostics/mod.rs

@@ -938,19 +938,38 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             "closure_span: def_id={:?} target_place={:?} places={:?}",
             def_id, target_place, places
         );
-        let hir_id = self.infcx.tcx.hir().local_def_id_to_hir_id(def_id.as_local()?);
+        let local_did = def_id.as_local()?;
+        let hir_id = self.infcx.tcx.hir().local_def_id_to_hir_id(local_did);
         let expr = &self.infcx.tcx.hir().expect_expr(hir_id).kind;
         debug!("closure_span: hir_id={:?} expr={:?}", hir_id, expr);
         if let hir::ExprKind::Closure(.., body_id, args_span, _) = expr {
-            for (upvar, place) in self.infcx.tcx.upvars_mentioned(def_id)?.values().zip(places) {
+            for ((upvar_hir_id, upvar), place) in
+                self.infcx.tcx.upvars_mentioned(def_id)?.iter().zip(places)
+            {
                 match place {
                     Operand::Copy(place) | Operand::Move(place)
                         if target_place == place.as_ref() =>
                     {
                         debug!("closure_span: found captured local {:?}", place);
                         let body = self.infcx.tcx.hir().body(*body_id);
                         let generator_kind = body.generator_kind();
-                        return Some((*args_span, generator_kind, upvar.span));
+                        let upvar_id = ty::UpvarId {
+                            var_path: ty::UpvarPath { hir_id: *upvar_hir_id },
+                            closure_expr_id: local_did,
+                        };
+
+                        // If we have a more specific span available, point to that.
+                        // We do this even though this span might be part of a borrow error
+                        // message rather than a move error message. Our goal is to point
+                        // to a span that shows why the upvar is used in the closure,
+                        // so a move-related span is as good as any (and potentially better,
+                        // if the overall error is due to a move of the upvar).
+                        let usage_span =
+                            match self.infcx.tcx.typeck(local_did).upvar_capture(upvar_id) {
+                                ty::UpvarCapture::ByValue(Some(span)) => span,
+                                _ => upvar.span,
+                            };
+                        return Some((*args_span, generator_kind, usage_span));
                     }
                     _ => {}
                 }

src/librustc_mir/borrow_check/mod.rs

@@ -163,7 +163,7 @@ fn do_mir_borrowck<'a, 'tcx>(
             let var_hir_id = upvar_id.var_path.hir_id;
             let capture = tables.upvar_capture(*upvar_id);
             let by_ref = match capture {
-                ty::UpvarCapture::ByValue => false,
+                ty::UpvarCapture::ByValue(_) => false,
                 ty::UpvarCapture::ByRef(..) => true,
             };
             let mut upvar = Upvar {

src/librustc_mir_build/build/mod.rs

@@ -876,7 +876,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     let mut projs = closure_env_projs.clone();
                     projs.push(ProjectionElem::Field(Field::new(i), ty));
                     match capture {
-                        ty::UpvarCapture::ByValue => {}
+                        ty::UpvarCapture::ByValue(_) => {}
                         ty::UpvarCapture::ByRef(..) => {
                             projs.push(ProjectionElem::Deref);
                         }

src/librustc_mir_build/thir/cx/expr.rs

@@ -959,7 +959,7 @@ fn convert_var<'tcx>(
             // ...but the upvar might be an `&T` or `&mut T` capture, at which
             // point we need an implicit deref
             match cx.typeck_results().upvar_capture(upvar_id) {
-                ty::UpvarCapture::ByValue => field_kind,
+                ty::UpvarCapture::ByValue(_) => field_kind,
                 ty::UpvarCapture::ByRef(borrow) => ExprKind::Deref {
                     arg: Expr {
                         temp_lifetime,
@@ -1074,7 +1074,7 @@ fn capture_upvar<'tcx>(
         kind: convert_var(cx, closure_expr, var_hir_id),
     };
     match upvar_capture {
-        ty::UpvarCapture::ByValue => captured_var.to_ref(),
+        ty::UpvarCapture::ByValue(_) => captured_var.to_ref(),
         ty::UpvarCapture::ByRef(upvar_borrow) => {
             let borrow_kind = match upvar_borrow.kind {
                 ty::BorrowKind::ImmBorrow => BorrowKind::Shared,

src/librustc_passes/liveness.rs

@@ -945,7 +945,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
                         let var = self.variable(var_hir_id, upvar.span);
                         self.acc(self.s.exit_ln, var, ACC_READ | ACC_USE);
                     }
-                    ty::UpvarCapture::ByValue => {}
+                    ty::UpvarCapture::ByValue(_) => {}
                 }
             }
         }
@@ -1610,7 +1610,7 @@ impl<'tcx> Liveness<'_, 'tcx> {
                 closure_expr_id: self.ir.body_owner,
             };
             match self.typeck_results.upvar_capture(upvar_id) {
-                ty::UpvarCapture::ByValue => {}
+                ty::UpvarCapture::ByValue(_) => {}
                 ty::UpvarCapture::ByRef(..) => continue,
             };
             if self.used_on_entry(entry_ln, var) {

src/librustc_typeck/check/regionck.rs

@@ -786,7 +786,7 @@ impl<'a, 'tcx> RegionCtxt<'a, 'tcx> {
                     return;
                 }
             }
-            ty::UpvarCapture::ByValue => {}
+            ty::UpvarCapture::ByValue(_) => {}
         }
         let fn_hir_id = self.tcx.hir().local_def_id_to_hir_id(upvar_id.closure_expr_id);
         let ty = self.resolve_node_type(fn_hir_id);

src/librustc_typeck/check/upvar.rs

@@ -42,6 +42,7 @@ use rustc_infer::infer::UpvarRegion;
 use rustc_middle::hir::place::{PlaceBase, PlaceWithHirId};
 use rustc_middle::ty::{self, Ty, TyCtxt, UpvarSubsts};
 use rustc_span::{Span, Symbol};
+use std::collections::hash_map::Entry;
 
 impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     pub fn closure_analyze(&self, body: &'tcx hir::Body<'tcx>) {
@@ -124,7 +125,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 closure_captures.insert(var_hir_id, upvar_id);
 
                 let capture_kind = match capture_clause {
-                    hir::CaptureBy::Value => ty::UpvarCapture::ByValue,
+                    hir::CaptureBy::Value => ty::UpvarCapture::ByValue(None),
                     hir::CaptureBy::Ref => {
                         let origin = UpvarRegion(upvar_id, span);
                         let upvar_region = self.next_region_var(origin);
@@ -237,7 +238,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     debug!("var_id={:?} upvar_ty={:?} capture={:?}", var_hir_id, upvar_ty, capture);
 
                     match capture {
-                        ty::UpvarCapture::ByValue => upvar_ty,
+                        ty::UpvarCapture::ByValue(_) => upvar_ty,
                         ty::UpvarCapture::ByRef(borrow) => tcx.mk_ref(
                             borrow.region,
                             ty::TypeAndMut { ty: upvar_ty, mutbl: borrow.kind.to_mutbl_lossy() },
@@ -300,15 +301,43 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
 
         debug!("adjust_upvar_borrow_kind_for_consume: upvar={:?}", upvar_id);
 
+        let usage_span = tcx.hir().span(place_with_id.hir_id);
+
         // To move out of an upvar, this must be a FnOnce closure
         self.adjust_closure_kind(
             upvar_id.closure_expr_id,
             ty::ClosureKind::FnOnce,
-            tcx.hir().span(place_with_id.hir_id),
+            usage_span,
             var_name(tcx, upvar_id.var_path.hir_id),
         );
 
-        self.adjust_upvar_captures.insert(upvar_id, ty::UpvarCapture::ByValue);
+        // In a case like `let pat = upvar`, don't use the span
+        // of the pattern, as this just looks confusing.
+        let by_value_span = match tcx.hir().get(place_with_id.hir_id) {
+            hir::Node::Pat(_) => None,
+            _ => Some(usage_span),
+        };
+
+        let new_capture = ty::UpvarCapture::ByValue(by_value_span);
+        match self.adjust_upvar_captures.entry(upvar_id) {
+            Entry::Occupied(mut e) => {
+                match e.get() {
+                    // We always overwrite `ByRef`, since we require
+                    // that the upvar be available by value.
+                    //
+                    // If we had a previous by-value usage without a specific
+                    // span, use ours instead. Otherwise, keep the first span
+                    // we encountered, since there isn't an obviously better one.
+                    ty::UpvarCapture::ByRef(_) | ty::UpvarCapture::ByValue(None) => {
+                        e.insert(new_capture);
+                    }
+                    _ => {}
+                }
+            }
+            Entry::Vacant(e) => {
+                e.insert(new_capture);
+            }
+        }
     }
 
     /// Indicates that `place_with_id` is being directly mutated (e.g., assigned
@@ -404,7 +433,7 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
         );
 
         match upvar_capture {
-            ty::UpvarCapture::ByValue => {
+            ty::UpvarCapture::ByValue(_) => {
                 // Upvar is already by-value, the strongest criteria.
             }
             ty::UpvarCapture::ByRef(mut upvar_borrow) => {

src/librustc_typeck/check/writeback.rs

@@ -331,7 +331,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
     fn visit_upvar_capture_map(&mut self) {
         for (upvar_id, upvar_capture) in self.fcx.typeck_results.borrow().upvar_capture_map.iter() {
             let new_upvar_capture = match *upvar_capture {
-                ty::UpvarCapture::ByValue => ty::UpvarCapture::ByValue,
+                ty::UpvarCapture::ByValue(span) => ty::UpvarCapture::ByValue(span),
                 ty::UpvarCapture::ByRef(ref upvar_borrow) => {
                     ty::UpvarCapture::ByRef(ty::UpvarBorrow {
                         kind: upvar_borrow.kind,

src/librustc_typeck/expr_use_visitor.rs

@@ -559,7 +559,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
                     var_id,
                 ));
                 match upvar_capture {
-                    ty::UpvarCapture::ByValue => {
+                    ty::UpvarCapture::ByValue(_) => {
                         let mode = copy_or_move(&self.mc, &captured_place);
                         self.delegate.consume(&captured_place, mode);
                     }

src/test/ui/moves/issue-75904-move-closure-loop.rs

@@ -0,0 +1,15 @@
+// Regression test for issue #75904
+// Tests that we point at an expression
+// that required the upvar to be moved, rather than just borrowed.
+
+struct NotCopy;
+
+fn main() {
+    let mut a = NotCopy;
+    loop {
+        || { //~ ERROR use of moved value
+            &mut a;
+            a;
+        };
+    }
+}

src/test/ui/moves/issue-75904-move-closure-loop.stderr

@@ -0,0 +1,15 @@
+error[E0382]: use of moved value: `a`
+  --> $DIR/issue-75904-move-closure-loop.rs:10:9
+   |
+LL |     let mut a = NotCopy;
+   |         ----- move occurs because `a` has type `NotCopy`, which does not implement the `Copy` trait
+LL |     loop {
+LL |         || {
+   |         ^^ value moved into closure here, in previous iteration of loop
+LL |             &mut a;
+LL |             a;
+   |             - use occurs due to use in closure
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0382`.