[implied_bounds_in_impls]: don't ICE on default generic parameter and move to nursery

[y21]

Fixes #11422

This fixes two ICEs (1, 2), and moves it to nursery for now, because this lint needs some improvements in its suggestion (see #11435, for one such example).

changelog: Moved [implied_bounds_in_impls] to nursery (Now allow-by-default)
#11437
changelog: [implied_bounds_in_impls]: don't ICE on default generic parameter in supertrait clause

r? @xFrednet (since you reviewed my PR that added this lint, I figured it might make sense to have you review this as well since you have seen this code before. If you don't want to review this, sorry! Feel free to reroll then)

---

As for the ICE, it's pretty complicated and very confusing imo, so I'm going to try to explain the idea here (partly for myself, too, because I've confused myself several times writing- and fixing this):

Expand

The general idea behind the lint is that, if we have this function:

fn f() -> impl PartialEq<i32> + PartialOrd<i32> { 0 }

We want to lint the PartialEq bound because it's unnecessary. That exact bound is already specified in PartialOrd<i32>'s supertrait clause:

trait PartialOrd<Rhs>: PartialEq<Rhs> {}
//    PartialOrd<i32>: PartialEq<i32>

The way it does this is in two steps:

• Go through all of the bounds in the impl Trait return type and collect each of the trait's supertrait bounds into a vec. We also store the generic arguments for later.
  • PartialEq has no supertraits, nothing to add.
  • PartialOrd is defined as trait PartialOrd: PartialEq, so add PartialEq to the list, as well as the generic argument(s) <i32>

Once we are done, we have these entries in the vec: [(PartialEq, [i32])]

• Go through all the bounds again, and looking for those bounds that have their trait DefId in the implied bounds vec.
  • PartialEq is in that vec. However, that is not enough, because the trait is generic. If the user wrote impl PartialEq<String> + PartialOrd<i32>, then PartialOrd clearly doesn't imply PartialEq. Which means, we also need to check that the generic parameters match. This is why we also collected the generic arguments in PartialOrd<i32>. This process of checking generic arguments is pretty complicated and is also where the two ICEs happened.

The way it checks that the generic arguments match is by comparing the generic parameters in the super trait clause:

trait PartialOrd<Rhs>: PartialEq<Rhs> {}
//                     ^^^^^^^^^^^^^^

...this needs to match...

fn f() -> impl PartialEq<i32> + ...
//             ^^^^^^^^^^^^^^

In the compiler, the Rhs generic parameter is its own type and we cannot just compare it to i32. We need to "substitute" it.
Internally, Rhs is represented as Rhs#1 (the number next to # represents the type parameter index. They start at 0, but 0 is "reserved" for the implicit Self generic parameter).

How do we go from Rhs#1 to i32? Well, we know that all the generic parameters had to be substituted in the impl ... + PartialOrd<i32> type. So we subtract 1 from the type parameter index, giving us 0 (Self is not specified in that list of arguments). We use that as the index into the generic argument list <i32>. That's i32. Now we know that the supertrait clause looks like : PartialEq<i32>.

Then, we can compare that to what the user actually wrote on the bound that we think is being implied: impl PartialEq<i32> + ....

Now to the actual bug: this whole logic doesn't take into account default generic parameters. Actually, PartialOrd is defined like this:

trait PartialOrd<Rhs = Self>: PartialEq<Rhs> {}

If we now have a function like this:

fn f() -> impl PartialOrd + PartialEq {}

that logic breaks apart... We look at the supertrait predicate : PartialEq<Rhs> (Rhs is Rhs#1), then take the first argument in the generic argument list PartialEq<..> to resolve the Rhs, but at this point we crash because there is no generic argument.
The index 0 is out of bounds. If this happens (and we even get to linting here, which could only happen if it passes typeck), it must mean that that generic parameter has a default type that is not required to be specified.

This PR changes the logic such that if we have a type parameter index that is out of bounds, it looks at the definition of the trait and check that there exists a default type that we can use instead.
So, we see <Rhs = Self>, and use Self for substitution, and end up with this predicate: : PartialEq<Self>. No crash this time.

[xFrednet]

LGTM, thank you for fixing the ICEs.

@bors r+

[bors]

📌 Commit 563abf9 has been approved by xFrednet

It is now in the queue for this repository.

[bors]

⌛ Testing commit 563abf9 with merge 3de0f19...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFrednet
Pushing 3de0f19 to master...