Replace suggest_constraining_param with suggest_restricting_param_bound

to fix incorrect suggestion for trait bounds involving binary operators.
Fixes #93927, #92347, #93744.

Author: willcrichton

compiler/rustc_typeck/src/check/op.rs

@@ -11,13 +11,12 @@ use rustc_middle::ty::adjustment::{
 };
 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,
-};
+use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable, TypeVisitor};
 use rustc_span::source_map::Spanned;
 use rustc_span::symbol::{sym, Ident};
 use rustc_span::Span;
 use rustc_trait_selection::infer::InferCtxtExt;
+use rustc_trait_selection::traits::error_reporting::suggestions::InferCtxtExt as _;
 use rustc_trait_selection::traits::{FulfillmentError, TraitEngine, TraitEngineExt};
 
 use std::ops::ControlFlow;
@@ -266,7 +265,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             Err(_) if lhs_ty.references_error() || rhs_ty.references_error() => self.tcx.ty_error(),
             Err(errors) => {
                 let source_map = self.tcx.sess.source_map();
-                let (mut err, missing_trait, use_output) = match is_assign {
+                let (mut err, missing_trait, _use_output) = match is_assign {
                     IsAssign::Yes => {
                         let mut err = struct_span_err!(
                             self.tcx.sess,
@@ -449,40 +448,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         // concatenation (e.g., "Hello " + "World!"). This means
                         // 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() {
-                            // 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),
-                                    Some(eraser.fold_ty(rhs_ty)),
-                                    Some(rhs_expr),
-                                    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 `{missing_trait}` is not implemented for `{lhs_ty}`"
-                                ));
-                            }
-                        } else {
-                            bug!("type param visitor stored a non type param: {:?}", ty.kind());
+                        // Look for a TraitPredicate in the Fulfillment errors,
+                        // and use it to generate a suggestion.
+                        //
+                        // Note that lookup_op_method must be called again but
+                        // with a specific rhs_ty instead of a placeholder so
+                        // the resulting predicate generates a more specific
+                        // suggestion for the user.
+                        let errors = self
+                        .lookup_op_method(lhs_ty, &[rhs_ty], Op::Binary(op, is_assign))
+                        .unwrap_err();
+                    let predicates = errors
+                        .into_iter()
+                        .filter_map(|error| error.obligation.predicate.to_opt_poly_trait_pred())
+                        .collect::<Vec<_>>();
+                    if !predicates.is_empty() {
+                        for pred in predicates {
+                            self.infcx.suggest_restricting_param_bound(&mut err,
+                                pred,
+                                self.body_id,
+                            );
                         }
+                     } 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 `{missing_trait}` is not implemented for `{lhs_ty}`"
+                        ));                    
                     }
                 }
                 err.emit();
@@ -973,46 +965,6 @@ fn is_builtin_binop<'tcx>(lhs: Ty<'tcx>, rhs: Ty<'tcx>, op: hir::BinOp) -> bool
     }
 }
 
-fn suggest_constraining_param(
-    tcx: TyCtxt<'_>,
-    body_id: hir::HirId,
-    mut err: &mut Diagnostic,
-    lhs_ty: Ty<'_>,
-    rhs_ty: Ty<'_>,
-    missing_trait: &str,
-    p: ty::ParamTy,
-    set_output: bool,
-) {
-    let hir = tcx.hir();
-    let msg = &format!("`{lhs_ty}` might need a bound for `{missing_trait}`");
-    // Try to find the def-id and details for the parameter p. We have only the index,
-    // so we have to find the enclosing function's def-id, then look through its declared
-    // generic parameters to get the declaration.
-    let def_id = hir.body_owner_def_id(hir::BodyId { hir_id: body_id });
-    let generics = tcx.generics_of(def_id);
-    let param_def_id = generics.type_param(&p, tcx).def_id;
-    if let Some(generics) = param_def_id
-        .as_local()
-        .map(|id| hir.local_def_id_to_hir_id(id))
-        .and_then(|id| hir.find_by_def_id(hir.get_parent_item(id)))
-        .as_ref()
-        .and_then(|node| node.generics())
-    {
-        let output = if set_output { format!("<Output = {rhs_ty}>") } else { String::new() };
-        suggest_constraining_type_param(
-            tcx,
-            generics,
-            &mut err,
-            &lhs_ty.to_string(),
-            &format!("{missing_trait}{output}"),
-            None,
-        );
-    } else {
-        let span = tcx.def_span(param_def_id);
-        err.span_label(span, msg);
-    }
-}
-
 struct TypeParamVisitor<'tcx>(Vec<Ty<'tcx>>);
 
 impl<'tcx> TypeVisitor<'tcx> for TypeParamVisitor<'tcx> {

