Skip to content
131 changes: 89 additions & 42 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,8 +602,14 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
BorrowedContentSource::DerefRawPointer
} else if base_ty.is_mutable_ptr() {
BorrowedContentSource::DerefMutableRef
} else {
} else if base_ty.is_ref() {
BorrowedContentSource::DerefSharedRef
} else {
// Custom type implementing `Deref` (e.g. `MyBox<T>`, `Rc<T>`, `Arc<T>`)
// that wasn't detected via the MIR init trace above. This can happen
// when the deref base is initialized by a regular statement rather than
// a `TerminatorKind::Call` with `CallSource::OverloadedOperator`.
BorrowedContentSource::OverloadedDeref(base_ty)
}
}

Expand Down Expand Up @@ -1002,6 +1008,14 @@ struct CapturedMessageOpt {
maybe_reinitialized_locations_is_empty: bool,
}

/// Tracks whether [`MirBorrowckCtxt::explain_captures`] emitted a clone
/// suggestion, so callers can avoid emitting redundant suggestions downstream.
#[derive(Copy, Clone, PartialEq, Eq)]
pub(super) enum CloneSuggestion {
Emitted,
NotEmitted,
}

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(
Expand Down Expand Up @@ -1226,7 +1240,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
move_spans: UseSpans<'tcx>,
moved_place: Place<'tcx>,
msg_opt: CapturedMessageOpt,
) {
) -> CloneSuggestion {
let CapturedMessageOpt {
is_partial_move: is_partial,
is_loop_message,
Expand All @@ -1235,6 +1249,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
has_suggest_reborrow,
maybe_reinitialized_locations_is_empty,
} = msg_opt;
let mut suggested_cloning = false;
if let UseSpans::FnSelfUse { var_span, fn_call_span, fn_span, kind } = move_spans {
let place_name = self
.describe_place(moved_place.as_ref())
Expand Down Expand Up @@ -1461,10 +1476,26 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
has_sugg = true;
}
if let Some(clone_trait) = tcx.lang_items().clone_trait() {
let sugg = if moved_place
// Check whether the deref is from a custom Deref impl
// (e.g. Rc, Box) or a built-in reference deref.
// For built-in derefs with Clone fully satisfied, we skip
// the UFCS suggestion here and let `suggest_cloning`
// downstream emit a simpler `.clone()` suggestion instead.
let has_overloaded_deref =
moved_place.iter_projections().any(|(place, elem)| {
matches!(elem, ProjectionElem::Deref)
&& matches!(
self.borrowed_content_source(place),
BorrowedContentSource::OverloadedDeref(_)
| BorrowedContentSource::OverloadedIndex(_)
)
});

let has_deref = moved_place
.iter_projections()
.any(|(_, elem)| matches!(elem, ProjectionElem::Deref))
{
.any(|(_, elem)| matches!(elem, ProjectionElem::Deref));

let sugg = if has_deref {
let (start, end) = if let Some(expr) = self.find_expr(move_span)
&& let Some(_) = self.clone_on_reference(expr)
&& let hir::ExprKind::MethodCall(_, rcvr, _, _) = expr.kind
Expand All @@ -1490,43 +1521,58 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
self.infcx.param_env,
) && !has_sugg
{
let msg = match &errors[..] {
[] => "you can `clone` the value and consume it, but this \
might not be your desired behavior"
.to_string(),
[error] => {
format!(
"you could `clone` the value and consume it, if the \
`{}` trait bound could be satisfied",
error.obligation.predicate,
)
}
_ => {
format!(
"you could `clone` the value and consume it, if the \
following trait bounds could be satisfied: {}",
listify(&errors, |e: &FulfillmentError<'tcx>| format!(
"`{}`",
e.obligation.predicate
))
.unwrap(),
)
}
};
err.multipart_suggestion(msg, sugg, Applicability::MaybeIncorrect);
for error in errors {
if let FulfillmentErrorCode::Select(
SelectionError::Unimplemented,
) = error.code
&& let ty::PredicateKind::Clause(ty::ClauseKind::Trait(
pred,
)) = error.obligation.predicate.kind().skip_binder()
{
self.infcx.err_ctxt().suggest_derive(
&error.obligation,
err,
error.obligation.predicate.kind().rebind(pred),
);
let skip_for_simple_clone =
has_deref && !has_overloaded_deref && errors.is_empty();
if !skip_for_simple_clone {
let msg = match &errors[..] {
[] => "you can `clone` the value and consume it, but \
this might not be your desired behavior"
.to_string(),
[error] => {
format!(
"you could `clone` the value and consume it, if \
the `{}` trait bound could be satisfied",
error.obligation.predicate,
)
}
_ => {
format!(
"you could `clone` the value and consume it, if \
the following trait bounds could be satisfied: \
{}",
listify(
&errors,
|e: &FulfillmentError<'tcx>| format!(
"`{}`",
e.obligation.predicate
)
)
.unwrap(),
)
}
};
err.multipart_suggestion(
msg,
sugg,
Applicability::MaybeIncorrect,
);

suggested_cloning = errors.is_empty();

for error in errors {
if let FulfillmentErrorCode::Select(
SelectionError::Unimplemented,
) = error.code
&& let ty::PredicateKind::Clause(ty::ClauseKind::Trait(
pred,
)) = error.obligation.predicate.kind().skip_binder()
{
self.infcx.err_ctxt().suggest_derive(
&error.obligation,
err,
error.obligation.predicate.kind().rebind(pred),
);
}
}
}
}
Expand Down Expand Up @@ -1558,6 +1604,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
})
}
}
if suggested_cloning { CloneSuggestion::Emitted } else { CloneSuggestion::NotEmitted }
}

