Suggest borrowing if a trait implementation is found for &/&mut <type>

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -686,17 +686,36 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
             return false;
         }
 
+        // Blacklist traits for which it would be nonsensical to suggest borrowing.
+        // For instance, immutable references are always Copy, so suggesting to
+        // borrow would always succeed, but it's probably not what the user wanted.
+        let blacklist: Vec<_> =
+            [LangItem::Copy, LangItem::Clone, LangItem::Unpin, LangItem::Sized, LangItem::Send]
+                .iter()
+                .filter_map(|lang_item| self.tcx.lang_items().require(*lang_item).ok())
+                .collect();
+
         let span = obligation.cause.span;
         let param_env = obligation.param_env;
         let trait_ref = trait_ref.skip_binder();
 
-        if let ObligationCauseCode::ImplDerivedObligation(obligation) = &obligation.cause.code {
-            // Try to apply the original trait binding obligation by borrowing.
-            let self_ty = trait_ref.self_ty();
-            let found = self_ty.to_string();
-            let new_self_ty = self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, self_ty);
-            let substs = self.tcx.mk_substs_trait(new_self_ty, &[]);
-            let new_trait_ref = ty::TraitRef::new(obligation.parent_trait_ref.def_id(), substs);
+        let found_ty = trait_ref.self_ty();
+        let found_ty_str = found_ty.to_string();
+        let imm_borrowed_found_ty = self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, found_ty);
+        let imm_substs = self.tcx.mk_substs_trait(imm_borrowed_found_ty, &[]);
+        let mut_borrowed_found_ty = self.tcx.mk_mut_ref(self.tcx.lifetimes.re_static, found_ty);
+        let mut_substs = self.tcx.mk_substs_trait(mut_borrowed_found_ty, &[]);
+
+        // Try to apply the original trait binding obligation by borrowing.
+        let mut try_borrowing = |new_trait_ref: ty::TraitRef<'tcx>,
+                                 expected_trait_ref: ty::TraitRef<'tcx>,
+                                 mtbl: bool,
+                                 blacklist: &[DefId]|
+         -> bool {
+            if blacklist.contains(&expected_trait_ref.def_id) {
+                return false;
+            }
+
             let new_obligation = Obligation::new(
                 ObligationCause::dummy(),
                 param_env,
@@ -713,8 +732,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
 
                     let msg = format!(
                         "the trait bound `{}: {}` is not satisfied",
-                        found,
-                        obligation.parent_trait_ref.skip_binder().print_only_trait_path(),
+                        found_ty_str,
+                        expected_trait_ref.print_only_trait_path(),
                     );
                     if has_custom_message {
                         err.note(&msg);
@@ -730,7 +749,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                         span,
                         &format!(
                             "expected an implementor of trait `{}`",
-                            obligation.parent_trait_ref.skip_binder().print_only_trait_path(),
+                            expected_trait_ref.print_only_trait_path(),
                         ),
                     );
 
@@ -745,16 +764,52 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
 
                         err.span_suggestion(
                             span,
-                            "consider borrowing here",
-                            format!("&{}", snippet),
+                            &format!(
+                                "consider{} borrowing here",
+                                if mtbl { " mutably" } else { "" }
+                            ),
+                            format!("&{}{}", if mtbl { "mut " } else { "" }, snippet),
                             Applicability::MaybeIncorrect,
                         );
                     }
                     return true;
                 }
             }
+            return false;
+        };
+
+        if let ObligationCauseCode::ImplDerivedObligation(obligation) = &obligation.cause.code {
+            let expected_trait_ref = obligation.parent_trait_ref.skip_binder();
+            let new_imm_trait_ref =
+                ty::TraitRef::new(obligation.parent_trait_ref.def_id(), imm_substs);
+            let new_mut_trait_ref =
+                ty::TraitRef::new(obligation.parent_trait_ref.def_id(), mut_substs);
+            if try_borrowing(new_imm_trait_ref, expected_trait_ref, false, &[]) {
+                return true;
+            } else {
+                return try_borrowing(new_mut_trait_ref, expected_trait_ref, true, &[]);
+            }
+        } else if let ObligationCauseCode::BindingObligation(_, _)
+        | ObligationCauseCode::ItemObligation(_) = &obligation.cause.code
+        {
+            if try_borrowing(
+                ty::TraitRef::new(trait_ref.def_id, imm_substs),
+                trait_ref,
+                false,
+                &blacklist[..],
+            ) {
+                return true;
+            } else {
+                return try_borrowing(
+                    ty::TraitRef::new(trait_ref.def_id, mut_substs),
+                    trait_ref,
+                    true,
+                    &blacklist[..],
+                );
+            }
+        } else {
+            false
         }
