diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 1e97bd65a79f4..e9dfef718fde9 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -279,11 +279,12 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { fn adjust_upvar_borrow_kind_for_consume( &mut self, place_with_id: &PlaceWithHirId<'tcx>, + diag_expr_id: hir::HirId, mode: euv::ConsumeMode, ) { debug!( - "adjust_upvar_borrow_kind_for_consume(place_with_id={:?}, mode={:?})", - place_with_id, mode + "adjust_upvar_borrow_kind_for_consume(place_with_id={:?}, diag_expr_id={:?}, mode={:?})", + place_with_id, diag_expr_id, mode ); // we only care about moves @@ -303,7 +304,7 @@ 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); + let usage_span = tcx.hir().span(diag_expr_id); // To move out of an upvar, this must be a FnOnce closure self.adjust_closure_kind( @@ -313,14 +314,7 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { var_name(tcx, upvar_id.var_path.hir_id), ); - // 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); + let new_capture = ty::UpvarCapture::ByValue(Some(usage_span)); match self.adjust_upvar_captures.entry(upvar_id) { Entry::Occupied(mut e) => { match e.get() { @@ -345,8 +339,15 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { /// Indicates that `place_with_id` is being directly mutated (e.g., assigned /// to). If the place is based on a by-ref upvar, this implies that /// the upvar must be borrowed using an `&mut` borrow. - fn adjust_upvar_borrow_kind_for_mut(&mut self, place_with_id: &PlaceWithHirId<'tcx>) { - debug!("adjust_upvar_borrow_kind_for_mut(place_with_id={:?})", place_with_id); + fn adjust_upvar_borrow_kind_for_mut( + &mut self, + place_with_id: &PlaceWithHirId<'tcx>, + diag_expr_id: hir::HirId, + ) { + debug!( + "adjust_upvar_borrow_kind_for_mut(place_with_id={:?}, diag_expr_id={:?})", + place_with_id, diag_expr_id + ); if let PlaceBase::Upvar(upvar_id) = place_with_id.place.base { let mut borrow_kind = ty::MutBorrow; @@ -362,16 +363,19 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { _ => (), } } - self.adjust_upvar_deref( - upvar_id, - self.fcx.tcx.hir().span(place_with_id.hir_id), - borrow_kind, - ); + self.adjust_upvar_deref(upvar_id, self.fcx.tcx.hir().span(diag_expr_id), borrow_kind); } } - fn adjust_upvar_borrow_kind_for_unique(&mut self, place_with_id: &PlaceWithHirId<'tcx>) { - debug!("adjust_upvar_borrow_kind_for_unique(place_with_id={:?})", place_with_id); + fn adjust_upvar_borrow_kind_for_unique( + &mut self, + place_with_id: &PlaceWithHirId<'tcx>, + diag_expr_id: hir::HirId, + ) { + debug!( + "adjust_upvar_borrow_kind_for_unique(place_with_id={:?}, diag_expr_id={:?})", + place_with_id, diag_expr_id + ); if let PlaceBase::Upvar(upvar_id) = place_with_id.place.base { if place_with_id.place.deref_tys().any(ty::TyS::is_unsafe_ptr) { @@ -381,7 +385,7 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { // for a borrowed pointer to be unique, its base must be unique self.adjust_upvar_deref( upvar_id, - self.fcx.tcx.hir().span(place_with_id.hir_id), + self.fcx.tcx.hir().span(diag_expr_id), ty::UniqueImmBorrow, ); } @@ -500,29 +504,44 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { } impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> { - fn consume(&mut self, place_with_id: &PlaceWithHirId<'tcx>, mode: euv::ConsumeMode) { - debug!("consume(place_with_id={:?},mode={:?})", place_with_id, mode); - self.adjust_upvar_borrow_kind_for_consume(place_with_id, mode); + fn consume( + &mut self, + place_with_id: &PlaceWithHirId<'tcx>, + diag_expr_id: hir::HirId, + mode: euv::ConsumeMode, + ) { + debug!( + "consume(place_with_id={:?}, diag_expr_id={:?}, mode={:?})", + place_with_id, diag_expr_id, mode + ); + self.adjust_upvar_borrow_kind_for_consume(&place_with_id, diag_expr_id, mode); } - fn borrow(&mut self, place_with_id: &PlaceWithHirId<'tcx>, bk: ty::BorrowKind) { - debug!("borrow(place_with_id={:?}, bk={:?})", place_with_id, bk); + fn borrow( + &mut self, + place_with_id: &PlaceWithHirId<'tcx>, + diag_expr_id: hir::HirId, + bk: ty::BorrowKind, + ) { + debug!( + "borrow(place_with_id={:?}, diag_expr_id={:?}, bk={:?})", + place_with_id, diag_expr_id, bk + ); match bk { ty::ImmBorrow => {} ty::UniqueImmBorrow => { - self.adjust_upvar_borrow_kind_for_unique(place_with_id); + self.adjust_upvar_borrow_kind_for_unique(&place_with_id, diag_expr_id); } ty::MutBorrow => { - self.adjust_upvar_borrow_kind_for_mut(place_with_id); + self.adjust_upvar_borrow_kind_for_mut(&place_with_id, diag_expr_id); } } } - fn mutate(&mut self, assignee_place: &PlaceWithHirId<'tcx>) { - debug!("mutate(assignee_place={:?})", assignee_place); - - self.adjust_upvar_borrow_kind_for_mut(assignee_place); + fn mutate(&mut self, assignee_place: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId) { + debug!("mutate(assignee_place={:?}, diag_expr_id={:?})", assignee_place, diag_expr_id); + self.adjust_upvar_borrow_kind_for_mut(assignee_place, diag_expr_id); } } diff --git a/compiler/rustc_typeck/src/expr_use_visitor.rs b/compiler/rustc_typeck/src/expr_use_visitor.rs index 471909a092f7b..57bd89b9d3da9 100644 --- a/compiler/rustc_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_typeck/src/expr_use_visitor.rs @@ -27,14 +27,31 @@ use rustc_span::Span; /// employing the ExprUseVisitor. pub trait Delegate<'tcx> { // The value found at `place` is either copied or moved, depending - // on mode. - fn consume(&mut self, place_with_id: &PlaceWithHirId<'tcx>, mode: ConsumeMode); + // on `mode`. Where `diag_expr_id` is the id used for diagnostics for `place`. + // + // The parameter `diag_expr_id` indicates the HIR id that ought to be used for + // diagnostics. Around pattern matching such as `let pat = expr`, the diagnostic + // id will be the id of the expression `expr` but the place itself will have + // the id of the binding in the pattern `pat`. + fn consume( + &mut self, + place_with_id: &PlaceWithHirId<'tcx>, + diag_expr_id: hir::HirId, + mode: ConsumeMode, + ); // The value found at `place` is being borrowed with kind `bk`. - fn borrow(&mut self, place_with_id: &PlaceWithHirId<'tcx>, bk: ty::BorrowKind); - - // The path at `place_with_id` is being assigned to. - fn mutate(&mut self, assignee_place: &PlaceWithHirId<'tcx>); + // `diag_expr_id` is the id used for diagnostics (see `consume` for more details). + fn borrow( + &mut self, + place_with_id: &PlaceWithHirId<'tcx>, + diag_expr_id: hir::HirId, + bk: ty::BorrowKind, + ); + + // The path at `assignee_place` is being assigned to. + // `diag_expr_id` is the id used for diagnostics (see `consume` for more details). + fn mutate(&mut self, assignee_place: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId); } #[derive(Copy, Clone, PartialEq, Debug)] @@ -116,11 +133,11 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { self.mc.tcx() } - fn delegate_consume(&mut self, place_with_id: &PlaceWithHirId<'tcx>) { + fn delegate_consume(&mut self, place_with_id: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId) { debug!("delegate_consume(place_with_id={:?})", place_with_id); let mode = copy_or_move(&self.mc, place_with_id); - self.delegate.consume(place_with_id, mode); + self.delegate.consume(place_with_id, diag_expr_id, mode); } fn consume_exprs(&mut self, exprs: &[hir::Expr<'_>]) { @@ -133,13 +150,13 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { debug!("consume_expr(expr={:?})", expr); let place_with_id = return_if_err!(self.mc.cat_expr(expr)); - self.delegate_consume(&place_with_id); + self.delegate_consume(&place_with_id, place_with_id.hir_id); self.walk_expr(expr); } fn mutate_expr(&mut self, expr: &hir::Expr<'_>) { let place_with_id = return_if_err!(self.mc.cat_expr(expr)); - self.delegate.mutate(&place_with_id); + self.delegate.mutate(&place_with_id, place_with_id.hir_id); self.walk_expr(expr); } @@ -147,7 +164,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { debug!("borrow_expr(expr={:?}, bk={:?})", expr, bk); let place_with_id = return_if_err!(self.mc.cat_expr(expr)); - self.delegate.borrow(&place_with_id, bk); + self.delegate.borrow(&place_with_id, place_with_id.hir_id, bk); self.walk_expr(expr) } @@ -404,7 +421,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { with_field.ty(self.tcx(), substs), ProjectionKind::Field(f_index as u32, VariantIdx::new(0)), ); - self.delegate_consume(&field_place); + self.delegate_consume(&field_place, field_place.hir_id); } } } @@ -436,7 +453,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { adjustment::Adjust::NeverToAny | adjustment::Adjust::Pointer(_) => { // Creating a closure/fn-pointer or unsizing consumes // the input and stores it into the resulting rvalue. - self.delegate_consume(&place_with_id); + self.delegate_consume(&place_with_id, place_with_id.hir_id); } adjustment::Adjust::Deref(None) => {} @@ -448,7 +465,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { // this is an autoref of `x`. adjustment::Adjust::Deref(Some(ref deref)) => { let bk = ty::BorrowKind::from_mutbl(deref.mutbl); - self.delegate.borrow(&place_with_id, bk); + self.delegate.borrow(&place_with_id, place_with_id.hir_id, bk); } adjustment::Adjust::Borrow(ref autoref) => { @@ -476,13 +493,17 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { match *autoref { adjustment::AutoBorrow::Ref(_, m) => { - self.delegate.borrow(base_place, ty::BorrowKind::from_mutbl(m.into())); + self.delegate.borrow( + base_place, + base_place.hir_id, + ty::BorrowKind::from_mutbl(m.into()), + ); } adjustment::AutoBorrow::RawPtr(m) => { debug!("walk_autoref: expr.hir_id={} base_place={:?}", expr.hir_id, base_place); - self.delegate.borrow(base_place, ty::BorrowKind::from_mutbl(m)); + self.delegate.borrow(base_place, base_place.hir_id, ty::BorrowKind::from_mutbl(m)); } } } @@ -525,19 +546,22 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { // binding being produced. let def = Res::Local(canonical_id); if let Ok(ref binding_place) = mc.cat_res(pat.hir_id, pat.span, pat_ty, def) { - delegate.mutate(binding_place); + delegate.mutate(binding_place, binding_place.hir_id); } // It is also a borrow or copy/move of the value being matched. + // In a cases of pattern like `let pat = upvar`, don't use the span + // of the pattern, as this just looks confusing, instead use the span + // of the discriminant. match bm { ty::BindByReference(m) => { let bk = ty::BorrowKind::from_mutbl(m); - delegate.borrow(place, bk); + delegate.borrow(place, discr_place.hir_id, bk); } ty::BindByValue(..) => { - let mode = copy_or_move(mc, place); + let mode = copy_or_move(mc, &place); debug!("walk_pat binding consuming pat"); - delegate.consume(place, mode); + delegate.consume(place, discr_place.hir_id, mode); } } } @@ -564,10 +588,14 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { match upvar_capture { ty::UpvarCapture::ByValue(_) => { let mode = copy_or_move(&self.mc, &captured_place); - self.delegate.consume(&captured_place, mode); + self.delegate.consume(&captured_place, captured_place.hir_id, mode); } ty::UpvarCapture::ByRef(upvar_borrow) => { - self.delegate.borrow(&captured_place, upvar_borrow.kind); + self.delegate.borrow( + &captured_place, + captured_place.hir_id, + upvar_borrow.kind, + ); } } } diff --git a/src/test/ui/borrowck/borrowck-closures-slice-patterns.stderr b/src/test/ui/borrowck/borrowck-closures-slice-patterns.stderr index 7f6c764ec2241..9e1e47a92412a 100644 --- a/src/test/ui/borrowck/borrowck-closures-slice-patterns.stderr +++ b/src/test/ui/borrowck/borrowck-closures-slice-patterns.stderr @@ -75,7 +75,7 @@ LL | fn arr_box_by_move(x: Box<[String; 3]>) { LL | let f = || { | -- value moved into closure here LL | let [y, z @ ..] = *x; - | - variable moved due to use in closure + | -- variable moved due to use in closure LL | }; LL | &x; | ^^ value borrowed here after move diff --git a/src/tools/clippy/clippy_lints/src/escape.rs b/src/tools/clippy/clippy_lints/src/escape.rs index 8b0229125738a..eebf4a81a8716 100644 --- a/src/tools/clippy/clippy_lints/src/escape.rs +++ b/src/tools/clippy/clippy_lints/src/escape.rs @@ -116,7 +116,7 @@ fn is_argument(map: rustc_middle::hir::map::Map<'_>, id: HirId) -> bool { } impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { - fn consume(&mut self, cmt: &PlaceWithHirId<'tcx>, mode: ConsumeMode) { + fn consume(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId, mode: ConsumeMode) { if cmt.place.projections.is_empty() { if let PlaceBase::Local(lid) = cmt.place.base { if let ConsumeMode::Move = mode { @@ -136,7 +136,7 @@ impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { } } - fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, _: ty::BorrowKind) { + fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId, _: ty::BorrowKind) { if cmt.place.projections.is_empty() { if let PlaceBase::Local(lid) = cmt.place.base { self.set.remove(&lid); @@ -144,7 +144,7 @@ impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { } } - fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>) { + fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId) { if cmt.place.projections.is_empty() { let map = &self.cx.tcx.hir(); if is_argument(*map, cmt.hir_id) { diff --git a/src/tools/clippy/clippy_lints/src/loops.rs b/src/tools/clippy/clippy_lints/src/loops.rs index 4fdcaca8f6039..7ea2d3d857b35 100644 --- a/src/tools/clippy/clippy_lints/src/loops.rs +++ b/src/tools/clippy/clippy_lints/src/loops.rs @@ -1694,28 +1694,28 @@ struct MutatePairDelegate<'a, 'tcx> { } impl<'tcx> Delegate<'tcx> for MutatePairDelegate<'_, 'tcx> { - fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: ConsumeMode) {} + fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId, _: ConsumeMode) {} - fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, bk: ty::BorrowKind) { + fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, diag_expr_id: HirId, bk: ty::BorrowKind) { if let ty::BorrowKind::MutBorrow = bk { if let PlaceBase::Local(id) = cmt.place.base { if Some(id) == self.hir_id_low { - self.span_low = Some(self.cx.tcx.hir().span(cmt.hir_id)) + self.span_low = Some(self.cx.tcx.hir().span(diag_expr_id)) } if Some(id) == self.hir_id_high { - self.span_high = Some(self.cx.tcx.hir().span(cmt.hir_id)) + self.span_high = Some(self.cx.tcx.hir().span(diag_expr_id)) } } } } - fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>) { + fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>, diag_expr_id: HirId) { if let PlaceBase::Local(id) = cmt.place.base { if Some(id) == self.hir_id_low { - self.span_low = Some(self.cx.tcx.hir().span(cmt.hir_id)) + self.span_low = Some(self.cx.tcx.hir().span(diag_expr_id)) } if Some(id) == self.hir_id_high { - self.span_high = Some(self.cx.tcx.hir().span(cmt.hir_id)) + self.span_high = Some(self.cx.tcx.hir().span(diag_expr_id)) } } } diff --git a/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs index 7e933c674dd78..2423eb4e6e394 100644 --- a/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs +++ b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs @@ -325,13 +325,13 @@ impl MovedVariablesCtxt { } impl<'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt { - fn consume(&mut self, cmt: &euv::PlaceWithHirId<'tcx>, mode: euv::ConsumeMode) { + fn consume(&mut self, cmt: &euv::PlaceWithHirId<'tcx>, _: HirId, mode: euv::ConsumeMode) { if let euv::ConsumeMode::Move = mode { self.move_common(cmt); } } - fn borrow(&mut self, _: &euv::PlaceWithHirId<'tcx>, _: ty::BorrowKind) {} + fn borrow(&mut self, _: &euv::PlaceWithHirId<'tcx>, _: HirId, _: ty::BorrowKind) {} - fn mutate(&mut self, _: &euv::PlaceWithHirId<'tcx>) {} + fn mutate(&mut self, _: &euv::PlaceWithHirId<'tcx>, _: HirId) {} } diff --git a/src/tools/clippy/clippy_lints/src/utils/usage.rs b/src/tools/clippy/clippy_lints/src/utils/usage.rs index ea1dc3be29ba0..a15b8621365ef 100644 --- a/src/tools/clippy/clippy_lints/src/utils/usage.rs +++ b/src/tools/clippy/clippy_lints/src/utils/usage.rs @@ -68,15 +68,15 @@ impl<'tcx> MutVarsDelegate { } impl<'tcx> Delegate<'tcx> for MutVarsDelegate { - fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: ConsumeMode) {} + fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId, _: ConsumeMode) {} - fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, bk: ty::BorrowKind) { + fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId, bk: ty::BorrowKind) { if let ty::BorrowKind::MutBorrow = bk { self.update(&cmt) } } - fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>) { + fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId) { self.update(&cmt) } }