/// Skip over locals that begin with an underscore or have no name
Expand Down
131 changes: 108 additions & 23 deletions compiler/rustc_borrowck/src/diagnostics/move_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ use rustc_trait_selection::infer::InferCtxtExt;
use tracing::debug;

use crate::MirBorrowckCtxt;
use crate::diagnostics::{CapturedMessageOpt, DescribePlaceOpt, UseSpans};
use crate::diagnostics::{
BorrowedContentSource, CapturedMessageOpt, CloneSuggestion, DescribePlaceOpt, UseSpans,
};
use crate::prefixes::PrefixSet;

#[derive(Debug)]
Expand Down Expand Up @@ -269,14 +271,19 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
.span_delayed_bug(span, "Type may implement copy, but there is no other error.");
return;
}

let mut has_clone_suggestion = CloneSuggestion::NotEmitted;
let mut err = match kind {
&IllegalMoveOriginKind::BorrowedContent { target_place } => self
.report_cannot_move_from_borrowed_content(
&IllegalMoveOriginKind::BorrowedContent { target_place } => {
let (diag, clone_sugg) = self.report_cannot_move_from_borrowed_content(
original_path,
target_place,
span,
use_spans,
),
);
has_clone_suggestion = clone_sugg;
diag
}
&IllegalMoveOriginKind::InteriorOfTypeWithDestructor { container_ty: ty } => {
self.cannot_move_out_of_interior_of_drop(span, ty)
}
Expand All @@ -285,7 +292,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
}
};

self.add_move_hints(error, &mut err, span);
self.add_move_hints(error, &mut err, span, has_clone_suggestion);
self.buffer_error(err);
}

Expand Down Expand Up @@ -426,7 +433,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
deref_target_place: Place<'tcx>,
span: Span,
use_spans: Option<UseSpans<'tcx>>,
) -> Diag<'infcx> {
) -> (Diag<'infcx>, CloneSuggestion) {
let tcx = self.infcx.tcx;
// Inspect the type of the content behind the
// borrow to provide feedback about why this
Expand All @@ -447,8 +454,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
let decl = &self.body.local_decls[local];
let local_name = self.local_name(local).map(|sym| format!("`{sym}`"));
if decl.is_ref_for_guard() {
return self
.cannot_move_out_of(
return (
self.cannot_move_out_of(
span,
&format!(
"{} in pattern guard",
Expand All @@ -458,9 +465,14 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
.with_note(
"variables bound in patterns cannot be moved from \
until after the end of the pattern guard",
);
),
CloneSuggestion::NotEmitted,
);
} else if decl.is_ref_to_static() {
return self.report_cannot_move_from_static(move_place, span);
return (
self.report_cannot_move_from_static(move_place, span),
CloneSuggestion::NotEmitted,
);
}
}

Expand Down Expand Up @@ -539,10 +551,12 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
has_suggest_reborrow: false,
maybe_reinitialized_locations_is_empty: true,
};
if let Some(use_spans) = use_spans {
self.explain_captures(&mut err, span, span, use_spans, move_place, msg_opt);
}
err
let suggested_cloning = if let Some(use_spans) = use_spans {
self.explain_captures(&mut err, span, span, use_spans, move_place, msg_opt)
} else {
CloneSuggestion::NotEmitted
};
(err, suggested_cloning)
}