-        false
     }
 
     /// Whenever references are used by mistake, like `for (i, e) in &vec.iter().enumerate()`,

src/test/ui/suggestions/imm-ref-trait-object-literal.stderr

@@ -21,10 +21,10 @@ LL | fn foo<X: Trait>(_: X) {}
    |           ----- required by this bound in `foo`
 ...
 LL |   foo(s);
-   |       ^ the trait `Trait` is not implemented for `S`
-   |
-   = help: the following implementations were found:
-             <&'a mut S as Trait>
+   |       ^
+   |       |
+   |       expected an implementor of trait `Trait`
+   |       help: consider mutably borrowing here: `&mut s`
 
 error: aborting due to 2 previous errors
 

src/test/ui/suggestions/issue-84973-2.rs

@@ -0,0 +1,13 @@
+// A slight variation of issue-84973.rs. Here, a mutable borrow is
+// required (and the obligation kind is different).
+
+trait Tr {}
+impl Tr for &mut i32 {}
+
+fn foo<T: Tr>(i: T) {}
+
+fn main() {
+    let a: i32 = 32;
+    foo(a);
+    //~^ ERROR: the trait bound `i32: Tr` is not satisfied [E0277]
+}

src/test/ui/suggestions/issue-84973-2.stderr

@@ -0,0 +1,15 @@
+error[E0277]: the trait bound `i32: Tr` is not satisfied
+  --> $DIR/issue-84973-2.rs:11:9
+   |
+LL | fn foo<T: Tr>(i: T) {}
+   |           -- required by this bound in `foo`
+...
+LL |     foo(a);
+   |         ^
+   |         |
+   |         expected an implementor of trait `Tr`
+   |         help: consider mutably borrowing here: `&mut a`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0277`.

src/test/ui/suggestions/issue-84973-blacklist.rs

@@ -0,0 +1,29 @@
+// Checks that certain traits for which we don't want to suggest borrowing
+// are blacklisted and don't cause the suggestion to be issued.
+
+#![feature(generators)]
+
+fn f_copy<T: Copy>(t: T) {}
+fn f_clone<T: Clone>(t: T) {}
+fn f_unpin<T: Unpin>(t: T) {}
+fn f_sized<T: Sized>(t: T) {}
+fn f_send<T: Send>(t: T) {}
+
+struct S;
+
+fn main() {
+    f_copy("".to_string()); //~ ERROR: the trait bound `String: Copy` is not satisfied [E0277]
+    f_clone(S); //~ ERROR: the trait bound `S: Clone` is not satisfied [E0277]
+    f_unpin(static || { yield; });
+    //~^ ERROR: cannot be unpinned [E0277]
+
+    let cl = || ();
+    let ref_cl: &dyn Fn() -> () = &cl;
+    f_sized(*ref_cl);
+    //~^ ERROR: the size for values of type `dyn Fn()` cannot be known at compilation time [E0277]
+    //~| ERROR: the size for values of type `dyn Fn()` cannot be known at compilation time [E0277]
+
+    use std::rc::Rc;
+    let rc = Rc::new(0);
+    f_send(rc); //~ ERROR: `Rc<{integer}>` cannot be sent between threads safely [E0277]
+}

src/test/ui/suggestions/issue-84973-blacklist.stderr

@@ -0,0 +1,64 @@
+error[E0277]: the trait bound `String: Copy` is not satisfied
+  --> $DIR/issue-84973-blacklist.rs:15:12
+   |
+LL | fn f_copy<T: Copy>(t: T) {}
+   |              ---- required by this bound in `f_copy`
+...
+LL |     f_copy("".to_string());
+   |            ^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `String`
+
+error[E0277]: the trait bound `S: Clone` is not satisfied
+  --> $DIR/issue-84973-blacklist.rs:16:13
+   |
+LL | fn f_clone<T: Clone>(t: T) {}
+   |               ----- required by this bound in `f_clone`
+...
+LL |     f_clone(S);
+   |             ^ the trait `Clone` is not implemented for `S`
+
+error[E0277]: `[static generator@$DIR/issue-84973-blacklist.rs:17:13: 17:33]` cannot be unpinned
+  --> $DIR/issue-84973-blacklist.rs:17:5
+   |
+LL | fn f_unpin<T: Unpin>(t: T) {}
+   |               ----- required by this bound in `f_unpin`
+...
+LL |     f_unpin(static || { yield; });
+   |     ^^^^^^^ the trait `Unpin` is not implemented for `[static generator@$DIR/issue-84973-blacklist.rs:17:13: 17:33]`
+   |
+   = note: consider using `Box::pin`
+
+error[E0277]: the size for values of type `dyn Fn()` cannot be known at compilation time
+  --> $DIR/issue-84973-blacklist.rs:22:13
+   |
+LL | fn f_sized<T: Sized>(t: T) {}
+   |            - required by this bound in `f_sized`
+...
+LL |     f_sized(*ref_cl);
+   |             ^^^^^^^ doesn't have a size known at compile-time
+   |
+   = help: the trait `Sized` is not implemented for `dyn Fn()`
+
+error[E0277]: `Rc<{integer}>` cannot be sent between threads safely
+  --> $DIR/issue-84973-blacklist.rs:28:12
+   |
+LL | fn f_send<T: Send>(t: T) {}
+   |              ---- required by this bound in `f_send`
+...
+LL |     f_send(rc);
+   |            ^^ `Rc<{integer}>` cannot be sent between threads safely
+   |
+   = help: the trait `Send` is not implemented for `Rc<{integer}>`
+
+error[E0277]: the size for values of type `dyn Fn()` cannot be known at compilation time
+  --> $DIR/issue-84973-blacklist.rs:22:5
+   |
+LL |     f_sized(*ref_cl);
+   |     ^^^^^^^ doesn't have a size known at compile-time
+   |
+   = help: the trait `Sized` is not implemented for `dyn Fn()`
+   = note: all function arguments must have a statically known size
+   = help: unsized fn params are gated as an unstable feature
+
+error: aborting due to 6 previous errors
+
+For more information about this error, try `rustc --explain E0277`.

src/test/ui/suggestions/issue-84973-negative.rs

@@ -0,0 +1,12 @@
+// Checks that we only suggest borrowing if &T actually implements the trait.
+
+trait Tr {}
+impl Tr for &f32 {}
+fn bar<T: Tr>(t: T) {}
+
+fn main() {
+    let a = 0i32;
+    let b = 0.0f32;
+    bar(a); //~ ERROR: the trait bound `i32: Tr` is not satisfied [E0277]
+    bar(b); //~ ERROR: the trait bound `f32: Tr` is not satisfied [E0277]
+}

src/test/ui/suggestions/issue-84973-negative.stderr

@@ -0,0 +1,24 @@
+error[E0277]: the trait bound `i32: Tr` is not satisfied
+  --> $DIR/issue-84973-negative.rs:10:9
+   |
+LL | fn bar<T: Tr>(t: T) {}
+   |           -- required by this bound in `bar`
+...
+LL |     bar(a);
+   |         ^ the trait `Tr` is not implemented for `i32`
+
+error[E0277]: the trait bound `f32: Tr` is not satisfied
+  --> $DIR/issue-84973-negative.rs:11:9
+   |
+LL | fn bar<T: Tr>(t: T) {}
+   |           -- required by this bound in `bar`
+...
+LL |     bar(b);
+   |         ^
+   |         |
+   |         expected an implementor of trait `Tr`
+   |         help: consider borrowing here: `&b`
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0277`.

