Skip to content

Commit 7e0430f

Browse files
borsbors
committed
Auto merge of rust-lang#155766 - jhpratt:rollup-EcXAaqS, r=jhpratt
Rollup of 7 pull requests Successful merges: - rust-lang#155643 (Improve suggestion for $-prefixed fragment specifiers) - rust-lang#154197 (Avoid redundant clone suggestions in borrowck diagnostics) - rust-lang#154372 (Exposing Float Masks) - rust-lang#155680 (Handle index projections in call destinations in DSE) - rust-lang#155732 (bootstrap: Don't clone submodules unconditionally in dry-run) - rust-lang#155737 (Account for `GetSyntheticValue` failures) - rust-lang#155738 (Pass fields to `is_tuple_fields` instead of `SBValue` object)
2 parents 0a4ee3f + 84fd561 commit 7e0430f

30 files changed

Lines changed: 690 additions & 188 deletions

‎compiler/rustc_borrowck/src/diagnostics/mod.rs‎

Lines changed: 89 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -602,8 +602,14 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
602602
BorrowedContentSource::DerefRawPointer
603603
} else if base_ty.is_mutable_ptr() {
604604
BorrowedContentSource::DerefMutableRef
605-
} else {
605+
} else if base_ty.is_ref() {
606606
BorrowedContentSource::DerefSharedRef
607+
} else {
608+
// Custom type implementing `Deref` (e.g. `MyBox<T>`, `Rc<T>`, `Arc<T>`)
609+
// that wasn't detected via the MIR init trace above. This can happen
610+
// when the deref base is initialized by a regular statement rather than
611+
// a `TerminatorKind::Call` with `CallSource::OverloadedOperator`.
612+
BorrowedContentSource::OverloadedDeref(base_ty)
607613
}
608614
}
609615

@@ -1002,6 +1008,14 @@ struct CapturedMessageOpt {
10021008
maybe_reinitialized_locations_is_empty: bool,
10031009
}
10041010

