Suggest borrowing when it would satisfy an unmet trait bound

When there are multiple implementors for the same trait that is present
in an unmet binding, modify the E0277 error to refer to the parent
obligation and verify whether borrowing the argument being passed in
would satisfy the unmet bound. If it would, suggest it.

10 files changed, 107 insertions, 24 deletions

diff --git a/src/libcore/ops/function.rs b/src/libcore/ops/function.rs
index 4a0a2720fe4..35790324a2f 100644
--- a/src/libcore/ops/function.rs
+++ b/src/libcore/ops/function.rs
@@ -137,10 +137,6 @@ pub trait Fn<Args> : FnMut<Args> {
 #[rustc_paren_sugar]
 #[rustc_on_unimplemented(
     on(Args="()", note="wrap the `{Self}` in a closure with no arguments: `|| {{ /* code */ }}"),
-    on(
-        all(Args="(char,)", _Self="std::string::String"),
-        note="borrowing the `{Self}` might fix the problem"
-    ),
     message="expected a `{FnMut}<{Args}>` closure, found `{Self}`",
     label="expected an `FnMut<{Args}>` closure, found `{Self}`",
 )]
diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs
index 697bb3f467c..f511862b37b 100644
--- a/src/librustc/traits/error_reporting.rs
+++ b/src/librustc/traits/error_reporting.rs
@@ -33,7 +33,7 @@ use crate::ty::subst::Subst;
 use crate::ty::SubtypePredicate;
 use crate::util::nodemap::{FxHashMap, FxHashSet};
 
-use errors::{Applicability, DiagnosticBuilder, pluralize};
+use errors::{Applicability, DiagnosticBuilder, pluralise, Style};
 use std::fmt;
 use syntax::ast;
 use syntax::symbol::{sym, kw};
@@ -723,7 +723,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                             post_message,
                             pre_message,
                         ) = self.get_parent_trait_ref(&obligation.cause.code)
-                                .map(|t| (format!(" in `{}`", t), format!("within `{}`, ", t)))
+                            .map(|t| (format!(" in `{}`", t), format!("within `{}`, ", t)))
                             .unwrap_or_default();
 
                         let OnUnimplementedNote {
@@ -787,6 +787,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                         self.suggest_borrow_on_unsized_slice(&obligation.cause.code, &mut err);
                         self.suggest_fn_call(&obligation, &mut err, &trait_ref, points_at_arg);
                         self.suggest_remove_reference(&obligation, &mut err, &trait_ref);
+                        if self.suggest_add_reference_to_arg(
+                            &obligation,
+                            &mut err,
+                            &trait_ref,
+                            points_at_arg,
+                        ) {
+                            self.note_obligation_cause(&mut err, obligation);
+                            err.emit();
+                            return;
+                        }
                         self.suggest_semicolon_removal(&obligation, &mut err, span, &trait_ref);
 
                         // Try to report a help message
@@ -1302,6 +1312,66 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         }
     }
 
