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

6 files changed, 154 insertions, 17 deletions

diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
index 6a4d41ffc1a..d0901204061 100644
--- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
+++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
@@ -686,17 +686,42 @@ 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::Pin,
+            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 +738,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 +755,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 +770,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()`,
diff --git a/src/test/ui/suggestions/imm-ref-trait-object-literal.stderr b/src/test/ui/suggestions/imm-ref-trait-object-literal.stderr
index df483b3912d..3cf74410569 100644
--- a/src/test/ui/suggestions/imm-ref-trait-object-literal.stderr
+++ b/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 borrowing mutably here: `&mut s`
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/suggestions/issue-84973-2.rs b/src/test/ui/suggestions/issue-84973-2.rs
new file mode 100644
index 00000000000..050cf8c64b3
--- /dev/null
+++ b/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]
+}
diff --git a/src/test/ui/suggestions/issue-84973-2.stderr b/src/test/ui/suggestions/issue-84973-2.stderr
new file mode 100644
index 00000000000..6394e58a2a9
--- /dev/null
+++ b/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 borrowing mutably here: `&mut a`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0277`.
diff --git a/src/test/ui/suggestions/issue-84973.rs b/src/test/ui/suggestions/issue-84973.rs
new file mode 100644
index 00000000000..42468478ed9
--- /dev/null
+++ b/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,
+        }
+    }
+}
diff --git a/src/test/ui/suggestions/issue-84973.stderr b/src/test/ui/suggestions/issue-84973.stderr
new file mode 100644
index 00000000000..49fa94da859
--- /dev/null
+++ b/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`.