Suggest bounds in more cases, accounting for type parameters referenced in predicate

Author: estebank

compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs

@@ -31,8 +31,8 @@ use rustc_middle::ty::print::{
 };
 use rustc_middle::ty::{
     self, AdtKind, GenericArgs, InferTy, IsSuggestable, Ty, TyCtxt, TypeFoldable, TypeFolder,
-    TypeSuperFoldable, TypeVisitableExt, TypeckResults, Upcast, suggest_arbitrary_trait_bound,
-    suggest_constraining_type_param,
+    TypeSuperFoldable, TypeSuperVisitable, TypeVisitableExt, TypeVisitor, TypeckResults, Upcast,
+    suggest_arbitrary_trait_bound, suggest_constraining_type_param,
 };
 use rustc_middle::{bug, span_bug};
 use rustc_span::def_id::LocalDefId;
@@ -262,6 +262,9 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
             _ => (false, None),
         };
 
+        let mut finder = ParamFinder { .. };
+        finder.visit_binder(&trait_pred);
+
         // FIXME: Add check for trait bound that is already present, particularly `?Sized` so we
         //        don't suggest `T: Sized + ?Sized`.
         loop {
@@ -410,6 +413,26 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
                     }
                 }
 
+                hir::Node::TraitItem(hir::TraitItem {
+                    generics,
+                    kind: hir::TraitItemKind::Fn(..),
+                    ..
+                })
+                | hir::Node::ImplItem(hir::ImplItem {
+                    generics,
+                    trait_item_def_id: None,
+                    kind: hir::ImplItemKind::Fn(..),
+                    ..
+                }) if finder.can_suggest_bound(generics) => {
+                    // Missing generic type parameter bound.
+                    suggest_arbitrary_trait_bound(
+                        self.tcx,
+                        generics,
+                        err,
+                        trait_pred,
+                        associated_ty,
+                    );
+                }
                 hir::Node::Item(hir::Item {
                     kind:
                         hir::ItemKind::Struct(_, generics, _)
@@ -422,7 +445,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
                         | hir::ItemKind::Const(_, generics, _, _)
                         | hir::ItemKind::TraitAlias(_, generics, _),
                     ..
-                }) if !param_ty => {
+                }) if finder.can_suggest_bound(generics) => {
                     // Missing generic type parameter bound.
                     if suggest_arbitrary_trait_bound(
                         self.tcx,
@@ -5034,8 +5057,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
         // Suggesting `T: ?Sized` is only valid in an ADT if `T` is only used in a
         // borrow. `struct S<'a, T: ?Sized>(&'a T);` is valid, `struct S<T: ?Sized>(T);`
         // is not. Look for invalid "bare" parameter uses, and suggest using indirection.
-        let mut visitor =
-            FindTypeParam { param: param.name.ident().name, invalid_spans: vec![], nested: false };
+        let mut visitor = FindTypeParam { param: param.name.ident().name, .. };
         visitor.visit_item(item);
         if visitor.invalid_spans.is_empty() {
             return false;
@@ -5198,7 +5220,7 @@ fn hint_missing_borrow<'tcx>(
 /// Used to suggest replacing associated types with an explicit type in `where` clauses.
 #[derive(Debug)]
 pub struct SelfVisitor<'v> {
-    pub paths: Vec<&'v hir::Ty<'v>>,
+    pub paths: Vec<&'v hir::Ty<'v>> = Vec::new(),
     pub name: Option<Symbol>,
 }
 
@@ -5568,7 +5590,7 @@ fn point_at_assoc_type_restriction<G: EmissionGuarantee>(
                 );
                 // Search for the associated type `Self::{name}`, get
                 // its type and suggest replacing the bound with it.
-                let mut visitor = SelfVisitor { paths: vec![], name: Some(name) };
+                let mut visitor = SelfVisitor { name: Some(name), .. };
                 visitor.visit_trait_ref(trait_ref);
                 for path in visitor.paths {
                     err.span_suggestion_verbose(
@@ -5579,7 +5601,7 @@ fn point_at_assoc_type_restriction<G: EmissionGuarantee>(
                     );
                 }
             } else {
-                let mut visitor = SelfVisitor { paths: vec![], name: None };
+                let mut visitor = SelfVisitor { name: None, .. };
                 visitor.visit_trait_ref(trait_ref);
                 let span: MultiSpan =
                     visitor.paths.iter().map(|p| p.span).collect::<Vec<Span>>().into();
@@ -5609,8 +5631,8 @@ fn get_deref_type_and_refs(mut ty: Ty<'_>) -> (Ty<'_>, Vec<hir::Mutability>) {
 /// `param: ?Sized` would be a valid constraint.
 struct FindTypeParam {
     param: rustc_span::Symbol,
-    invalid_spans: Vec<Span>,
-    nested: bool,
+    invalid_spans: Vec<Span> = Vec::new(),
+    nested: bool = false,
 }
 
 impl<'v> Visitor<'v> for FindTypeParam {
@@ -5648,3 +5670,38 @@ impl<'v> Visitor<'v> for FindTypeParam {
         }
     }
 }
+
+/// Look for type parameters in predicates. We use this to identify whether a bound is suitable in
+/// on a given item.
+struct ParamFinder {
+    params: Vec<Symbol> = Vec::new(),
+}
+
+impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for ParamFinder {
+    fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result {
+        match t.kind() {
+            ty::Param(p) => self.params.push(p.name),
+            _ => {}
+        }
+        t.super_visit_with(self)
+    }
+}
+
+impl ParamFinder {
+    /// Whether the `hir::Generics` of the current item can suggest the evaluated bound because its
+    /// references to type parameters are present in the generics.
+    fn can_suggest_bound(&self, generics: &hir::Generics<'_>) -> bool {
+        if self.params.is_empty() {
+            // There are no references to type parameters at all, so suggesting the bound
+            // would be reasonable.
+            return true;
+        }
+        generics.params.iter().any(|p| match p.name {
+            hir::ParamName::Plain(p_name) => {
+                // All of the parameters in the bound can be referenced in the current item.
+                self.params.iter().any(|p| *p == p_name.name || *p == kw::SelfUpper)
+            }
+            _ => true,
+        })
+    }
+}

compiler/rustc_trait_selection/src/lib.rs

@@ -19,6 +19,7 @@
 #![feature(assert_matches)]
 #![feature(associated_type_defaults)]
 #![feature(box_patterns)]
+#![feature(default_field_values)]
 #![feature(if_let_guard)]
 #![feature(iter_intersperse)]
 #![feature(iterator_try_reduce)]

tests/ui/const-generics/failing_goal_with_repeat_expr_anon_const.stderr

@@ -21,10 +21,6 @@ LL |         [0u8; std::mem::size_of::<Self::A>()] == Self::P;
    |                                               ^^ no implementation for `[u8; std::mem::size_of::<Self::A>()] == <Self as T>::A`
    |
    = help: the trait `PartialEq<<Self as T>::A>` is not implemented for `[u8; std::mem::size_of::<Self::A>()]`
-help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement
-   |
-LL | pub trait T where [u8; std::mem::size_of::<Self::A>()]: PartialEq<<Self as T>::A> {
-   |             +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 
 error: aborting due to 3 previous errors
 

tests/ui/traits/bound/suggest-complex-bound-on-method.fixed

@@ -0,0 +1,23 @@
+//@ run-rustfix
+#![allow(dead_code)]
+struct Application;
+
+trait Trait {
+    type Error: std::error::Error;
+
+    fn run(&self) -> Result<(), Self::Error>;
+}
+
+#[derive(Debug)]
+enum ApplicationError {
+    Quit,
+}
+
+impl Application {
+    fn thing<T: Trait>(&self, t: T) -> Result<(), ApplicationError> where ApplicationError: From<<T as Trait>::Error> {
+        t.run()?; //~ ERROR E0277
+        Ok(())
+    }
+}
+
+fn main() {}

tests/ui/traits/bound/suggest-complex-bound-on-method.rs

@@ -0,0 +1,23 @@
+//@ run-rustfix
+#![allow(dead_code)]
+struct Application;
+
+trait Trait {
+    type Error: std::error::Error;
+
+    fn run(&self) -> Result<(), Self::Error>;
+}
+
+#[derive(Debug)]
+enum ApplicationError {
+    Quit,
+}
+
+impl Application {
+    fn thing<T: Trait>(&self, t: T) -> Result<(), ApplicationError> {
+        t.run()?; //~ ERROR E0277
+        Ok(())
+    }
+}
+
+fn main() {}

tests/ui/traits/bound/suggest-complex-bound-on-method.stderr

@@ -0,0 +1,22 @@
+error[E0277]: `?` couldn't convert the error to `ApplicationError`
+  --> $DIR/suggest-complex-bound-on-method.rs:18:16
+   |
+LL |         t.run()?;
+   |           -----^ the trait `From<<T as Trait>::Error>` is not implemented for `ApplicationError`
+   |           |
+   |           this can't be annotated with `?` because it has type `Result<_, <T as Trait>::Error>`
+   |
+note: `ApplicationError` needs to implement `From<<T as Trait>::Error>`
+  --> $DIR/suggest-complex-bound-on-method.rs:12:1
+   |
+LL | enum ApplicationError {
+   | ^^^^^^^^^^^^^^^^^^^^^
+   = note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
+help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement
+   |
+LL |     fn thing<T: Trait>(&self, t: T) -> Result<(), ApplicationError> where ApplicationError: From<<T as Trait>::Error> {
+   |                                                                     +++++++++++++++++++++++++++++++++++++++++++++++++
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0277`.