src/test/ui/binop/issue-93927.rs

@@ -0,0 +1,20 @@
+// Regression test for #93927: suggested trait bound for T should be Eq, not PartialEq
+struct MyType<T>(T);
+
+impl<T> PartialEq for MyType<T>
+where
+    T: Eq,
+{
+    fn eq(&self, other: &Self) -> bool {
+        true
+    }
+}
+
+fn cond<T: PartialEq>(val: MyType<T>) -> bool {
+    val == val
+    //~^ ERROR binary operation `==` cannot be applied to type `MyType<T>`
+}
+
+fn main() {
+    cond(MyType(0));
+}

src/test/ui/binop/issue-93927.stderr

@@ -0,0 +1,16 @@
+error[E0369]: binary operation `==` cannot be applied to type `MyType<T>`
+  --> $DIR/issue-93927.rs:14:9
+   |
+LL |     val == val
+   |     --- ^^ --- MyType<T>
+   |     |
+   |     MyType<T>
+   |
+help: consider further restricting this bound
+   |
+LL | fn cond<T: PartialEq + std::cmp::Eq>(val: MyType<T>) -> bool {
+   |                      ++++++++++++++
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0369`.

src/test/ui/generic-associated-types/missing-bounds.fixed

@@ -24,7 +24,7 @@ impl<B: Add + Add<Output = B>> Add for C<B> {
 
 struct D<B>(B);
 
-impl<B: std::ops::Add<Output = B>> Add for D<B> {
+impl<B: std::ops::Add> Add for D<B> {
     type Output = Self;
 
     fn add(self, rhs: Self) -> Self {

src/test/ui/generic-associated-types/missing-bounds.stderr

@@ -66,8 +66,8 @@ LL |         Self(self.0 + rhs.0)
    |
 help: consider restricting type parameter `B`
    |
-LL | impl<B: std::ops::Add<Output = B>> Add for D<B> {
-   |       +++++++++++++++++++++++++++
+LL | impl<B: std::ops::Add> Add for D<B> {
+   |       +++++++++++++++
 
 error[E0308]: mismatched types
   --> $DIR/missing-bounds.rs:42:14

src/test/ui/issues/issue-35668.stderr

@@ -6,10 +6,10 @@ LL |     a.iter().map(|a| a*a)
    |                      |
    |                      &T
    |
-help: consider restricting type parameter `T`
+help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement
    |
-LL | fn func<'a, T: std::ops::Mul<Output = &T>>(a: &'a [T]) -> impl Iterator<Item=&'a T> {
-   |              ++++++++++++++++++++++++++++
+LL | fn func<'a, T>(a: &'a [T]) -> impl Iterator<Item=&'a T> where &T: Mul<&T> {
+   |                                                         +++++++++++++++++
 
 error: aborting due to previous error
 

src/test/ui/suggestions/invalid-bin-op.stderr

@@ -11,11 +11,14 @@ note: an implementation of `PartialEq<_>` might be missing for `S<T>`
    |
 LL | struct S<T>(T);
    | ^^^^^^^^^^^^^^^ must implement `PartialEq<_>`
-   = note: the trait `std::cmp::PartialEq` is not implemented for `S<T>`
 help: consider annotating `S<T>` with `#[derive(PartialEq)]`
    |
 LL | #[derive(PartialEq)]
    |
+help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement
+   |
+LL | pub fn foo<T>(s: S<T>, t: S<T>) where S<T>: PartialEq {
+   |                                 +++++++++++++++++++++
 
 error: aborting due to previous error
 

src/test/ui/traits/resolution-in-overloaded-op.stderr

@@ -6,10 +6,10 @@ LL |     a * b
    |     |
    |     &T
    |
-help: consider further restricting this bound
+help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement
    |
-LL | fn foo<T: MyMul<f64, f64> + std::ops::Mul<Output = f64>>(a: &T, b: f64) -> f64 {
-   |                           +++++++++++++++++++++++++++++
+LL | fn foo<T: MyMul<f64, f64>>(a: &T, b: f64) -> f64 where &T: Mul<f64> {
+   |                                                  ++++++++++++++++++
 
 error: aborting due to previous error
 

src/test/ui/type/type-check/missing_trait_impl.stderr

@@ -8,8 +8,8 @@ LL |     let z = x + y;
    |
 help: consider restricting type parameter `T`
    |
-LL | fn foo<T: std::ops::Add<Output = T>>(x: T, y: T) {
-   |         +++++++++++++++++++++++++++
+LL | fn foo<T: std::ops::Add>(x: T, y: T) {
+   |         +++++++++++++++
 
 error[E0368]: binary assignment operation `+=` cannot be applied to type `T`
   --> $DIR/missing_trait_impl.rs:9:5