Suggest borrowing on fn argument that is impl AsRef

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

@@ -449,31 +449,81 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 } else {
                     (None, &[][..], 0)
                 };
+                let mut can_suggest_clone = true;
                 if let Some(def_id) = def_id
                     && let node = self.infcx.tcx.hir_node_by_def_id(def_id)
                     && let Some(fn_sig) = node.fn_sig()
                     && let Some(ident) = node.ident()
                     && let Some(pos) = args.iter().position(|arg| arg.hir_id == expr.hir_id)
                     && let Some(arg) = fn_sig.decl.inputs.get(pos + offset)
                 {
-                    let mut span: MultiSpan = arg.span.into();
-                    span.push_span_label(
-                        arg.span,
-                        "this parameter takes ownership of the value".to_string(),
-                    );
-                    let descr = match node.fn_kind() {
-                        Some(hir::intravisit::FnKind::ItemFn(..)) | None => "function",
-                        Some(hir::intravisit::FnKind::Method(..)) => "method",
-                        Some(hir::intravisit::FnKind::Closure) => "closure",
-                    };
-                    span.push_span_label(ident.span, format!("in this {descr}"));
-                    err.span_note(
-                        span,
-                        format!(
-                            "consider changing this parameter type in {descr} `{ident}` to borrow \
-                             instead if owning the value isn't necessary",
-                        ),
-                    );
+                    let mut is_mut = false;
+                    if let hir::TyKind::Path(hir::QPath::Resolved(None, path)) = arg.kind
+                        && let Res::Def(DefKind::TyParam, param_def_id) = path.res
+                        && self
+                            .infcx
+                            .tcx
+                            .predicates_of(def_id)
+                            .instantiate_identity(self.infcx.tcx)
+                            .predicates
+                            .into_iter()
+                            .any(|pred| {
+                                if let ty::ClauseKind::Trait(predicate) = pred.kind().skip_binder()
+                                    && [
+                                        self.infcx.tcx.get_diagnostic_item(sym::AsRef),
+                                        self.infcx.tcx.get_diagnostic_item(sym::AsMut),
+                                        self.infcx.tcx.get_diagnostic_item(sym::Borrow),
+                                        self.infcx.tcx.get_diagnostic_item(sym::BorrowMut),
+                                    ]
+                                    .contains(&Some(predicate.def_id()))
+                                    && let ty::Param(param) = predicate.self_ty().kind()
+                                    && let generics = self.infcx.tcx.generics_of(def_id)
+                                    && let param = generics.type_param(*param, self.infcx.tcx)
+                                    && param.def_id == param_def_id
+                                {
+                                    if [
+                                        self.infcx.tcx.get_diagnostic_item(sym::AsMut),
+                                        self.infcx.tcx.get_diagnostic_item(sym::BorrowMut),
+                                    ]
+                                    .contains(&Some(predicate.def_id()))
+                                    {
+                                        is_mut = true;
+                                    }
+                                    true
+                                } else {
+                                    false
+                                }
+                            })
+                    {
+                        // The type of the argument corresponding to the expression that got moved
+                        // is a type parameter `T`, which is has a `T: AsRef` obligation.
+                        err.span_suggestion_verbose(
+                            expr.span.shrink_to_lo(),
+                            "borrow the value to avoid moving it",
+                            format!("&{}", if is_mut { "mut " } else { "" }),
+                            Applicability::MachineApplicable,
+                        );
+                        can_suggest_clone = is_mut;
+                    } else {
+                        let mut span: MultiSpan = arg.span.into();
+                        span.push_span_label(
+                            arg.span,
+                            "this parameter takes ownership of the value".to_string(),
+                        );
+                        let descr = match node.fn_kind() {
+                            Some(hir::intravisit::FnKind::ItemFn(..)) | None => "function",
+                            Some(hir::intravisit::FnKind::Method(..)) => "method",
+                            Some(hir::intravisit::FnKind::Closure) => "closure",
+                        };
+                        span.push_span_label(ident.span, format!("in this {descr}"));
+                        err.span_note(
+                            span,
+                            format!(
+                                "consider changing this parameter type in {descr} `{ident}` to \
+                                 borrow instead if owning the value isn't necessary",
+                            ),
+                        );
+                    }
                 }
                 let place = &self.move_data.move_paths[mpi].place;
                 let ty = place.ty(self.body, self.infcx.tcx).ty;
@@ -491,9 +541,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                         ClosureKind::Coroutine(CoroutineKind::Desugared(_, CoroutineSource::Block)),
                     ..
                 } = move_spans
