Rollup merge of #85369 - FabianWolff:issue-84973, r=jackh726

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

This pull request fixes #84973 by suggesting to borrow if a trait is not implemented for some type `T`, but it is for `&T` or `&mut T`. For instance:
```rust
trait Ti {}
impl<T> Ti for &T {}
fn foo<T: Ti>(_: T) {}

trait Tm {}
impl<T> Tm for &mut T {}
fn bar<T: Tm>(_: T) {}

fn main() {
    let a: i32 = 5;
    foo(a);

    let b: Box<i32> = Box::new(42);
    bar(b);
}
```
gives, on current nightly:
```
error[E0277]: the trait bound `i32: Ti` is not satisfied
  --> t2.rs:11:9
   |
3  | fn foo<T: Ti>(_: T) {}
   |           -- required by this bound in `foo`
...
11 |     foo(a);
   |         ^ the trait `Ti` is not implemented for `i32`

error[E0277]: the trait bound `Box<i32>: Tm` is not satisfied
  --> t2.rs:14:9
   |
7  | fn bar<T: Tm>(_: T) {}
   |           -- required by this bound in `bar`
...
14 |     bar(b);
   |         ^ the trait `Tm` is not implemented for `Box<i32>`

error: aborting due to 2 previous errors
```
whereas with my changes, I get:
```
error[E0277]: the trait bound `i32: Ti` is not satisfied
  --> t2.rs:11:9
   |
3  | fn foo<T: Ti>(_: T) {}
   |           -- required by this bound in `foo`
...
11 |     foo(a);
   |         ^
   |         |
   |         expected an implementor of trait `Ti`
   |         help: consider borrowing here: `&a`

error[E0277]: the trait bound `Box<i32>: Tm` is not satisfied
  --> t2.rs:14:9
   |
7  | fn bar<T: Tm>(_: T) {}
   |           -- required by this bound in `bar`
...
14 |     bar(b);
   |         ^
   |         |
   |         expected an implementor of trait `Tm`
   |         help: consider borrowing mutably here: `&mut b`

error: aborting due to 2 previous errors
```
In my implementation, I have added a "blacklist" to make these suggestions flexible. In particular, suggesting to borrow can interfere with other suggestions, such as to add another trait bound to a generic argument. I have tried to configure this blacklist to cause the least amount of test case failures, i.e. to model the current behavior as closely as possible (I only had to change one existing test case, and this change was quite clearly an improvement).

Author: GuillaumeGomez

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`.