1011+
/// Tracks whether [`MirBorrowckCtxt::explain_captures`] emitted a clone
1012+
/// suggestion, so callers can avoid emitting redundant suggestions downstream.
1013+
#[derive(Copy, Clone, PartialEq, Eq)]
1014+
pub(super) enum CloneSuggestion {
1015+
Emitted,
1016+
NotEmitted,
1017+
}
1018+
10051019
impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
10061020
/// Finds the spans associated to a move or copy of move_place at location.
10071021
pub(super) fn move_spans(
@@ -1226,7 +1240,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
12261240
move_spans: UseSpans<'tcx>,
12271241
moved_place: Place<'tcx>,
12281242
msg_opt: CapturedMessageOpt,
1229-
) {
1243+
) -> CloneSuggestion {
12301244
let CapturedMessageOpt {
12311245
is_partial_move: is_partial,
12321246
is_loop_message,
@@ -1235,6 +1249,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
12351249
has_suggest_reborrow,
12361250
maybe_reinitialized_locations_is_empty,
12371251
} = msg_opt;
1252+
let mut suggested_cloning = false;
12381253
if let UseSpans::FnSelfUse { var_span, fn_call_span, fn_span, kind } = move_spans {
12391254
let place_name = self
12401255
.describe_place(moved_place.as_ref())
@@ -1461,10 +1476,26 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
14611476
has_sugg = true;
14621477
}
14631478
if let Some(clone_trait) = tcx.lang_items().clone_trait() {
1464-
let sugg = if moved_place
1479+
// Check whether the deref is from a custom Deref impl
1480+
// (e.g. Rc, Box) or a built-in reference deref.
1481+
// For built-in derefs with Clone fully satisfied, we skip
1482+
// the UFCS suggestion here and let `suggest_cloning`
1483+
// downstream emit a simpler `.clone()` suggestion instead.
1484+
let has_overloaded_deref =
1485+
moved_place.iter_projections().any(|(place, elem)| {
1486+
matches!(elem, ProjectionElem::Deref)
1487+
&& matches!(
1488+
self.borrowed_content_source(place),
1489+
BorrowedContentSource::OverloadedDeref(_)
1490+
| BorrowedContentSource::OverloadedIndex(_)
1491+
)
1492+
});
1493+
1494+
let has_deref = moved_place
14651495
.iter_projections()
1466-
.any(|(_, elem)| matches!(elem, ProjectionElem::Deref))
1467-
{
1496+
.any(|(_, elem)| matches!(elem, ProjectionElem::Deref));
1497+
1498+
let sugg = if has_deref {
14681499
let (start, end) = if let Some(expr) = self.find_expr(move_span)
14691500
&& let Some(_) = self.clone_on_reference(expr)
14701501
&& let hir::ExprKind::MethodCall(_, rcvr, _, _) = expr.kind
@@ -1490,43 +1521,58 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
14901521
self.infcx.param_env,
14911522
) && !has_sugg
14921523
{
1493-
let msg = match &errors[..] {
1494-
[] => "you can `clone` the value and consume it, but this \
1495-
might not be your desired behavior"
1496-
.to_string(),
1497-
[error] => {
1498-
format!(
1499-
"you could `clone` the value and consume it, if the \
1500-
`{}` trait bound could be satisfied",
1501-
error.obligation.predicate,
1502-
)
1503-
}
1504-
_ => {
1505-
format!(
1506-
"you could `clone` the value and consume it, if the \
1507-
following trait bounds could be satisfied: {}",
1508-
listify(&errors, |e: &FulfillmentError<'tcx>| format!(
1509-
"`{}`",
1510-
e.obligation.predicate
1511-
))
1512-
.unwrap(),
1513-
)
1514-
}
1515-
};
1516-
err.multipart_suggestion(msg, sugg, Applicability::MaybeIncorrect);
1517-
for error in errors {
1518-
if let FulfillmentErrorCode::Select(
1519-
SelectionError::Unimplemented,
1520-
) = error.code
1521-
&& let ty::PredicateKind::Clause(ty::ClauseKind::Trait(
1522-
pred,
1523-
)) = error.obligation.predicate.kind().skip_binder()
1524-
{
1525-
self.infcx.err_ctxt().suggest_derive(
1526-
&error.obligation,
1527-
err,
1528-
error.obligation.predicate.kind().rebind(pred),
1529-
);
1524+
let skip_for_simple_clone =
1525+
has_deref && !has_overloaded_deref && errors.is_empty();
1526+
if !skip_for_simple_clone {
1527+
let msg = match &errors[..] {
1528+
[] => "you can `clone` the value and consume it, but \
1529+
this might not be your desired behavior"
1530+
.to_string(),
1531+
[error] => {
1532+
format!(
1533+
"you could `clone` the value and consume it, if \
1534+
the `{}` trait bound could be satisfied",
1535+
error.obligation.predicate,
1536+
)
1537+
}
1538+
_ => {
1539+
format!(
1540+
"you could `clone` the value and consume it, if \
1541+
the following trait bounds could be satisfied: \
1542+
{}",
1543+
listify(
1544+
&errors,
1545+
|e: &FulfillmentError<'tcx>| format!(
1546+
"`{}`",
1547+
e.obligation.predicate
1548+
)
1549+
)
1550+
.unwrap(),
1551+
)
1552+
}
1553+
};
1554+
err.multipart_suggestion(
1555+
msg,
1556+
sugg,
1557+
Applicability::MaybeIncorrect,
1558+
);
1559+
1560+
suggested_cloning = errors.is_empty();
1561+
1562+
for error in errors {
1563+
if let FulfillmentErrorCode::Select(
1564+
SelectionError::Unimplemented,
1565+
) = error.code
1566+
&& let ty::PredicateKind::Clause(ty::ClauseKind::Trait(
1567+
pred,
1568+
)) = error.obligation.predicate.kind().skip_binder()
1569+
{
1570+
self.infcx.err_ctxt().suggest_derive(
1571+
&error.obligation,
1572+
err,
1573+
error.obligation.predicate.kind().rebind(pred),
1574+
);
1575+
}
15301576
}
15311577
}
15321578
}
@@ -1558,6 +1604,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
15581604
})
15591605
}
15601606
}
1607+
if suggested_cloning { CloneSuggestion::Emitted } else { CloneSuggestion::NotEmitted }
15611608
}
15621609

