From 5f973264bdcd75b8d0cc3613dcfe5b7608e435d3 Mon Sep 17 00:00:00 2001 From: Dhruv Jauhar Date: Sun, 1 Nov 2020 02:15:22 -0500 Subject: [PATCH] Provide diagnostic suggestion in ExprUseVisitor Delegate The [Delegate trait](https://github.com/rust-lang/rust/blob/981346fc07dd5ef414c5b1b21999f7604cece006/compiler/rustc_typeck/src/expr_use_visitor.rs#L28-L38) currently use `PlaceWithHirId` which is composed of Hir `Place` and the corresponding expression id. Even though this is an accurate way of expressing how a Place is used, it can cause confusion during diagnostics. Eg: ``` let arr : [String; 5]; let [a, ...] = arr; ^^^ E1 ^^^ = ^^E2^^ ``` Here `arr` is moved because of the binding created E1. However, when we point to E1 in diagnostics with the message `arr` was moved, it can be confusing. Rather we would like to report E2 to the user. Closes: https://github.com/rust-lang/project-rfc-2229/issues/20 --- clippy_lints/src/escape.rs | 6 +++--- clippy_lints/src/loops.rs | 14 +++++++------- clippy_lints/src/needless_pass_by_value.rs | 6 +++--- clippy_lints/src/utils/usage.rs | 6 +++--- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/clippy_lints/src/escape.rs b/clippy_lints/src/escape.rs index 8b022912573..eebf4a81a87 100644 --- a/clippy_lints/src/escape.rs +++ b/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/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 4fdcaca8f60..7ea2d3d857b 100644 --- a/clippy_lints/src/loops.rs +++ b/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/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index 7e933c674dd..2423eb4e6e3 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/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/clippy_lints/src/utils/usage.rs b/clippy_lints/src/utils/usage.rs index ea1dc3be29b..a15b8621365 100644 --- a/clippy_lints/src/utils/usage.rs +++ b/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) } }