Suggest type param trait bound for binop only when appropriate

Verify that the binop trait *is* implemented for the types *if* all the
involved type parameters are replaced with fresh inferred types. When
this is the case, it means that the type parameter was indeed missing a
trait bound. If this is not the case, provide a generic `note` refering
to the type that doesn't implement the expected trait.

8 files changed, 78 insertions, 50 deletions

diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs
index 0184c00c475..e333b706e74 100644
--- a/src/librustc_typeck/check/op.rs
+++ b/src/librustc_typeck/check/op.rs
@@ -8,6 +8,7 @@ use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKi
 use rustc_middle::ty::adjustment::{
     Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability,
 };
+use rustc_middle::ty::fold::TypeFolder;
 use rustc_middle::ty::TyKind::{Adt, Array, Char, FnDef, Never, Ref, Str, Tuple, Uint};
 use rustc_middle::ty::{
     self, suggest_constraining_type_param, Ty, TyCtxt, TypeFoldable, TypeVisitor,
@@ -436,29 +437,36 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         // we don't want the note in the else clause to be emitted
                     } else if let [ty] = &visitor.0[..] {
                         if let ty::Param(p) = ty.kind {
-                            // FIXME: This *guesses* that constraining the type param
-                            // will make the operation available, but this is only true
-                            // when the corresponding trait has a blanket
-                            // implementation, like the following:
-                            // `impl<'a> PartialEq for &'a [T] where T: PartialEq {}`
-                            // The correct thing to do would be to verify this
-                            // projection would hold.
-                            if *ty != lhs_ty {
+                            // Check if the method would be found if the type param wasn't
+                            // involved. If so, it means that adding a trait bound to the param is
+                            // enough. Otherwise we do not give the suggestion.
+                            let mut eraser = TypeParamEraser(&self, expr.span);
+                            let needs_bound = self
+                                .lookup_op_method(
+                                    eraser.fold_ty(lhs_ty),
+                                    &[eraser.fold_ty(rhs_ty)],
+                                    Op::Binary(op, is_assign),
+                                )
+                                .is_ok();
+                            if needs_bound {
+                                suggest_constraining_param(
+                                    self.tcx,
+                                    self.body_id,
+                                    &mut err,
+                                    ty,
+                                    rhs_ty,
+                                    missing_trait,
+                                    p,
+                                    use_output,
+                                );
+                            } else if *ty != lhs_ty {
+                                // When we know that a missing bound is responsible, we don't show
+                                // this note as it is redundant.
                                 err.note(&format!(
                                     "the trait `{}` is not implemented for `{}`",
                                     missing_trait, lhs_ty
                                 ));
                             }
-                            suggest_constraining_param(
-                                self.tcx,
-                                self.body_id,
-                                &mut err,
-                                ty,
-                                rhs_ty,
-                                missing_trait,
-                                p,
-                                use_output,
-                            );
                         } else {
                             bug!("type param visitor stored a non type param: {:?}", ty.kind);
                         }
@@ -656,10 +664,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     );
                     err.span_label(
                         ex.span,
-                        format!(
-                            "cannot apply unary operator `{}`",
-                            op.as_str()
-                        ),
+                        format!("cannot apply unary operator `{}`", op.as_str()),
                     );
                     match actual.kind {
                         Uint(_) if op == hir::UnOp::UnNeg => {
@@ -954,3 +959,21 @@ impl<'tcx> TypeVisitor<'tcx> for TypeParamVisitor<'tcx> {
         ty.super_visit_with(self)
     }
 }
+
+struct TypeParamEraser<'a, 'tcx>(&'a FnCtxt<'a, 'tcx>, Span);
+
+impl TypeFolder<'tcx> for TypeParamEraser<'_, 'tcx> {
+    fn tcx(&self) -> TyCtxt<'tcx> {
+        self.0.tcx
+    }
+
+    fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> {
+        match ty.kind {
+            ty::Param(_) => self.0.next_ty_var(TypeVariableOrigin {
+                kind: TypeVariableOriginKind::MiscVariable,
+                span: self.1,
+            }),
+            _ => ty.super_fold_with(self),
+        }
+    }
+}
diff --git a/src/test/ui/issues/issue-35668.stderr b/src/test/ui/issues/issue-35668.stderr
index c8f1f88a470..600cacc23ae 100644
--- a/src/test/ui/issues/issue-35668.stderr
+++ b/src/test/ui/issues/issue-35668.stderr
@@ -6,7 +6,6 @@ LL |     a.iter().map(|a| a*a)
    |                      |
    |                      &T
    |
-   = note: the trait `std::ops::Mul` is not implemented for `&T`
 help: consider restricting type parameter `T`
    |
 LL | fn func<'a, T: std::ops::Mul<Output = &T>>(a: &'a [T]) -> impl Iterator<Item=&'a T> {