15631610
/// Skip over locals that begin with an underscore or have no name

‎compiler/rustc_borrowck/src/diagnostics/move_errors.rs‎

Lines changed: 108 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ use rustc_trait_selection::infer::InferCtxtExt;
1414
use tracing::debug;
1515

1616
use crate::MirBorrowckCtxt;
17-
use crate::diagnostics::{CapturedMessageOpt, DescribePlaceOpt, UseSpans};
17+
use crate::diagnostics::{
18+
BorrowedContentSource, CapturedMessageOpt, CloneSuggestion, DescribePlaceOpt, UseSpans,
19+
};
1820
use crate::prefixes::PrefixSet;
1921

2022
#[derive(Debug)]
@@ -269,14 +271,19 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
269271
.span_delayed_bug(span, "Type may implement copy, but there is no other error.");
270272
return;
271273
}
274+
275+
let mut has_clone_suggestion = CloneSuggestion::NotEmitted;
272276
let mut err = match kind {
273-
&IllegalMoveOriginKind::BorrowedContent { target_place } => self
274-
.report_cannot_move_from_borrowed_content(
277+
&IllegalMoveOriginKind::BorrowedContent { target_place } => {
278+
let (diag, clone_sugg) = self.report_cannot_move_from_borrowed_content(
275279
original_path,
276280
target_place,
277281
span,
278282
use_spans,
279-
),
283+
);
284+
has_clone_suggestion = clone_sugg;
285+
diag
286+
}
280287
&IllegalMoveOriginKind::InteriorOfTypeWithDestructor { container_ty: ty } => {
281288
self.cannot_move_out_of_interior_of_drop(span, ty)
282289
}
@@ -285,7 +292,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
285292
}
286293
};
287294

288-
self.add_move_hints(error, &mut err, span);
295+
self.add_move_hints(error, &mut err, span, has_clone_suggestion);
289296
self.buffer_error(err);
290297
}
291298

@@ -426,7 +433,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
426433
deref_target_place: Place<'tcx>,
427434
span: Span,
428435
use_spans: Option<UseSpans<'tcx>>,
429-
) -> Diag<'infcx> {
436+
) -> (Diag<'infcx>, CloneSuggestion) {
430437
let tcx = self.infcx.tcx;
431438
// Inspect the type of the content behind the
432439
// borrow to provide feedback about why this
@@ -447,8 +454,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
447454
let decl = &self.body.local_decls[local];
448455
let local_name = self.local_name(local).map(|sym| format!("`{sym}`"));
449456
if decl.is_ref_for_guard() {
450-
return self
451-
.cannot_move_out_of(
457+
return (
458+
self.cannot_move_out_of(
452459
span,
453460
&format!(
454461
"{} in pattern guard",
@@ -458,9 +465,14 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
458465
.with_note(
459466
"variables bound in patterns cannot be moved from \
460467
until after the end of the pattern guard",
461-
);
468+
),
469+
CloneSuggestion::NotEmitted,
470+
);
462471
} else if decl.is_ref_to_static() {
463-
return self.report_cannot_move_from_static(move_place, span);
472+
return (
473+
self.report_cannot_move_from_static(move_place, span),
474+
CloneSuggestion::NotEmitted,
475+
);
464476
}
465477
}
466478

@@ -539,10 +551,12 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
539551
has_suggest_reborrow: false,
540552
maybe_reinitialized_locations_is_empty: true,
541553
};
542-
if let Some(use_spans) = use_spans {
543-
self.explain_captures(&mut err, span, span, use_spans, move_place, msg_opt);
544-
}
545-
err
554+
let suggested_cloning = if let Some(use_spans) = use_spans {
555+
self.explain_captures(&mut err, span, span, use_spans, move_place, msg_opt)
556+
} else {
557+
CloneSuggestion::NotEmitted
558+
};
559+
(err, suggested_cloning)
546560
}
547561

548562
fn report_closure_move_error(
@@ -676,7 +690,49 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
676690
}
677691
}
678692

