From 4821b84b92a2f2bbed06191cab5cc44693e0652b Mon Sep 17 00:00:00 2001 From: surechen Date: Wed, 10 Jul 2024 20:27:38 +0800 Subject: [PATCH] If the moved value is a mut reference, it is used in a generic function and it's type is a generic param, it can be reborrowed to avoid moving. for example: ```rust struct Y(u32); // x's type is '& mut Y' and it is used in `fn generic(x: T) {}`. fn generic(x: T) {} ``` fixes #127285 --- .../src/diagnostics/conflict_errors.rs | 76 +++++++++++++++---- .../rustc_borrowck/src/diagnostics/mod.rs | 25 +++--- .../src/diagnostics/move_errors.rs | 1 + ...-value-suggest-reborrow-issue-127285.fixed | 17 +++++ ...ved-value-suggest-reborrow-issue-127285.rs | 17 +++++ ...value-suggest-reborrow-issue-127285.stderr | 18 +++++ tests/ui/borrowck/mut-borrow-in-loop-2.stderr | 15 +--- 7 files changed, 128 insertions(+), 41 deletions(-) create mode 100644 tests/ui/borrowck/moved-value-suggest-reborrow-issue-127285.fixed create mode 100644 tests/ui/borrowck/moved-value-suggest-reborrow-issue-127285.rs create mode 100644 tests/ui/borrowck/moved-value-suggest-reborrow-issue-127285.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index c7f6840e401c6..f7e4bba371220 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -205,9 +205,17 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { is_loop_move = true; } + let mut has_suggest_reborrow = false; if !seen_spans.contains(&move_span) { if !closure { - self.suggest_ref_or_clone(mpi, &mut err, &mut in_pattern, move_spans); + self.suggest_ref_or_clone( + mpi, + &mut err, + &mut in_pattern, + move_spans, + moved_place.as_ref(), + &mut has_suggest_reborrow, + ); } let msg_opt = CapturedMessageOpt { @@ -215,6 +223,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { is_loop_message, is_move_msg, is_loop_move, + has_suggest_reborrow, maybe_reinitialized_locations_is_empty: maybe_reinitialized_locations .is_empty(), }; @@ -259,17 +268,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { if is_loop_move & !in_pattern && !matches!(use_spans, UseSpans::ClosureUse { .. }) { if let ty::Ref(_, _, hir::Mutability::Mut) = ty.kind() { // We have a `&mut` ref, we need to reborrow on each iteration (#62112). - err.span_suggestion_verbose( - span.shrink_to_lo(), - format!( - "consider creating a fresh reborrow of {} here", - self.describe_place(moved_place) - .map(|n| format!("`{n}`")) - .unwrap_or_else(|| "the mutable reference".to_string()), - ), - "&mut *", - Applicability::MachineApplicable, - ); + self.suggest_reborrow(&mut err, span, moved_place); } } @@ -346,6 +345,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { err: &mut Diag<'infcx>, in_pattern: &mut bool, move_spans: UseSpans<'tcx>, + moved_place: PlaceRef<'tcx>, + has_suggest_reborrow: &mut bool, ) { let move_span = match move_spans { UseSpans::ClosureUse { capture_kind_span, .. } => capture_kind_span, @@ -435,20 +436,44 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { let parent = self.infcx.tcx.parent_hir_node(expr.hir_id); let (def_id, args, offset) = if let hir::Node::Expr(parent_expr) = parent && let hir::ExprKind::MethodCall(_, _, args, _) = parent_expr.kind - && let Some(def_id) = typeck.type_dependent_def_id(parent_expr.hir_id) { - (def_id.as_local(), args, 1) + (typeck.type_dependent_def_id(parent_expr.hir_id), args, 1) } else if let hir::Node::Expr(parent_expr) = parent && let hir::ExprKind::Call(call, args) = parent_expr.kind && let ty::FnDef(def_id, _) = typeck.node_type(call.hir_id).kind() { - (def_id.as_local(), args, 0) + (Some(*def_id), args, 0) } else { (None, &[][..], 0) }; + + // If the moved value is a mut reference, it is used in a + // generic function and it's type is a generic param, it can be + // reborrowed to avoid moving. + // for example: + // struct Y(u32); + // x's type is '& mut Y' and it is used in `fn generic(x: T) {}`. + if let Some(def_id) = def_id + && self.infcx.tcx.def_kind(def_id).is_fn_like() + && let Some(pos) = args.iter().position(|arg| arg.hir_id == expr.hir_id) + && let ty::Param(_) = + self.infcx.tcx.fn_sig(def_id).skip_binder().skip_binder().inputs() + [pos + offset] + .kind() + { + let place = &self.move_data.move_paths[mpi].place; + let ty = place.ty(self.body, self.infcx.tcx).ty; + if let ty::Ref(_, _, hir::Mutability::Mut) = ty.kind() { + *has_suggest_reborrow = true; + self.suggest_reborrow(err, expr.span, moved_place); + return; + } + } + let mut can_suggest_clone = true; if let Some(def_id) = def_id - && let node = self.infcx.tcx.hir_node_by_def_id(def_id) + && let Some(local_def_id) = def_id.as_local() + && let node = self.infcx.tcx.hir_node_by_def_id(local_def_id) && let Some(fn_sig) = node.fn_sig() && let Some(ident) = node.ident() && let Some(pos) = args.iter().position(|arg| arg.hir_id == expr.hir_id) @@ -622,6 +647,25 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { } } + pub fn suggest_reborrow( + &self, + err: &mut Diag<'infcx>, + span: Span, + moved_place: PlaceRef<'tcx>, + ) { + err.span_suggestion_verbose( + span.shrink_to_lo(), + format!( + "consider creating a fresh reborrow of {} here", + self.describe_place(moved_place) + .map(|n| format!("`{n}`")) + .unwrap_or_else(|| "the mutable reference".to_string()), + ), + "&mut *", + Applicability::MachineApplicable, + ); + } + fn report_use_of_uninitialized( &self, mpi: MovePathIndex, diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index b7fbb71a0cfa5..f97459d16bacf 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -768,10 +768,11 @@ struct CapturedMessageOpt { is_loop_message: bool, is_move_msg: bool, is_loop_move: bool, + has_suggest_reborrow: bool, maybe_reinitialized_locations_is_empty: bool, } -impl<'tcx> MirBorrowckCtxt<'_, '_, '_, 'tcx> { +impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { /// Finds the spans associated to a move or copy of move_place at location. pub(super) fn move_spans( &self, @@ -997,7 +998,7 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, '_, 'tcx> { #[allow(rustc::untranslatable_diagnostic)] // FIXME: make this translatable fn explain_captures( &mut self, - err: &mut Diag<'_>, + err: &mut Diag<'infcx>, span: Span, move_span: Span, move_spans: UseSpans<'tcx>, @@ -1009,6 +1010,7 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, '_, 'tcx> { is_loop_message, is_move_msg, is_loop_move, + has_suggest_reborrow, maybe_reinitialized_locations_is_empty, } = msg_opt; if let UseSpans::FnSelfUse { var_span, fn_call_span, fn_span, kind } = move_spans { @@ -1182,18 +1184,15 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, '_, 'tcx> { if let ty::Ref(_, _, hir::Mutability::Mut) = moved_place.ty(self.body, self.infcx.tcx).ty.kind() { - // If we are in a loop this will be suggested later. - if !is_loop_move { - err.span_suggestion_verbose( + // Suggest `reborrow` in other place for following situations: + // 1. If we are in a loop this will be suggested later. + // 2. If the moved value is a mut reference, it is used in a + // generic function and the corresponding arg's type is generic param. + if !is_loop_move && !has_suggest_reborrow { + self.suggest_reborrow( + err, move_span.shrink_to_lo(), - format!( - "consider creating a fresh reborrow of {} here", - self.describe_place(moved_place.as_ref()) - .map(|n| format!("`{n}`")) - .unwrap_or_else(|| "the mutable reference".to_string()), - ), - "&mut *", - Applicability::MachineApplicable, + moved_place.as_ref(), ); } } diff --git a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs index 4b6c1b29f285d..fcf23aa478555 100644 --- a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs @@ -554,6 +554,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { is_loop_message: false, is_move_msg: false, is_loop_move: false, + has_suggest_reborrow: false, maybe_reinitialized_locations_is_empty: true, }; if let Some(use_spans) = use_spans { diff --git a/tests/ui/borrowck/moved-value-suggest-reborrow-issue-127285.fixed b/tests/ui/borrowck/moved-value-suggest-reborrow-issue-127285.fixed new file mode 100644 index 0000000000000..cec52272feeb9 --- /dev/null +++ b/tests/ui/borrowck/moved-value-suggest-reborrow-issue-127285.fixed @@ -0,0 +1,17 @@ +//@ run-rustfix + +#![allow(dead_code)] + +struct X(u32); + +impl X { + fn f(&mut self) { + generic(&mut *self); + self.0 += 1; + //~^ ERROR: use of moved value: `self` [E0382] + } +} + +fn generic(_x: T) {} + +fn main() {} diff --git a/tests/ui/borrowck/moved-value-suggest-reborrow-issue-127285.rs b/tests/ui/borrowck/moved-value-suggest-reborrow-issue-127285.rs new file mode 100644 index 0000000000000..dd015697fdcdb --- /dev/null +++ b/tests/ui/borrowck/moved-value-suggest-reborrow-issue-127285.rs @@ -0,0 +1,17 @@ +//@ run-rustfix + +#![allow(dead_code)] + +struct X(u32); + +impl X { + fn f(&mut self) { + generic(self); + self.0 += 1; + //~^ ERROR: use of moved value: `self` [E0382] + } +} + +fn generic(_x: T) {} + +fn main() {} diff --git a/tests/ui/borrowck/moved-value-suggest-reborrow-issue-127285.stderr b/tests/ui/borrowck/moved-value-suggest-reborrow-issue-127285.stderr new file mode 100644 index 0000000000000..3da8b6e9dff7f --- /dev/null +++ b/tests/ui/borrowck/moved-value-suggest-reborrow-issue-127285.stderr @@ -0,0 +1,18 @@ +error[E0382]: use of moved value: `self` + --> $DIR/moved-value-suggest-reborrow-issue-127285.rs:10:9 + | +LL | fn f(&mut self) { + | --------- move occurs because `self` has type `&mut X`, which does not implement the `Copy` trait +LL | generic(self); + | ---- value moved here +LL | self.0 += 1; + | ^^^^^^^^^^^ value used here after move + | +help: consider creating a fresh reborrow of `self` here + | +LL | generic(&mut *self); + | ++++++ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0382`. diff --git a/tests/ui/borrowck/mut-borrow-in-loop-2.stderr b/tests/ui/borrowck/mut-borrow-in-loop-2.stderr index 7a569d1da41c4..4f32df1eb24e2 100644 --- a/tests/ui/borrowck/mut-borrow-in-loop-2.stderr +++ b/tests/ui/borrowck/mut-borrow-in-loop-2.stderr @@ -8,19 +8,10 @@ LL | for _ in 0..3 { LL | Other::handle(value); | ^^^^^ value moved here, in previous iteration of loop | -note: consider changing this parameter type in function `handle` to borrow instead if owning the value isn't necessary - --> $DIR/mut-borrow-in-loop-2.rs:8:22 - | -LL | fn handle(value: T) -> Self; - | ------ ^ this parameter takes ownership of the value - | | - | in this function -help: consider moving the expression out of the loop so it is only moved once - | -LL ~ let mut value = Other::handle(value); -LL ~ for _ in 0..3 { -LL ~ value; +help: consider creating a fresh reborrow of `value` here | +LL | Other::handle(&mut *value); + | ++++++ help: consider creating a fresh reborrow of `value` here | LL | Other::handle(&mut *value);