Mirrors/rust
1
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2026-04-27 18:57:42 +03:00
Code Issues Packages Projects Releases Wiki Activity

Avoid redundant clone suggestions in borrowck diagnostics

Browse Source
This commit is contained in:
yuk1ty
2026-03-21 23:12:46 +09:00
parent ec6f9a5b44
commit f69946ac64
13 changed files with 129 additions and 94 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
+3 -1
View File
@@ -254,7 +254,9 @@ pub(crate) fn report_use_of_moved_or_uninitialized(
maybe_reinitialized_locations_is_empty: maybe_reinitialized_locations
.is_empty(),
};
self.explain_captures(
// The return value indicates whether a clone suggestion was emitted;
// redundancy is already handled via the `FnSelfUse` check below.
let _ = self.explain_captures(
&mut err,
span,
move_span,
compiler/rustc_borrowck/src/diagnostics/mod.rs
+80 -41
View File
@@ -1002,6 +1002,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,
}
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(
@@ -1226,7 +1232,7 @@ fn explain_captures(
move_spans: UseSpans<'tcx>,
moved_place: Place<'tcx>,
msg_opt: CapturedMessageOpt,
) {
) -> ExplainCapturesResult {
let CapturedMessageOpt {
is_partial_move: is_partial,
is_loop_message,
@@ -1235,6 +1241,7 @@ fn explain_captures(
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())
@@ -1461,10 +1468,26 @@ fn explain_captures(
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
@@ -1490,43 +1513,58 @@ fn explain_captures(
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),
);
}
}
}
}
@@ -1558,6 +1596,7 @@ fn explain_captures(
})
}
}
ExplainCapturesResult { clone_suggestion: suggested_cloning }
}
/// Skip over locals that begin with an underscore or have no name
compiler/rustc_borrowck/src/diagnostics/move_errors.rs
+40 -22
View File
@@ -269,14 +269,19 @@ fn report(&mut self, error: GroupedMoveError<'tcx>) {
.span_delayed_bug(span, "Type may implement copy, but there is no other error.");
return;
}
let mut has_clone_suggestion = false;
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)
}
@@ -285,7 +290,7 @@ fn report(&mut self, error: GroupedMoveError<'tcx>) {
}
};
self.add_move_hints(error, &mut err, span);
self.add_move_hints(error, &mut err, span, has_clone_suggestion);
self.buffer_error(err);
}
@@ -426,7 +431,7 @@ fn report_cannot_move_from_borrowed_content(
deref_target_place: Place<'tcx>,
span: Span,
use_spans: Option<UseSpans<'tcx>>,
) -> Diag<'infcx> {
) -> (Diag<'infcx>, bool) {
let tcx = self.infcx.tcx;
// Inspect the type of the content behind the
// borrow to provide feedback about why this
@@ -447,8 +452,8 @@ fn report_cannot_move_from_borrowed_content(
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",
@@ -458,9 +463,11 @@ fn report_cannot_move_from_borrowed_content(
.with_note(
"variables bound in patterns cannot be moved from \
until after the end of the pattern guard",
);
),
false,
);
} 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), false);
}
}
@@ -539,10 +546,13 @@ fn report_cannot_move_from_borrowed_content(
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)
.clone_suggestion
} else {
false
};
(err, suggested_cloning)
}
fn report_closure_move_error(
@@ -676,7 +686,13 @@ fn closure_clause_kind(
}
}
fn add_move_hints(&self, error: GroupedMoveError<'tcx>, err: &mut Diag<'_>, span: Span) {
fn add_move_hints(
&self,
error: GroupedMoveError<'tcx>,
err: &mut Diag<'_>,
span: Span,
has_clone_suggestion: bool,
) {
match error {
GroupedMoveError::MovesFromPlace { mut binds_to, move_from, .. } => {
self.add_borrow_suggestions(err, span, !binds_to.is_empty());
@@ -719,14 +735,16 @@ fn add_move_hints(&self, error: GroupedMoveError<'tcx>, err: &mut Diag<'_>, span
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 {
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
tests/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.fixed
+1 -1
View File
@@ -2,6 +2,6 @@
use std::rc::Rc;
pub fn main() {
let _x = <Vec<i32> as Clone>::clone(&Rc::new(vec![1, 2]).clone()).into_iter();
let _x = <Vec<i32> as Clone>::clone(&Rc::new(vec![1, 2])).into_iter();
//~^ ERROR [E0507]
}
tests/ui/borrowck/borrowck-move-out-of-overloaded-auto-deref.stderr
-4
View File
@@ -12,10 +12,6 @@ help: you can `clone` the value and consume it, but this might not be your desir
|
LL | let _x = <Vec<i32> as Clone>::clone(&Rc::new(vec![1, 2])).into_iter();
| ++++++++++++++++++++++++++++ +
help: consider cloning the value if the performance cost is acceptable
|
LL | let _x = Rc::new(vec![1, 2]).clone().into_iter();
| ++++++++
error: aborting due to 1 previous error
tests/ui/borrowck/clone-span-on-try-operator.fixed
+1 -1
View File
@@ -7,5 +7,5 @@ impl Foo {
}
fn main() {
let foo = &Foo;
<Foo as Clone>::clone(&foo.clone()).foo(); //~ ERROR cannot move out
foo.clone().foo(); //~ ERROR cannot move out
}
tests/ui/borrowck/clone-span-on-try-operator.stderr
-4
View File
@@ -11,10 +11,6 @@ note: `Foo::foo` takes ownership of the receiver `self`, which moves `*foo`
|
LL | fn foo(self) {}
| ^^^^
help: you can `clone` the value and consume it, but this might not be your desired behavior
|
LL | <Foo as Clone>::clone(&(*foo)).foo();
| +++++++++++++++++++++++ +
help: consider cloning the value if the performance cost is acceptable
|
LL - (*foo).foo();
tests/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.fixed
+1 -1
View File
@@ -9,7 +9,7 @@ fn call<F>(f: F) where F : Fn() {
fn main() {
let y = vec![format!("World")];
call(|| {
<Vec<String> as Clone>::clone(&y.clone()).into_iter();
y.clone().into_iter();
//~^ ERROR cannot move out of `y`, a captured variable in an `Fn` closure
});
}
tests/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.stderr
-4
View File
@@ -17,10 +17,6 @@ LL | fn call<F>(f: F) where F : Fn() {
| ^^^^
note: `into_iter` takes ownership of the receiver `self`, which moves `y`
--> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
help: you can `clone` the value and consume it, but this might not be your desired behavior
|
LL | <Vec<String> as Clone>::clone(&y).into_iter();
| +++++++++++++++++++++++++++++++ +
help: consider cloning the value if the performance cost is acceptable
|
LL | y.clone().into_iter();
tests/ui/moves/suggest-clone.fixed
+1 -1
View File
@@ -7,5 +7,5 @@ impl Foo {
}
fn main() {
let foo = &Foo;
<Foo as Clone>::clone(&foo.clone()).foo(); //~ ERROR cannot move out
foo.clone().foo(); //~ ERROR cannot move out
}
tests/ui/moves/suggest-clone.stderr
-4
View File
@@ -11,10 +11,6 @@ note: `Foo::foo` takes ownership of the receiver `self`, which moves `*foo`
|
LL | fn foo(self) {}
| ^^^^
help: you can `clone` the value and consume it, but this might not be your desired behavior
|
LL | <Foo as Clone>::clone(&foo).foo();
| +++++++++++++++++++++++ +
help: consider cloning the value if the performance cost is acceptable
|
LL | foo.clone().foo();
tests/ui/suggestions/option-content-move.fixed
+2 -2
View File
@@ -7,7 +7,7 @@ impl LipogramCorpora {
pub fn validate_all(&mut self) -> Result<(), char> {
for selection in &self.selections {
if selection.1.is_some() {
if <Option<String> as Clone>::clone(&selection.1.clone()).as_mut().as_ref().unwrap().contains(selection.0) {
if selection.1.clone().as_mut().as_ref().unwrap().contains(selection.0) {
//~^ ERROR cannot move out of `selection.1`
return Err(selection.0);
}
@@ -25,7 +25,7 @@ impl LipogramCorpora2 {
pub fn validate_all(&mut self) -> Result<(), char> {
for selection in &self.selections {
if selection.1.is_ok() {
if <Result<String, String> as Clone>::clone(&selection.1.clone()).as_mut().as_ref().unwrap().contains(selection.0) {
if selection.1.clone().as_mut().as_ref().unwrap().contains(selection.0) {
//~^ ERROR cannot move out of `selection.1`
return Err(selection.0);
}
tests/ui/suggestions/option-content-move.stderr
-8
View File
@@ -16,10 +16,6 @@ help: consider calling `.as_mut()` to mutably borrow the value's contents
|
LL | if selection.1.as_mut().unwrap().contains(selection.0) {
| +++++++++
help: you can `clone` the value and consume it, but this might not be your desired behavior
|
LL | if <Option<String> as Clone>::clone(&selection.1).unwrap().contains(selection.0) {
| ++++++++++++++++++++++++++++++++++ +
help: consider cloning the value if the performance cost is acceptable
|
LL | if selection.1.clone().unwrap().contains(selection.0) {
@@ -43,10 +39,6 @@ help: consider calling `.as_mut()` to mutably borrow the value's contents
|
LL | if selection.1.as_mut().unwrap().contains(selection.0) {
| +++++++++
help: you can `clone` the value and consume it, but this might not be your desired behavior
|
LL | if <Result<String, String> as Clone>::clone(&selection.1).unwrap().contains(selection.0) {
| ++++++++++++++++++++++++++++++++++++++++++ +
help: consider cloning the value if the performance cost is acceptable
|
LL | if selection.1.clone().unwrap().contains(selection.0) {