From 61ff157bd47ce626795f1672d6cc7bf3ba292fd6 Mon Sep 17 00:00:00 2001 From: yuk1ty Date: Sat, 11 Apr 2026 21:28:03 +0900 Subject: [PATCH] Address custom type implementing Derefs to suggest UFCS clone --- .../src/diagnostics/conflict_errors.rs | 4 +- .../rustc_borrowck/src/diagnostics/mod.rs | 22 ++-- .../src/diagnostics/move_errors.rs | 101 +++++++++++++++--- .../ufcs-for-deref-inner-clone.fixed | 16 +++ .../suggestions/ufcs-for-deref-inner-clone.rs | 16 +++ .../ufcs-for-deref-inner-clone.stderr | 14 +++ 6 files changed, 146 insertions(+), 27 deletions(-) create mode 100644 tests/ui/suggestions/ufcs-for-deref-inner-clone.fixed create mode 100644 tests/ui/suggestions/ufcs-for-deref-inner-clone.rs create mode 100644 tests/ui/suggestions/ufcs-for-deref-inner-clone.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 639cb75fdd28..2735c800c4a3 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -254,9 +254,7 @@ pub(crate) fn report_use_of_moved_or_uninitialized( maybe_reinitialized_locations_is_empty: maybe_reinitialized_locations .is_empty(), }; - // The return value indicates whether a clone suggestion was emitted; - // redundancy is already handled via the `FnSelfUse` check below. - let _ = self.explain_captures( + self.explain_captures( &mut err, span, move_span, diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index 3ee0369c9da6..ab2c3bdba85f 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -602,8 +602,14 @@ pub(super) fn borrowed_content_source( 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`, `Rc`, `Arc`) + // 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) } } @@ -1002,10 +1008,12 @@ struct CapturedMessageOpt { maybe_reinitialized_locations_is_empty: bool, } -/// Tracks which suggestions were emitted by [`MirBorrowckCtxt::explain_captures`], -/// so callers can avoid emitting redundant suggestions downstream. -struct ExplainCapturesResult { - clone_suggestion: 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> { @@ -1232,7 +1240,7 @@ fn explain_captures( move_spans: UseSpans<'tcx>, moved_place: Place<'tcx>, msg_opt: CapturedMessageOpt, - ) -> ExplainCapturesResult { + ) -> CloneSuggestion { let CapturedMessageOpt { is_partial_move: is_partial, is_loop_message, @@ -1596,7 +1604,7 @@ fn explain_captures( }) } } - ExplainCapturesResult { clone_suggestion: suggested_cloning } + if suggested_cloning { CloneSuggestion::Emitted } else { CloneSuggestion::NotEmitted } } /// Skip over locals that begin with an underscore or have no name diff --git a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs index 1dfd83956e49..9aa0bc1b745d 100644 --- a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs @@ -14,7 +14,9 @@ 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)] @@ -270,7 +272,7 @@ fn report(&mut self, error: GroupedMoveError<'tcx>) { return; } - let mut has_clone_suggestion = false; + let mut has_clone_suggestion = CloneSuggestion::NotEmitted; let mut err = match kind { &IllegalMoveOriginKind::BorrowedContent { target_place } => { let (diag, clone_sugg) = self.report_cannot_move_from_borrowed_content( @@ -431,7 +433,7 @@ fn report_cannot_move_from_borrowed_content( deref_target_place: Place<'tcx>, span: Span, use_spans: Option>, - ) -> (Diag<'infcx>, bool) { + ) -> (Diag<'infcx>, CloneSuggestion) { let tcx = self.infcx.tcx; // Inspect the type of the content behind the // borrow to provide feedback about why this @@ -464,10 +466,13 @@ fn report_cannot_move_from_borrowed_content( "variables bound in patterns cannot be moved from \ until after the end of the pattern guard", ), - false, + CloneSuggestion::NotEmitted, ); } else if decl.is_ref_to_static() { - return (self.report_cannot_move_from_static(move_place, span), false); + return ( + self.report_cannot_move_from_static(move_place, span), + CloneSuggestion::NotEmitted, + ); } } @@ -548,9 +553,8 @@ fn report_cannot_move_from_borrowed_content( }; let suggested_cloning = if let Some(use_spans) = use_spans { self.explain_captures(&mut err, span, span, use_spans, move_place, msg_opt) - .clone_suggestion } else { - false + CloneSuggestion::NotEmitted }; (err, suggested_cloning) } @@ -686,12 +690,48 @@ fn closure_clause_kind( } } + /// Suggest cloning via UFCS when a move occurs through a custom `Deref` impl. + /// + /// A simple `.clone()` on a type like `MyBox>` would clone the wrapper, + /// not the inner `Vec`. Instead, we suggest ` 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: bool, + has_clone_suggestion: CloneSuggestion, ) { match error { GroupedMoveError::MovesFromPlace { mut binds_to, move_from, .. } => { @@ -735,15 +775,42 @@ fn add_move_hints( None => "value".to_string(), }; - if !has_clone_suggestion { - 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), + ); + } } } diff --git a/tests/ui/suggestions/ufcs-for-deref-inner-clone.fixed b/tests/ui/suggestions/ufcs-for-deref-inner-clone.fixed new file mode 100644 index 000000000000..de85216e8880 --- /dev/null +++ b/tests/ui/suggestions/ufcs-for-deref-inner-clone.fixed @@ -0,0 +1,16 @@ +//@ run-rustfix +use std::ops::Deref; + +struct MyBox(T); + +impl Deref for MyBox { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +pub fn main() { + let _x = as Clone>::clone(&MyBox(vec![1, 2])).into_iter(); + //~^ ERROR cannot move out of dereference of `MyBox>` +} diff --git a/tests/ui/suggestions/ufcs-for-deref-inner-clone.rs b/tests/ui/suggestions/ufcs-for-deref-inner-clone.rs new file mode 100644 index 000000000000..dc0dd02f97a9 --- /dev/null +++ b/tests/ui/suggestions/ufcs-for-deref-inner-clone.rs @@ -0,0 +1,16 @@ +//@ run-rustfix +use std::ops::Deref; + +struct MyBox(T); + +impl Deref for MyBox { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +pub fn main() { + let _x = MyBox(vec![1, 2]).into_iter(); + //~^ ERROR cannot move out of dereference of `MyBox>` +} diff --git a/tests/ui/suggestions/ufcs-for-deref-inner-clone.stderr b/tests/ui/suggestions/ufcs-for-deref-inner-clone.stderr new file mode 100644 index 000000000000..7fde14f1a61b --- /dev/null +++ b/tests/ui/suggestions/ufcs-for-deref-inner-clone.stderr @@ -0,0 +1,14 @@ +error[E0507]: cannot move out of dereference of `MyBox>` + --> $DIR/ufcs-for-deref-inner-clone.rs:14:14 + | +LL | let _x = MyBox(vec![1, 2]).into_iter(); + | ^^^^^^^^^^^^^^^^^ move occurs because value has type `Vec`, which does not implement the `Copy` trait + | +help: you can `clone` the value and consume it, but this might not be your desired behavior + | +LL | let _x = as Clone>::clone(&MyBox(vec![1, 2])).into_iter(); + | ++++++++++++++++++++++++++++ + + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0507`.