diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
index 7c4623e6ea7..27cb0bbe537 100644
--- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
@@ -476,7 +476,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
} else if self.suggest_hoisting_call_outside_loop(err, expr) {
// The place where the the type moves would be misleading to suggest clone.
// #121466
- self.suggest_cloning(err, ty, expr);
+ self.suggest_cloning(err, ty, expr, None);
}
}
if let Some(pat) = finder.pat {
@@ -987,7 +987,78 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
can_suggest_clone
}
- pub(crate) fn suggest_cloning(&self, err: &mut Diag<'_>, ty: Ty<'tcx>, expr: &hir::Expr<'_>) {
+ pub(crate) fn suggest_cloning(
+ &self,
+ err: &mut Diag<'_>,
+ ty: Ty<'tcx>,
+ expr: &hir::Expr<'_>,
+ other_expr: Option<&hir::Expr<'_>>,
+ ) {
+ 'outer: {
+ if let ty::Ref(..) = ty.kind() {
+ // We check for either `let binding = foo(expr, other_expr);` or
+ // `foo(expr, other_expr);` and if so we don't suggest an incorrect
+ // `foo(expr, other_expr).clone()`
+ if let Some(other_expr) = other_expr
+ && let Some(parent_let) =
+ self.infcx.tcx.hir().parent_iter(expr.hir_id).find_map(|n| {
+ if let (hir_id, hir::Node::Local(_) | hir::Node::Stmt(_)) = n {
+ Some(hir_id)
+ } else {
+ None
+ }
+ })
+ && let Some(other_parent_let) =
+ self.infcx.tcx.hir().parent_iter(other_expr.hir_id).find_map(|n| {
+ if let (hir_id, hir::Node::Local(_) | hir::Node::Stmt(_)) = n {
+ Some(hir_id)
+ } else {
+ None
+ }
+ })
+ && parent_let == other_parent_let
+ {
+ // Explicitly check that we don't have `foo(&*expr, other_expr)`, as cloning the
+ // result of `foo(...)` won't help.
+ break 'outer;
+ }
+
+ // We're suggesting `.clone()` on an borrowed value. See if the expression we have
+ // is an argument to a function or method call, and try to suggest cloning the
+ // *result* of the call, instead of the argument. This is closest to what people
+ // would actually be looking for in most cases, with maybe the exception of things
+ // like `fn(T) -> T`, but even then it is reasonable.
+ let typeck_results = self.infcx.tcx.typeck(self.mir_def_id());
+ let mut prev = expr;
+ while let hir::Node::Expr(parent) = self.infcx.tcx.parent_hir_node(prev.hir_id) {
+ if let hir::ExprKind::Call(..) | hir::ExprKind::MethodCall(..) = parent.kind
+ && let Some(call_ty) = typeck_results.node_type_opt(parent.hir_id)
+ && let call_ty = call_ty.peel_refs()
+ && (!call_ty
+ .walk()
+ .any(|t| matches!(t.unpack(), ty::GenericArgKind::Lifetime(_)))
+ || if let ty::Alias(ty::Projection, _) = call_ty.kind() {
+ // FIXME: this isn't quite right with lifetimes on assoc types,
+ // but ignore for now. We will only suggest cloning if
+ // `<Ty as Trait>::Assoc: Clone`, which should keep false positives
+ // down to a managable ammount.
+ true
+ } else {
+ false
+ })
+ && let Some(clone_trait_def) = self.infcx.tcx.lang_items().clone_trait()
+ && self
+ .infcx
+ .type_implements_trait(clone_trait_def, [call_ty], self.param_env)
+ .must_apply_modulo_regions()
+ && self.suggest_cloning_inner(err, call_ty, parent)
+ {
+ return;
+ }
+ prev = parent;
+ }
+ }
+ }
let ty = ty.peel_refs();
if let Some(clone_trait_def) = self.infcx.tcx.lang_items().clone_trait()
&& self
@@ -1014,12 +1085,17 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}
- fn suggest_cloning_inner(&self, err: &mut Diag<'_>, ty: Ty<'tcx>, expr: &hir::Expr<'_>) {
+ fn suggest_cloning_inner(
+ &self,
+ err: &mut Diag<'_>,
+ ty: Ty<'tcx>,
+ expr: &hir::Expr<'_>,
+ ) -> bool {
let tcx = self.infcx.tcx;
if let Some(_) = self.clone_on_reference(expr) {
// Avoid redundant clone suggestion already suggested in `explain_captures`.
// See `tests/ui/moves/needs-clone-through-deref.rs`
- return;
+ return false;
}
// Try to find predicates on *generic params* that would allow copying `ty`
let suggestion =
@@ -1036,7 +1112,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
if let hir::ExprKind::AddrOf(_, hir::Mutability::Mut, _) = inner_expr.kind {
// We assume that `&mut` refs are desired for their side-effects, so cloning the
// value wouldn't do what the user wanted.
- return;
+ return false;
}
inner_expr = inner;
}
@@ -1059,6 +1135,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
"consider cloning the value if the performance cost is acceptable"
};
err.multipart_suggestion_verbose(msg, sugg, Applicability::MachineApplicable);
+ true
}
fn suggest_adding_bounds(&self, err: &mut Diag<'_>, ty: Ty<'tcx>, def_id: DefId, span: Span) {
@@ -1167,7 +1244,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
if let Some(expr) = self.find_expr(borrow_span)
&& let Some(ty) = typeck_results.node_type_opt(expr.hir_id)
{
- self.suggest_cloning(&mut err, ty, expr);
+ self.suggest_cloning(&mut err, ty, expr, self.find_expr(span));
}
self.buffer_error(err);
}
diff --git a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs
index 31ef3519fff..bc02c5be93d 100644
--- a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs
@@ -435,7 +435,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
fn add_move_hints(&self, error: GroupedMoveError<'tcx>, err: &mut Diag<'_>, span: Span) {
match error {
- GroupedMoveError::MovesFromPlace { mut binds_to, move_from, .. } => {
+ GroupedMoveError::MovesFromPlace {
+ mut binds_to, move_from, span: other_span, ..
+ } => {
self.add_borrow_suggestions(err, span);
if binds_to.is_empty() {
let place_ty = move_from.ty(self.body, self.infcx.tcx).ty;
@@ -445,7 +447,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
};
if let Some(expr) = self.find_expr(span) {
- self.suggest_cloning(err, place_ty, expr);
+ self.suggest_cloning(err, place_ty, expr, self.find_expr(other_span));
}
err.subdiagnostic(
@@ -472,15 +474,15 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
}
// No binding. Nothing to suggest.
GroupedMoveError::OtherIllegalMove { ref original_path, use_spans, .. } => {
- let span = use_spans.var_or_use();
+ let use_span = use_spans.var_or_use();
let place_ty = original_path.ty(self.body, self.infcx.tcx).ty;
let place_desc = match self.describe_place(original_path.as_ref()) {
Some(desc) => format!("`{desc}`"),
None => "value".to_string(),
};
- if let Some(expr) = self.find_expr(span) {
- self.suggest_cloning(err, place_ty, expr);
+ if let Some(expr) = self.find_expr(use_span) {
+ self.suggest_cloning(err, place_ty, expr, self.find_expr(span));
}
err.subdiagnostic(
@@ -489,7 +491,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
is_partial_move: false,
ty: place_ty,
place: &place_desc,
- span,
+ span: use_span,
},
);
@@ -593,7 +595,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
let place_desc = &format!("`{}`", self.local_names[*local].unwrap());
if let Some(expr) = self.find_expr(binding_span) {
- self.suggest_cloning(err, bind_to.ty, expr);
+ self.suggest_cloning(err, bind_to.ty, expr, None);
}
err.subdiagnostic(
diff --git a/tests/ui/associated-types/associated-types-outlives.rs b/tests/ui/associated-types/associated-types-outlives.rs
index 55c276280b9..245218067b4 100644
--- a/tests/ui/associated-types/associated-types-outlives.rs
+++ b/tests/ui/associated-types/associated-types-outlives.rs
@@ -3,10 +3,10 @@
// fn body, causing this (invalid) code to be accepted.
pub trait Foo<'a> {
- type Bar;
+ type Bar: Clone;
}
-impl<'a, T:'a> Foo<'a> for T {
+impl<'a, T: 'a> Foo<'a> for T {
type Bar = &'a T;
}
diff --git a/tests/ui/associated-types/associated-types-outlives.stderr b/tests/ui/associated-types/associated-types-outlives.stderr
index deeedd22266..c97af672c33 100644
--- a/tests/ui/associated-types/associated-types-outlives.stderr
+++ b/tests/ui/associated-types/associated-types-outlives.stderr
@@ -10,6 +10,11 @@ LL | drop(x);
| ^ move out of `x` occurs here
LL | return f(y);
| - borrow later used here
+ |
+help: consider cloning the value if the performance cost is acceptable
+ |
+LL | 's: loop { y = denormalise(&x).clone(); break }
+ | ++++++++
error: aborting due to 1 previous error
diff --git a/tests/ui/variance/variance-issue-20533.rs b/tests/ui/variance/variance-issue-20533.rs
index a2459f8730c..4c109608906 100644
--- a/tests/ui/variance/variance-issue-20533.rs
+++ b/tests/ui/variance/variance-issue-20533.rs
@@ -19,8 +19,15 @@ fn baz<'a, T>(_x: &'a T) -> Baked<'a> {
Baked(PhantomData)
}
+fn bat(x: &AffineU32) -> &u32 {
+ &x.0
+}
+
struct AffineU32(u32);
+#[derive(Clone)]
+struct ClonableAffineU32(u32);
+
fn main() {
{
let a = AffineU32(1);
@@ -40,4 +47,16 @@ fn main() {
drop(a); //~ ERROR cannot move out of `a`
drop(x);
}
+ {
+ let a = AffineU32(1);
+ let x = bat(&a);
+ drop(a); //~ ERROR cannot move out of `a`
+ drop(x);
+ }
+ {
+ let a = ClonableAffineU32(1);
+ let x = foo(&a);
+ drop(a); //~ ERROR cannot move out of `a`
+ drop(x);
+ }
}
diff --git a/tests/ui/variance/variance-issue-20533.stderr b/tests/ui/variance/variance-issue-20533.stderr
index 258f67db5ce..7060144c776 100644
--- a/tests/ui/variance/variance-issue-20533.stderr
+++ b/tests/ui/variance/variance-issue-20533.stderr
@@ -1,5 +1,5 @@
error[E0505]: cannot move out of `a` because it is borrowed
- --> $DIR/variance-issue-20533.rs:28:14
+ --> $DIR/variance-issue-20533.rs:35:14
|
LL | let a = AffineU32(1);
| - binding `a` declared here
@@ -11,7 +11,7 @@ LL | drop(x);
| - borrow later used here
error[E0505]: cannot move out of `a` because it is borrowed
- --> $DIR/variance-issue-20533.rs:34:14
+ --> $DIR/variance-issue-20533.rs:41:14
|
LL | let a = AffineU32(1);
| - binding `a` declared here
@@ -23,7 +23,7 @@ LL | drop(x);
| - borrow later used here
error[E0505]: cannot move out of `a` because it is borrowed
- --> $DIR/variance-issue-20533.rs:40:14
+ --> $DIR/variance-issue-20533.rs:47:14
|
LL | let a = AffineU32(1);
| - binding `a` declared here
@@ -34,6 +34,41 @@ LL | drop(a);
LL | drop(x);
| - borrow later used here
-error: aborting due to 3 previous errors
+error[E0505]: cannot move out of `a` because it is borrowed
+ --> $DIR/variance-issue-20533.rs:53:14
+ |
+LL | let a = AffineU32(1);
+ | - binding `a` declared here
+LL | let x = bat(&a);
+ | -- borrow of `a` occurs here
+LL | drop(a);
+ | ^ move out of `a` occurs here
+LL | drop(x);
+ | - borrow later used here
+ |
+help: consider cloning the value if the performance cost is acceptable
+ |
+LL | let x = bat(&a).clone();
+ | ++++++++
+
+error[E0505]: cannot move out of `a` because it is borrowed
+ --> $DIR/variance-issue-20533.rs:59:14
+ |
+LL | let a = ClonableAffineU32(1);
+ | - binding `a` declared here
+LL | let x = foo(&a);
+ | -- borrow of `a` occurs here
+LL | drop(a);
+ | ^ move out of `a` occurs here
+LL | drop(x);
+ | - borrow later used here
+ |
+help: consider cloning the value if the performance cost is acceptable
+ |
+LL - let x = foo(&a);
+LL + let x = foo(a.clone());
+ |
+
+error: aborting due to 5 previous errors
For more information about this error, try `rustc --explain E0505`.