+    fn suggest_add_reference_to_arg(
+        &self,
+        obligation: &PredicateObligation<'tcx>,
+        err: &mut DiagnosticBuilder<'tcx>,
+        trait_ref: &ty::Binder<ty::TraitRef<'tcx>>,
+        points_at_arg: bool,
+    ) -> bool {
+        if !points_at_arg {
+            return false;
+        }
+
+        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 new_obligation = Obligation::new(
+                ObligationCause::dummy(),
+                param_env,
+                new_trait_ref.to_predicate(),
+            );
+            if self.predicate_may_hold(&new_obligation) {
+                if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
+                    // We have a very specific type of error, where just borrowing this argument
+                    // might solve the problem. In cases like this, the important part is the
+                    // original type obligation, not the last one that failed, which is arbitrary.
+                    // Because of this, we modify the error to refer to the original obligation and
+                    // return early in the caller.
+                    err.message = vec![(
+                        format!(
+                            "the trait bound `{}: {}` is not satisfied",
+                            found,
+                            obligation.parent_trait_ref.skip_binder(),
+                        ),
+                        Style::NoStyle,
+                    )];
+                    if snippet.starts_with('&') {
+                        // This is already a literal borrow and the obligation is failing
+                        // somewhere else in the obligation chain. Do not suggest non-sense.
+                        return false;
+                    }
+                    err.span_suggestion(
+                        span,
+                        "consider borrowing here",
+                        format!("&{}", snippet),
+                        Applicability::MachineApplicable,
+                    );
+                    return true;
+                }
+            }
+        }
+        false
+    }
+
     /// Whenever references are used by mistake, like `for (i, e) in &vec.iter().enumerate()`,
     /// suggest removing these references until we reach a type that implements the trait.
     fn suggest_remove_reference(
diff --git a/src/test/ui/derives/deriving-copyclone.rs b/src/test/ui/derives/deriving-copyclone.rs
index 4565412bff7..06b3157a77a 100644
--- a/src/test/ui/derives/deriving-copyclone.rs
+++ b/src/test/ui/derives/deriving-copyclone.rs
@@ -28,10 +28,10 @@ fn main() {
     is_clone(B { a: 1, b: 2 });
 
     // B<C> cannot be copied or cloned
-    is_copy(B { a: 1, b: C }); //~ERROR Copy
-    is_clone(B { a: 1, b: C }); //~ERROR Clone
+    is_copy(B { a: 1, b: C }); //~ ERROR Copy
+    is_clone(B { a: 1, b: C }); //~ ERROR Clone
 
     // B<D> can be cloned but not copied
-    is_copy(B { a: 1, b: D }); //~ERROR Copy
+    is_copy(B { a: 1, b: D }); //~ ERROR Copy
     is_clone(B { a: 1, b: D });
 }
diff --git a/src/test/ui/derives/deriving-copyclone.stderr b/src/test/ui/derives/deriving-copyclone.stderr
index 4cca14ae089..561706469c6 100644
--- a/src/test/ui/derives/deriving-copyclone.stderr
+++ b/src/test/ui/derives/deriving-copyclone.stderr
@@ -5,7 +5,10 @@ LL | fn is_copy<T: Copy>(_: T) {}
    |    -------    ---- required by this bound in `is_copy`
 ...
 LL |     is_copy(B { a: 1, b: C });
-   |             ^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `C`
+   |             ^^^^^^^^^^^^^^^^
+   |             |
+   |             the trait `std::marker::Copy` is not implemented for `C`
+   |             help: consider borrowing here: `&B { a: 1, b: C }`
    |
    = note: required because of the requirements on the impl of `std::marker::Copy` for `B<C>`
 
@@ -16,7 +19,10 @@ LL | fn is_clone<T: Clone>(_: T) {}
    |    --------    ----- required by this bound in `is_clone`
 ...
 LL |     is_clone(B { a: 1, b: C });
-   |              ^^^^^^^^^^^^^^^^ the trait `std::clone::Clone` is not implemented for `C`
+   |              ^^^^^^^^^^^^^^^^
+   |              |
+   |              the trait `std::clone::Clone` is not implemented for `C`
+   |              help: consider borrowing here: `&B { a: 1, b: C }`
    |
    = note: required because of the requirements on the impl of `std::clone::Clone` for `B<C>`
 
@@ -27,7 +33,10 @@ LL | fn is_copy<T: Copy>(_: T) {}
    |    -------    ---- required by this bound in `is_copy`
 ...
 LL |     is_copy(B { a: 1, b: D });
-   |             ^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `D`
+   |             ^^^^^^^^^^^^^^^^
+   |             |
+   |             the trait `std::marker::Copy` is not implemented for `D`
+   |             help: consider borrowing here: `&B { a: 1, b: D }`
    |
    = note: required because of the requirements on the impl of `std::marker::Copy` for `B<D>`
 
diff --git a/src/test/ui/kindck/kindck-impl-type-params-2.rs b/src/test/ui/kindck/kindck-impl-type-params-2.rs
index ac9cc1a08f3..d5fcc68a759 100644
--- a/src/test/ui/kindck/kindck-impl-type-params-2.rs
+++ b/src/test/ui/kindck/kindck-impl-type-params-2.rs
@@ -11,5 +11,5 @@ fn take_param<T:Foo>(foo: &T) { }
 fn main() {
     let x: Box<_> = box 3;
     take_param(&x);
-    //~^ ERROR `std::boxed::Box<{integer}>: std::marker::Copy` is not satisfied
+    //~^ ERROR the trait bound `std::boxed::Box<{integer}>: Foo` is not satisfied
 }
diff --git a/src/test/ui/kindck/kindck-impl-type-params-2.stderr b/src/test/ui/kindck/kindck-impl-type-params-2.stderr
index 8e989113244..318b7b0f10a 100644
--- a/src/test/ui/kindck/kindck-impl-type-params-2.stderr
+++ b/src/test/ui/kindck/kindck-impl-type-params-2.stderr
@@ -1,4 +1,4 @@
-error[E0277]: the trait bound `std::boxed::Box<{integer}>: std::marker::Copy` is not satisfied
+error[E0277]: the trait bound `std::boxed::Box<{integer}>: Foo` is not satisfied
   --> $DIR/kindck-impl-type-params-2.rs:13:16
    |
 LL | fn take_param<T:Foo>(foo: &T) { }
diff --git a/src/test/ui/kindck/kindck-inherited-copy-bound.curr.stderr b/src/test/ui/kindck/kindck-inherited-copy-bound.curr.stderr
index a93f4686496..da1a7a7520e 100644
--- a/src/test/ui/kindck/kindck-inherited-copy-bound.curr.stderr
+++ b/src/test/ui/kindck/kindck-inherited-copy-bound.curr.stderr
@@ -1,4 +1,4 @@
-error[E0277]: the trait bound `std::boxed::Box<{integer}>: std::marker::Copy` is not satisfied
+error[E0277]: the trait bound `std::boxed::Box<{integer}>: Foo` is not satisfied
   --> $DIR/kindck-inherited-copy-bound.rs:21:16
    |
 LL | fn take_param<T:Foo>(foo: &T) { }
diff --git a/src/test/ui/suggestions/issue-62843.stderr b/src/test/ui/suggestions/issue-62843.stderr
index b5801e9162f..9535bf1b2ce 100644
--- a/src/test/ui/suggestions/issue-62843.stderr
+++ b/src/test/ui/suggestions/issue-62843.stderr
@@ -1,11 +1,13 @@
-error[E0277]: expected a `std::ops::FnMut<(char,)>` closure, found `std::string::String`
+error[E0277]: the trait bound `std::string::String: std::str::pattern::Pattern<'_>` is not satisfied
   --> $DIR/issue-62843.rs:4:32
    |
 LL |     println!("{:?}", line.find(pattern));
-   |                                ^^^^^^^ expected an `FnMut<(char,)>` closure, found `std::string::String`
+   |                                ^^^^^^^
+   |                                |
+   |                                expected an `FnMut<(char,)>` closure, found `std::string::String`
+   |                                help: consider borrowing here: `&pattern`
    |
    = help: the trait `std::ops::FnMut<(char,)>` is not implemented for `std::string::String`
-   = note: borrowing the `std::string::String` might fix the problem
    = note: required because of the requirements on the impl of `std::str::pattern::Pattern<'_>` for `std::string::String`
 
 error: aborting due to previous error
diff --git a/src/test/ui/traits/traits-negative-impls.rs b/src/test/ui/traits/traits-negative-impls.rs
index fb9a3a99748..8ed39986781 100644
--- a/src/test/ui/traits/traits-negative-impls.rs
+++ b/src/test/ui/traits/traits-negative-impls.rs
@@ -46,7 +46,7 @@ fn dummy2() {
     impl !Send for TestType {}
 
     is_send(Box::new(TestType));
-    //~^ ERROR `dummy2::TestType` cannot be sent between threads safely
+    //~^ ERROR the trait bound `dummy2::TestType: std::marker::Send` is not satisfied
 }
 
 fn dummy3() {
@@ -64,5 +64,5 @@ fn main() {
     // This will complain about a missing Send impl because `Sync` is implement *just*
     // for T that are `Send`. Look at #20366 and #19950
     is_sync(Outer2(TestType));
-    //~^ ERROR `main::TestType` cannot be sent between threads safely
+    //~^ ERROR the trait bound `main::TestType: std::marker::Sync` is not satisfied
 }
diff --git a/src/test/ui/traits/traits-negative-impls.stderr b/src/test/ui/traits/traits-negative-impls.stderr
index 22b6d2a0c4e..448bb90205d 100644
--- a/src/test/ui/traits/traits-negative-impls.stderr
+++ b/src/test/ui/traits/traits-negative-impls.stderr
@@ -43,14 +43,17 @@ LL |     is_send((8, TestType));
    = help: within `({integer}, dummy1c::TestType)`, the trait `std::marker::Send` is not implemented for `dummy1c::TestType`
    = note: required because it appears within the type `({integer}, dummy1c::TestType)`
 
-error[E0277]: `dummy2::TestType` cannot be sent between threads safely
+error[E0277]: the trait bound `dummy2::TestType: std::marker::Send` is not satisfied
   --> $DIR/traits-negative-impls.rs:48:13
    |
 LL | fn is_send<T: Send>(_: T) {}
    |    -------    ---- required by this bound in `is_send`
 ...
 LL |     is_send(Box::new(TestType));
-   |             ^^^^^^^^^^^^^^^^^^ `dummy2::TestType` cannot be sent between threads safely
+   |             ^^^^^^^^^^^^^^^^^^
+   |             |
+   |             `dummy2::TestType` cannot be sent between threads safely
+   |             help: consider borrowing here: `&Box::new(TestType)`
    |
    = help: the trait `std::marker::Send` is not implemented for `dummy2::TestType`
    = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<dummy2::TestType>`
@@ -70,14 +73,17 @@ LL |     is_send(Box::new(Outer2(TestType)));
    = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<Outer2<dummy3::TestType>>`
    = note: required because it appears within the type `std::boxed::Box<Outer2<dummy3::TestType>>`
 
-error[E0277]: `main::TestType` cannot be sent between threads safely
+error[E0277]: the trait bound `main::TestType: std::marker::Sync` is not satisfied
   --> $DIR/traits-negative-impls.rs:66:13
    |
 LL | fn is_sync<T: Sync>(_: T) {}
    |    -------    ---- required by this bound in `is_sync`
 ...
 LL |     is_sync(Outer2(TestType));
-   |             ^^^^^^^^^^^^^^^^ `main::TestType` cannot be sent between threads safely
+   |             ^^^^^^^^^^^^^^^^
+   |             |
+   |             `main::TestType` cannot be sent between threads safely
+   |             help: consider borrowing here: `&Outer2(TestType)`
    |
    = help: the trait `std::marker::Send` is not implemented for `main::TestType`
    = note: required because of the requirements on the impl of `std::marker::Sync` for `Outer2<main::TestType>`