Auto merge of #65456 - estebank:trait-bound-borrow, r=matthewjasper

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.

Fix #56368.

Author: bors

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}`",
 )]

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, pluralize, Style};
 use std::fmt;
 use syntax::ast;
 use syntax::symbol::{sym, kw};
@@ -713,20 +713,24 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                 }
                 match obligation.predicate {
                     ty::Predicate::Trait(ref trait_predicate) => {
-                        let trait_predicate =
-                            self.resolve_vars_if_possible(trait_predicate);
+                        let trait_predicate = self.resolve_vars_if_possible(trait_predicate);
 
                         if self.tcx.sess.has_errors() && trait_predicate.references_error() {
                             return;
                         }
                         let trait_ref = trait_predicate.to_poly_trait_ref();
-                        let (post_message, pre_message) =
-                            self.get_parent_trait_ref(&obligation.cause.code)
-                                .map(|t| (format!(" in `{}`", t), format!("within `{}`, ", t)))
+                        let (
+                            post_message,
+                            pre_message,
+                        ) = self.get_parent_trait_ref(&obligation.cause.code)
+                            .map(|t| (format!(" in `{}`", t), format!("within `{}`, ", t)))
                             .unwrap_or_default();
 
-                        let OnUnimplementedNote { message, label, note }
-                            = self.on_unimplemented_note(trait_ref, obligation);
+                        let OnUnimplementedNote {
+                            message,
+                            label,
+                            note,
+                        } = self.on_unimplemented_note(trait_ref, obligation);
                         let have_alt_message = message.is_some() || label.is_some();
                         let is_try = self.tcx.sess.source_map().span_to_snippet(span)
                             .map(|s| &s == "?")
@@ -767,6 +771,17 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                                 )
                             };
 
+                        if self.suggest_add_reference_to_arg(
+                            &obligation,
+                            &mut err,
+                            &trait_ref,
+                            points_at_arg,
+                            have_alt_message,
+                        ) {
+                            self.note_obligation_cause(&mut err, obligation);
+                            err.emit();
+                            return;
+                        }
                         if let Some(ref s) = label {
                             // If it has a custom `#[rustc_on_unimplemented]`
                             // error message, let's display it as the label!
@@ -1298,6 +1313,73 @@ 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,
+        has_custom_message: 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_must_hold_modulo_regions(&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.
+                    let msg = format!(
+                        "the trait bound `{}: {}` is not satisfied",
+                        found,
+                        obligation.parent_trait_ref.skip_binder(),
+                    );
+                    if has_custom_message {
+                        err.note(&msg);
+                    } else {
+                        err.message = vec![(msg, 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_label(span, &format!(
+                        "expected an implementor of trait `{}`",
+                        obligation.parent_trait_ref.skip_binder(),
+                    ));
+                    err.span_suggestion(
+                        span,
+                        "consider borrowing here",
+                        format!("&{}", snippet),
+                        Applicability::MaybeIncorrect,
+                    );
+                    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(

src/librustc_errors/lib.rs

@@ -38,6 +38,7 @@ pub mod registry;
 mod styled_buffer;
 mod lock;
 pub mod json;
+pub use snippet::Style;
 
 pub type PResult<'a, T> = Result<T, DiagnosticBuilder<'a>>;
 

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 });
 }

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`
+   |             ^^^^^^^^^^^^^^^^
+   |             |
+   |             expected an implementor of trait `std::marker::Copy`
+   |             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`
+   |              ^^^^^^^^^^^^^^^^
+   |              |
+   |              expected an implementor of trait `std::clone::Clone`
+   |              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`
+   |             ^^^^^^^^^^^^^^^^
+   |             |
+   |             expected an implementor of trait `std::marker::Copy`
+   |             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>`
 

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
 }

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) { }

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) { }

src/test/ui/kindck/kindck-inherited-copy-bound.object_safe_for_dispatch.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) { }

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

@@ -2,10 +2,12 @@ error[E0277]: expected a `std::ops::FnMut<(char,)>` closure, found `std::string:
   --> $DIR/issue-62843.rs:4:32
    |
 LL |     println!("{:?}", line.find(pattern));
-   |                                ^^^^^^^ expected an `FnMut<(char,)>` closure, found `std::string::String`
+   |                                ^^^^^^^
+   |                                |
+   |                                expected an implementor of trait `std::str::pattern::Pattern<'_>`
+   |                                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: the trait bound `std::string::String: std::str::pattern::Pattern<'_>` is not satisfied
    = note: required because of the requirements on the impl of `std::str::pattern::Pattern<'_>` for `std::string::String`
 
 error: aborting due to previous error