diff --git a/src/librustc_middle/ty/mod.rs b/src/librustc_middle/ty/mod.rs index 364059a5993be..b6300a40b0d8d 100644 --- a/src/librustc_middle/ty/mod.rs +++ b/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), /// Upvar is captured by reference. ByRef(UpvarBorrow<'tcx>), diff --git a/src/librustc_mir/borrow_check/diagnostics/mod.rs b/src/librustc_mir/borrow_check/diagnostics/mod.rs index e73a78811d490..dfaa75d9f23f8 100644 --- a/src/librustc_mir/borrow_check/diagnostics/mod.rs +++ b/src/librustc_mir/borrow_check/diagnostics/mod.rs @@ -938,11 +938,14 @@ 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() => @@ -950,7 +953,23 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { 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)); } _ => {} } diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index b486b8b589cfd..86908eaabd1f4 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/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 { diff --git a/src/librustc_mir_build/build/mod.rs b/src/librustc_mir_build/build/mod.rs index 71026f5096df6..249cce0ba1994 100644 --- a/src/librustc_mir_build/build/mod.rs +++ b/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); } diff --git a/src/librustc_mir_build/thir/cx/expr.rs b/src/librustc_mir_build/thir/cx/expr.rs index c51c3bcf56288..066e46fec1456 100644 --- a/src/librustc_mir_build/thir/cx/expr.rs +++ b/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, diff --git a/src/librustc_passes/liveness.rs b/src/librustc_passes/liveness.rs index de21f0b5e09b3..55525586479d9 100644 --- a/src/librustc_passes/liveness.rs +++ b/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) { diff --git a/src/librustc_typeck/check/regionck.rs b/src/librustc_typeck/check/regionck.rs index 221e5f72dc977..484961dbdb8f2 100644 --- a/src/librustc_typeck/check/regionck.rs +++ b/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); diff --git a/src/librustc_typeck/check/upvar.rs b/src/librustc_typeck/check/upvar.rs index 030c0ab668a80..9bb84c0786847 100644 --- a/src/librustc_typeck/check/upvar.rs +++ b/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) => { diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index 50e2d6a94bb71..67f67e64dd47b 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/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, diff --git a/src/librustc_typeck/expr_use_visitor.rs b/src/librustc_typeck/expr_use_visitor.rs index d1b386c9d4d47..e774f2d095d9c 100644 --- a/src/librustc_typeck/expr_use_visitor.rs +++ b/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); } diff --git a/src/test/ui/moves/issue-75904-move-closure-loop.rs b/src/test/ui/moves/issue-75904-move-closure-loop.rs new file mode 100644 index 0000000000000..6641a0376c6ae --- /dev/null +++ b/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; + }; + } +} diff --git a/src/test/ui/moves/issue-75904-move-closure-loop.stderr b/src/test/ui/moves/issue-75904-move-closure-loop.stderr new file mode 100644 index 0000000000000..5e427a1fcdc7d --- /dev/null +++ b/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`.