Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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