Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @lcnr (or someone else) soon.

Please see the contribution instructions for more information.

While I agree with the fact that listing basically every <i8 as Add<i8>>, <i16 for Add<i16>>, .. is noisy, I'm not sure if I like this approach. We should probably just suggest fewer candidates if there are a ton of them, especially if they're foreign impls.

While I agree with the fact that listing basically every <i8 as Add<i8>>, <i16 for Add<i16>>, .. is noisy, I'm not sure if I like this approach. We should probably just suggest fewer candidates if there are a ton of them, especially if they're foreign impls.

I agree with this, maybe we can collapse shown candidates from 8 to 6/5?

While I agree with the fact that listing basically every <i8 as Add<i8>>, <i16 for Add<i16>>, .. is noisy, I'm not sure if I like this approach. We should probably just suggest fewer candidates if there are a ton of them, especially if they're foreign impls.

My problem with them is that most of these recommendations are useless; not just that some are really noisy.

I'd like it if suggestions such as these were not shown to users:

= help: the trait `Into<U>` is implemented for `T`

   = help: the following other types implement trait `FromResidual<R>`:
             <Result<T, F> as FromResidual<Result<Infallible, E>>>
             <Result<T, F> as FromResidual<Yeet<E>>>

and these errors get exponentially worse as the traits get more complicated:

    = help: the following other types implement trait `IntoPyCallbackOutput<Target>`:
              <() as IntoPyCallbackOutput<()>>
              <() as IntoPyCallbackOutput<i32>>
              <*mut PyObject as IntoPyCallbackOutput<*mut PyObject>>
              <HashCallbackOutput as IntoPyCallbackOutput<isize>>
              <IterANextOutput<Py<PyAny>, Py<PyAny>> as IntoPyCallbackOutput<*mut PyObject>>
              <IterANextOutput<T, U> as IntoPyCallbackOutput<IterANextOutput<Py<PyAny>, Py<PyAny>>>>
              <IterNextOutput<Py<PyAny>, Py<PyAny>> as IntoPyCallbackOutput<*mut PyObject>>
              <IterNextOutput<T, U> as IntoPyCallbackOutput<IterNextOutput<Py<PyAny>, Py<PyAny>>>>
            and 7 others

On the other hand, consider a variant of this diagnostic that is actually helpful (this PR keeps it, don't worry):

   = help: the following other types implement trait `Pattern<'a>`:
             &'b String
             &'b [char; N]
             &'b [char]
             &'b str
             &'c &'b str
             [char; N]
             char

Even without reading the users code, you know what they did, why it doesn't work and how they can probably fix it. It is simple and helpful. If we aren't reasonably sure the suggestion is good, we should err on the side of not showing it - that is what this PR does.

I'll admit to not really knowing what I'm doing - I barely know enough to be dangerous 😆 . If there's a better way to do what I'm trying to do I love to hear it 🙂 .

I agree with this, maybe we can collapse shown candidates from 8 to 6/5?

For many of these errors, that boils down to "this help is bad, so lets show less of it". I would much rather just not show it at all. It's hard for users to parse long error messages, especially if they're only sometimes helpful.

I don't think that the thing that distinguishes "the following other types implement trait Pattern<'a>" being useful and "the following other types implement trait IntoPyCallbackOutput<Target>" being extraneous in their respective examples is whether the trait takes type parameters as your has_no_types condition is implementing.

For example, I actually want a failed Ty: Trait<i32> to tell me that, for example, Ty: Trait<(i32,)> is satisfied instead (along with other candidates), which as far as I can tell would be suppressed by this.

The issue here is twofold:

(1.) Blanket impls are sometimes relevant, but also sometimes extraneous to mention. We don't take in account where clauses, for example, but also some blanket impls that are not completely unsubstituted are still helpful in guiding the user...

For example, I understand that T: Into<U> by itself is not particularly useful to show, but where do we draw the line? We could check if blanket impls apply to a failed trait predicate -- e.g. i32: Into<()> fits into the blanket impl T: Into<U> inferring T = i32, U = (), but doesn't satisfy the where clause of the impl. We could alternatively check if the blanket impl has any non-generic concrete types

Ideally, instead of suppressing this blanket impl, we could explain what where clauses don't hold here. On the other hand, some blanket impls are too specific, and explaining that "this actually does apply, but $WHERE_CLAUSE doesn't hold" itself doesn't offer useful advice.

Also, perhaps, we could first filter out trait implementations that either match the Self type exactly, or (alternatively) match the trait's generics exactly. I say that given Ty: Trait<A> failing, mentioning Ty: Trait<B> and OtherTy: Trait<A> are both more useful than mentioning OtherTy: Trait<B>. Though that's subject to other issues that I discuss below about "closeness" in type space...

(2.) Some traits just have lots of impls, and/or it's really hard to tell whether an impl is notable when compared against a trait predicate... For example, std::ops::Add just has a lot of implementations, and some of those implementations are more relevant to the caller. Unfortunately, we don't currently have heuristics to:

(a.) filter out implementations that are "too far away" in type space from the implementor's input types, and...

(b.) order impl suggestions by how "close" they are in type space to the implementor's input types. Also...

(c.) sometimes there are just a combinatorial blow-up of impls due to limitations of the language, like having to manually implement a trait for fn(A) -> O, fn(A, B) -> O... While it's obviously desirable to remove all of those extraneous suggestions, how do we make sure we still convey the fact that the pattern of fn(..) -> O implements a trait, for example?

So yeah, I understand this is a lot of extraneous information that you're stripping out of the ui test outputs right now, but I also feel like it's too heavy of a hammer, or at least currently a somewhat arbitrary heurustic you're using to decide when to skip these suggestions. If you want to do some more brainstorming, feel free to reach out.

☔ The latest upstream changes (presumably #102867) made this pull request unmergeable. Please resolve the merge conflicts.

Closing this, I think we need to discuss this some more, for which i made #103822