679-
fn add_move_hints(&self, error: GroupedMoveError<'tcx>, err: &mut Diag<'_>, span: Span) {
693+
/// Suggest cloning via UFCS when a move occurs through a custom `Deref` impl.
694+
///
695+
/// A simple `.clone()` on a type like `MyBox<Vec<i32>>` would clone the wrapper,
696+
/// not the inner `Vec<i32>`. Instead, we suggest `<Vec<i32> as Clone>::clone(&val)`.
697+
fn suggest_cloning_through_overloaded_deref(
698+
&self,
699+
err: &mut Diag<'_>,
700+
ty: Ty<'tcx>,
701+
span: Span,
702+
) -> CloneSuggestion {
703+
let tcx = self.infcx.tcx;
704+
let Some(clone_trait) = tcx.lang_items().clone_trait() else {
705+
return CloneSuggestion::NotEmitted;
706+
};
707+
let Some(errors) =
708+
self.infcx.type_implements_trait_shallow(clone_trait, ty, self.infcx.param_env)
709+
else {
710+
return CloneSuggestion::NotEmitted;
711+
};
712+
713+
if !errors.is_empty() {
714+
return CloneSuggestion::NotEmitted;
715+
}
716+
let sugg = vec![
717+
(span.shrink_to_lo(), format!("<{ty} as Clone>::clone(&")),
718+
(span.shrink_to_hi(), ")".to_string()),
719+
];
720+
err.multipart_suggestion(
721+
"you can `clone` the value and consume it, but this might not be \
722+
your desired behavior",
723+
sugg,
724+
Applicability::MaybeIncorrect,
725+
);
726+
CloneSuggestion::Emitted
727+
}
728+
729+
fn add_move_hints(
730+
&self,
731+
error: GroupedMoveError<'tcx>,
732+
err: &mut Diag<'_>,
733+
span: Span,
734+
has_clone_suggestion: CloneSuggestion,
735+
) {
680736
match error {
681737
GroupedMoveError::MovesFromPlace { mut binds_to, move_from, .. } => {
682738
self.add_borrow_suggestions(err, span, !binds_to.is_empty());
@@ -719,14 +775,43 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
719775
None => "value".to_string(),
720776
};
721777

722-
if let Some(expr) = self.find_expr(use_span) {
723-
self.suggest_cloning(
724-
err,
725-
original_path.as_ref(),
726-
place_ty,
727-
expr,
728-
Some(use_spans),
729-
);
778+
if has_clone_suggestion == CloneSuggestion::NotEmitted {
779+
// Check if the move is directly through a custom Deref impl
780+
// (e.g. `*my_box` where MyBox implements Deref).
781+
// A simple `.clone()` would clone the wrapper type rather than
782+
// the inner value, so we need a UFCS suggestion instead.
783+
//
784+
// However, if there are further projections after the deref
785+
// (e.g. `(*rc).field`), the value accessed is already the inner
786+
// type and a simple `.clone()` works correctly.
787+
let needs_ufcs = original_path.projection.last()
788+
== Some(&ProjectionElem::Deref)
789+
&& original_path.iter_projections().any(|(place, elem)| {
790+
matches!(elem, ProjectionElem::Deref)
791+
&& matches!(
792+
self.borrowed_content_source(place),
793+
BorrowedContentSource::OverloadedDeref(_)
794+
| BorrowedContentSource::OverloadedIndex(_)
795+
)
796+
});
797+
798+
let emitted_ufcs = if needs_ufcs {
799+
self.suggest_cloning_through_overloaded_deref(err, place_ty, use_span)
800+
} else {
801+
CloneSuggestion::NotEmitted
802+
};
803+
804+
if emitted_ufcs == CloneSuggestion::NotEmitted {
805+
if let Some(expr) = self.find_expr(use_span) {
806+
self.suggest_cloning(
807+
err,
808+
original_path.as_ref(),
809+
place_ty,
810+
expr,
811+
Some(use_spans),
812+
);
813+
}
814+
}
730815
}
731816

732817
if let Some(upvar_field) = self

0 commit comments

Comments
 (0)
⚡