src/test/ui/suggestions/issue-84973.rs

@@ -0,0 +1,33 @@
+// Checks whether borrowing is suggested when a trait bound is not satisfied
+// for found type `T`, but is for `&/&mut T`.
+
+fn main() {
+    let f = Fancy{};
+    let o = Other::new(f);
+    //~^ ERROR: the trait bound `Fancy: SomeTrait` is not satisfied [E0277]
+}
+
+struct Fancy {}
+
+impl <'a> SomeTrait for &'a Fancy {
+}
+
+trait SomeTrait {}
+
+struct Other<'a, G> {
+    a: &'a str,
+    g: G,
+}
+
+// Broadly copied from https://docs.rs/petgraph/0.5.1/src/petgraph/dot.rs.html#70
+impl<'a, G> Other<'a, G>
+where
+    G: SomeTrait,
+{
+    pub fn new(g: G) -> Self {
+        Other {
+            a: "hi",
+            g: g,
+        }
+    }
+}

src/test/ui/suggestions/issue-84973.stderr

@@ -0,0 +1,15 @@
+error[E0277]: the trait bound `Fancy: SomeTrait` is not satisfied
+  --> $DIR/issue-84973.rs:6:24
+   |
+LL |     let o = Other::new(f);
+   |                        ^
+   |                        |
+   |                        expected an implementor of trait `SomeTrait`
+   |                        help: consider borrowing here: `&f`
+...
+LL |     pub fn new(g: G) -> Self {
+   |     ------------------------ required by `Other::<'a, G>::new`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0277`.