+                    && can_suggest_clone
                 {
                     self.suggest_cloning(err, ty, expr, None, Some(move_spans));
-                } else if self.suggest_hoisting_call_outside_loop(err, expr) {
+                } else if self.suggest_hoisting_call_outside_loop(err, expr) && can_suggest_clone {
                     // The place where the type moves would be misleading to suggest clone.
                     // #121466
                     self.suggest_cloning(err, ty, expr, None, Some(move_spans));

library/core/src/borrow.rs

@@ -184,6 +184,7 @@ pub trait Borrow<Borrowed: ?Sized> {
 /// an underlying type by providing a mutable reference. See [`Borrow<T>`]
 /// for more information on borrowing as another type.
 #[stable(feature = "rust1", since = "1.0.0")]
+#[rustc_diagnostic_item = "BorrowMut"]
 pub trait BorrowMut<Borrowed: ?Sized>: Borrow<Borrowed> {
     /// Mutably borrows from an owned value.
     ///

tests/ui/moves/moved-value-on-as-ref-arg.fixed

@@ -0,0 +1,37 @@
+//@ run-rustfix
+#![allow(unused_mut)]
+use std::borrow::{Borrow, BorrowMut};
+use std::convert::{AsMut, AsRef};
+struct Bar;
+
+impl AsRef<Bar> for Bar {
+    fn as_ref(&self) -> &Bar {
+        self
+    }
+}
+
+impl AsMut<Bar> for Bar {
+    fn as_mut(&mut self) -> &mut Bar {
+        self
+    }
+}
+
+fn foo<T: AsRef<Bar>>(_: T) {}
+fn qux<T: AsMut<Bar>>(_: T) {}
+fn bat<T: Borrow<T>>(_: T) {}
+fn baz<T: BorrowMut<T>>(_: T) {}
+
+pub fn main() {
+    let bar = Bar;
+    foo(&bar);
+    let _baa = bar; //~ ERROR use of moved value
+    let mut bar = Bar;
+    qux(&mut bar);
+    let _baa = bar; //~ ERROR use of moved value
+    let bar = Bar;
+    bat(&bar);
+    let _baa = bar; //~ ERROR use of moved value
+    let mut bar = Bar;
+    baz(&mut bar);
+    let _baa = bar; //~ ERROR use of moved value
+}

tests/ui/moves/moved-value-on-as-ref-arg.rs

@@ -0,0 +1,37 @@
+//@ run-rustfix
+#![allow(unused_mut)]
+use std::borrow::{Borrow, BorrowMut};
+use std::convert::{AsMut, AsRef};
+struct Bar;
+
+impl AsRef<Bar> for Bar {
+    fn as_ref(&self) -> &Bar {
+        self
+    }
+}
+
+impl AsMut<Bar> for Bar {
+    fn as_mut(&mut self) -> &mut Bar {
+        self
+    }
+}
+
+fn foo<T: AsRef<Bar>>(_: T) {}
+fn qux<T: AsMut<Bar>>(_: T) {}
+fn bat<T: Borrow<T>>(_: T) {}
+fn baz<T: BorrowMut<T>>(_: T) {}
+
+pub fn main() {
+    let bar = Bar;
+    foo(bar);
+    let _baa = bar; //~ ERROR use of moved value
+    let mut bar = Bar;
+    qux(bar);
+    let _baa = bar; //~ ERROR use of moved value
+    let bar = Bar;
+    bat(bar);
+    let _baa = bar; //~ ERROR use of moved value
+    let mut bar = Bar;
+    baz(bar);
+    let _baa = bar; //~ ERROR use of moved value
+}

tests/ui/moves/moved-value-on-as-ref-arg.stderr

@@ -0,0 +1,79 @@
+error[E0382]: use of moved value: `bar`
+  --> $DIR/moved-value-on-as-ref-arg.rs:27:16
+   |
+LL |     let bar = Bar;
+   |         --- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait
+LL |     foo(bar);
+   |         --- value moved here
+LL |     let _baa = bar;
+   |                ^^^ value used here after move
+   |
+help: borrow the value to avoid moving it
+   |
+LL |     foo(&bar);
+   |         +
+
+error[E0382]: use of moved value: `bar`
+  --> $DIR/moved-value-on-as-ref-arg.rs:30:16
+   |
+LL |     let mut bar = Bar;
+   |         ------- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait
+LL |     qux(bar);
+   |         --- value moved here
+LL |     let _baa = bar;
+   |                ^^^ value used here after move
+   |
+note: if `Bar` implemented `Clone`, you could clone the value
+  --> $DIR/moved-value-on-as-ref-arg.rs:5:1
+   |
+LL | struct Bar;
+   | ^^^^^^^^^^ consider implementing `Clone` for this type
+...
+LL |     qux(bar);
+   |         --- you could clone this value
+help: borrow the value to avoid moving it
+   |
+LL |     qux(&mut bar);
+   |         ++++
+
+error[E0382]: use of moved value: `bar`
+  --> $DIR/moved-value-on-as-ref-arg.rs:33:16
+   |
+LL |     let bar = Bar;
+   |         --- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait
+LL |     bat(bar);
+   |         --- value moved here
+LL |     let _baa = bar;
+   |                ^^^ value used here after move
+   |
+help: borrow the value to avoid moving it
+   |
+LL |     bat(&bar);
+   |         +
+
+error[E0382]: use of moved value: `bar`
+  --> $DIR/moved-value-on-as-ref-arg.rs:36:16
+   |
+LL |     let mut bar = Bar;
+   |         ------- move occurs because `bar` has type `Bar`, which does not implement the `Copy` trait
+LL |     baz(bar);
+   |         --- value moved here
+LL |     let _baa = bar;
+   |                ^^^ value used here after move
+   |
+note: if `Bar` implemented `Clone`, you could clone the value
+  --> $DIR/moved-value-on-as-ref-arg.rs:5:1
+   |
+LL | struct Bar;
+   | ^^^^^^^^^^ consider implementing `Clone` for this type
+...
+LL |     baz(bar);
+   |         --- you could clone this value
+help: borrow the value to avoid moving it
+   |
+LL |     baz(&mut bar);
+   |         ++++
+
+error: aborting due to 4 previous errors
+
+For more information about this error, try `rustc --explain E0382`.