diff --git a/src/test/ui/suggestions/invalid-bin-op.rs b/src/test/ui/suggestions/invalid-bin-op.rs
new file mode 100644
index 00000000000..bea1b915586
--- /dev/null
+++ b/src/test/ui/suggestions/invalid-bin-op.rs
@@ -0,0 +1,7 @@
+pub fn foo<T>(s: S<T>, t: S<T>) {
+    let _ = s == t; //~ ERROR binary operation `==` cannot be applied to type `S<T>`
+}
+
+struct S<T>(T);
+
+fn main() {}
diff --git a/src/test/ui/suggestions/invalid-bin-op.stderr b/src/test/ui/suggestions/invalid-bin-op.stderr
new file mode 100644
index 00000000000..7668eddf607
--- /dev/null
+++ b/src/test/ui/suggestions/invalid-bin-op.stderr
@@ -0,0 +1,13 @@
+error[E0369]: binary operation `==` cannot be applied to type `S<T>`
+  --> $DIR/invalid-bin-op.rs:2:15
+   |
+LL |     let _ = s == t;
+   |             - ^^ - S<T>
+   |             |
+   |             S<T>
+   |
+   = note: the trait `std::cmp::PartialEq` is not implemented for `S<T>`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0369`.
diff --git a/src/test/ui/suggestions/missing-trait-bound-for-op.fixed b/src/test/ui/suggestions/missing-trait-bound-for-op.fixed
index 02886bb845c..6b24375e415 100644
--- a/src/test/ui/suggestions/missing-trait-bound-for-op.fixed
+++ b/src/test/ui/suggestions/missing-trait-bound-for-op.fixed
@@ -1,13 +1,7 @@
 // run-rustfix
 
-pub fn strip_prefix<'a, T: std::cmp::PartialEq>(s: &'a [T], prefix: &[T]) -> Option<&'a [T]> {
-    let n = prefix.len();
-    if n <= s.len() {
-        let (head, tail) = s.split_at(n);
-        if head == prefix { //~ ERROR binary operation `==` cannot be applied to type `&[T]`
-            return Some(tail);
-        }
-    }
-    None
+pub fn foo<T: std::cmp::PartialEq>(s: &[T], t: &[T]) {
+    let _ = s == t; //~ ERROR binary operation `==` cannot be applied to type `&[T]`
 }
+
 fn main() {}
diff --git a/src/test/ui/suggestions/missing-trait-bound-for-op.rs b/src/test/ui/suggestions/missing-trait-bound-for-op.rs
index aa4ef467360..df47be070c9 100644
--- a/src/test/ui/suggestions/missing-trait-bound-for-op.rs
+++ b/src/test/ui/suggestions/missing-trait-bound-for-op.rs
@@ -1,13 +1,7 @@
 // run-rustfix
 
-pub fn strip_prefix<'a, T>(s: &'a [T], prefix: &[T]) -> Option<&'a [T]> {
-    let n = prefix.len();
-    if n <= s.len() {
-        let (head, tail) = s.split_at(n);
-        if head == prefix { //~ ERROR binary operation `==` cannot be applied to type `&[T]`
-            return Some(tail);
-        }
-    }
-    None
+pub fn foo<T>(s: &[T], t: &[T]) {
+    let _ = s == t; //~ ERROR binary operation `==` cannot be applied to type `&[T]`
 }
+
 fn main() {}
diff --git a/src/test/ui/suggestions/missing-trait-bound-for-op.stderr b/src/test/ui/suggestions/missing-trait-bound-for-op.stderr
index dab4e575be1..0e0d397d6fc 100644
--- a/src/test/ui/suggestions/missing-trait-bound-for-op.stderr
+++ b/src/test/ui/suggestions/missing-trait-bound-for-op.stderr
@@ -1,16 +1,15 @@
 error[E0369]: binary operation `==` cannot be applied to type `&[T]`
-  --> $DIR/missing-trait-bound-for-op.rs:7:17
+  --> $DIR/missing-trait-bound-for-op.rs:4:15
    |
-LL |         if head == prefix {
-   |            ---- ^^ ------ &[T]
-   |            |
-   |            &[T]
+LL |     let _ = s == t;
+   |             - ^^ - &[T]
+   |             |
+   |             &[T]
    |
-   = note: the trait `std::cmp::PartialEq` is not implemented for `&[T]`
 help: consider restricting type parameter `T`
    |
-LL | pub fn strip_prefix<'a, T: std::cmp::PartialEq>(s: &'a [T], prefix: &[T]) -> Option<&'a [T]> {
-   |                          ^^^^^^^^^^^^^^^^^^^^^
+LL | pub fn foo<T: std::cmp::PartialEq>(s: &[T], t: &[T]) {
+   |             ^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr b/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr
index 1c424ce7da6..507d53dc07c 100644
--- a/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr
+++ b/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr
@@ -6,7 +6,6 @@ LL |     a * b
    |     |
    |     &T
    |
-   = note: the trait `std::ops::Mul` is not implemented for `&T`
 help: consider further restricting this bound
    |
 LL | fn foo<T: MyMul<f64, f64> + std::ops::Mul<Output = f64>>(a: &T, b: f64) -> f64 {