fn report_closure_move_error(
Expand Down Expand Up @@ -676,7 +690,49 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
}
}

fn add_move_hints(&self, error: GroupedMoveError<'tcx>, err: &mut Diag<'_>, span: Span) {
/// Suggest cloning via UFCS when a move occurs through a custom `Deref` impl.
///
/// A simple `.clone()` on a type like `MyBox<Vec<i32>>` would clone the wrapper,
/// not the inner `Vec<i32>`. Instead, we suggest `<Vec<i32> as Clone>::clone(&val)`.
fn suggest_cloning_through_overloaded_deref(
&self,
err: &mut Diag<'_>,
ty: Ty<'tcx>,
span: Span,
) -> CloneSuggestion {
let tcx = self.infcx.tcx;
let Some(clone_trait) = tcx.lang_items().clone_trait() else {
return CloneSuggestion::NotEmitted;
};
let Some(errors) =
self.infcx.type_implements_trait_shallow(clone_trait, ty, self.infcx.param_env)
else {
return CloneSuggestion::NotEmitted;
};

if !errors.is_empty() {
return CloneSuggestion::NotEmitted;
}
let sugg = vec![
(span.shrink_to_lo(), format!("<{ty} as Clone>::clone(&")),
(span.shrink_to_hi(), ")".to_string()),
];
err.multipart_suggestion(
"you can `clone` the value and consume it, but this might not be \
your desired behavior",
sugg,
Applicability::MaybeIncorrect,
);
CloneSuggestion::Emitted
}

fn add_move_hints(
&self,
error: GroupedMoveError<'tcx>,
err: &mut Diag<'_>,
span: Span,
has_clone_suggestion: CloneSuggestion,
) {
match error {
GroupedMoveError::MovesFromPlace { mut binds_to, move_from, .. } => {
self.add_borrow_suggestions(err, span, !binds_to.is_empty());
Expand Down Expand Up @@ -719,14 +775,43 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
None => "value".to_string(),
};

if let Some(expr) = self.find_expr(use_span) {
self.suggest_cloning(
err,
original_path.as_ref(),
place_ty,
expr,
Some(use_spans),
);
if has_clone_suggestion == CloneSuggestion::NotEmitted {
// Check if the move is directly through a custom Deref impl
// (e.g. `*my_box` where MyBox implements Deref).
// A simple `.clone()` would clone the wrapper type rather than
// the inner value, so we need a UFCS suggestion instead.
//
// However, if there are further projections after the deref
// (e.g. `(*rc).field`), the value accessed is already the inner
// type and a simple `.clone()` works correctly.
let needs_ufcs = original_path.projection.last()
== Some(&ProjectionElem::Deref)
&& original_path.iter_projections().any(|(place, elem)| {
matches!(elem, ProjectionElem::Deref)
&& matches!(
self.borrowed_content_source(place),
BorrowedContentSource::OverloadedDeref(_)
| BorrowedContentSource::OverloadedIndex(_)
)
});

let emitted_ufcs = if needs_ufcs {
self.suggest_cloning_through_overloaded_deref(err, place_ty, use_span)
} else {
CloneSuggestion::NotEmitted
};

if emitted_ufcs == CloneSuggestion::NotEmitted {
if let Some(expr) = self.find_expr(use_span) {
self.suggest_cloning(
err,
original_path.as_ref(),
place_ty,
expr,
Some(use_spans),
);
}
}
}

if let Some(upvar_field) = self
Expand Down
Loading
Loading