Auto merge of #11437 - y21:issue-11422, r=xFrednet

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

Fixes #11422

This fixes two ICEs ([1](https://github.com/rust-lang/rust-clippy/issues/11422#issue-1872351763), [2](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=2901e6febb479d3bd2a74f8a5b8a9305)), 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](https://github.com/rust-lang/rust-clippy/pull/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):
<details>
<summary>Expand</summary>

The general idea behind the lint is that, if we have this function:
```rs
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:
```rs
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:
```rs
trait PartialOrd<Rhs>: PartialEq<Rhs> {}
//                     ^^^^^^^^^^^^^^
```
...this needs to match...
```rs
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:
```rs
trait PartialOrd<Rhs = Self>: PartialEq<Rhs> {}
```
If we now have a function like this:
```rs
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.

</details>

clippy_lints/src/implied_bounds_in_impls.rs

@@ -1,7 +1,7 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::snippet;
 use rustc_errors::{Applicability, SuggestionStyle};
-use rustc_hir::def_id::LocalDefId;
+use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_hir::intravisit::FnKind;
 use rustc_hir::{
     Body, FnDecl, FnRetTy, GenericArg, GenericBound, ImplItem, ImplItemKind, ItemKind, TraitBoundModifier, TraitItem,
@@ -9,7 +9,7 @@
 };
 use rustc_hir_analysis::hir_ty_to_ty;
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty::{self, ClauseKind, TyCtxt};
+use rustc_middle::ty::{self, ClauseKind, Generics, Ty, TyCtxt};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::Span;
 
@@ -45,52 +45,80 @@
     /// ```
     #[clippy::version = "1.73.0"]
     pub IMPLIED_BOUNDS_IN_IMPLS,
-    complexity,
+    nursery,
     "specifying bounds that are implied by other bounds in `impl Trait` type"
 }
 declare_lint_pass!(ImpliedBoundsInImpls => [IMPLIED_BOUNDS_IN_IMPLS]);
 
-/// This function tries to, for all type parameters in a supertype predicate `GenericTrait<U>`,
+/// Tries to "resolve" a type.
-/// check if the substituted type in the implied-by bound matches with what's subtituted in the
+/// The index passed to this function must start with `Self=0`, i.e. it must be a valid
-/// implied type.
+/// type parameter index.
+/// If the index is out of bounds, it means that the generic parameter has a default type.
+fn try_resolve_type<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    args: &'tcx [GenericArg<'tcx>],
+    generics: &'tcx Generics,
+    index: usize,
+) -> Option<Ty<'tcx>> {
+    match args.get(index - 1) {
+        Some(GenericArg::Type(ty)) => Some(hir_ty_to_ty(tcx, ty)),
+        Some(_) => None,
+        None => Some(tcx.type_of(generics.params[index].def_id).skip_binder()),
+    }
+}
+
+/// This function tries to, for all generic type parameters in a supertrait predicate `trait ...<U>:
+/// GenericTrait<U>`, check if the substituted type in the implied-by bound matches with what's
+/// subtituted in the implied bound.
 ///
 /// Consider this example.
 /// ```rust,ignore
 /// trait GenericTrait<T> {}
 /// trait GenericSubTrait<T, U, V>: GenericTrait<U> {}
-///                                              ^ trait_predicate_args: [Self#0, U#2]
+///                                 ^^^^^^^^^^^^^^^ trait_predicate_args: [Self#0, U#2]
+///                                                 (the Self#0 is implicit: `<Self as GenericTrait<U>>`)
 /// impl GenericTrait<i32> for () {}
 /// impl GenericSubTrait<(), i32, ()> for () {}
-/// impl GenericSubTrait<(), [u8; 8], ()> for () {}
+/// impl GenericSubTrait<(), i64, ()> for () {}
 ///
-/// fn f() -> impl GenericTrait<i32> + GenericSubTrait<(), [u8; 8], ()> {
+/// fn f() -> impl GenericTrait<i32> + GenericSubTrait<(), i64, ()> {
-///                             ^^^ implied_args       ^^^^^^^^^^^^^^^ implied_by_args
+///                             ^^^ implied_args       ^^^^^^^^^^^ implied_by_args
-///                                                                    (we are interested in `[u8; 8]` specifically, as that
+///                                                                (we are interested in `i64` specifically, as that
 ///                                                                 is what `U` in `GenericTrait<U>` is substituted with)
-///     ()
 /// }
 /// ```
-/// Here i32 != [u8; 8], so this will return false.
+/// Here i32 != i64, so this will return false.
-fn is_same_generics(
+fn is_same_generics<'tcx>(
-    tcx: TyCtxt<'_>,
+    tcx: TyCtxt<'tcx>,
-    trait_predicate_args: &[ty::GenericArg<'_>],
+    trait_predicate_args: &'tcx [ty::GenericArg<'tcx>],
-    implied_by_args: &[GenericArg<'_>],
+    implied_by_args: &'tcx [GenericArg<'tcx>],
-    implied_args: &[GenericArg<'_>],
+    implied_args: &'tcx [GenericArg<'tcx>],
+    implied_by_def_id: DefId,
+    implied_def_id: DefId,
 ) -> bool {
+    // Get the generics of the two traits to be able to get default generic parameter.
+    let implied_by_generics = tcx.generics_of(implied_by_def_id);
+    let implied_generics = tcx.generics_of(implied_def_id);
+
     trait_predicate_args
         .iter()
         .enumerate()
         .skip(1) // skip `Self` implicit arg
         .all(|(arg_index, arg)| {
-            if let Some(ty) = arg.as_type()
+            if let Some(ty) = arg.as_type() {
-                && let &ty::Param(ty::ParamTy{ index, .. }) = ty.kind()
+                if let &ty::Param(ty::ParamTy { index, .. }) = ty.kind()
-                // Since `trait_predicate_args` and type params in traits start with `Self=0`
+                    // `index == 0` means that it's referring to `Self`,
-                // and generic argument lists `GenericTrait<i32>` don't have `Self`,
+                    // in which case we don't try to substitute it
-                // we need to subtract 1 from the index.
+                    && index != 0
-                && let GenericArg::Type(ty_a) = implied_by_args[index as usize - 1]
+                    && let Some(ty_a) = try_resolve_type(tcx, implied_by_args, implied_by_generics, index as usize)
-                && let GenericArg::Type(ty_b) = implied_args[arg_index - 1]
+                    && let Some(ty_b) = try_resolve_type(tcx, implied_args, implied_generics, arg_index)
                 {
-                hir_ty_to_ty(tcx, ty_a) == hir_ty_to_ty(tcx, ty_b)
+                    ty_a == ty_b
+                } else if let Some(ty_b) = try_resolve_type(tcx, implied_args, implied_generics, arg_index) {
+                    ty == ty_b
+                } else {
+                    false
+                }
             } else {
                 false
             }
@@ -121,7 +149,7 @@ fn check(cx: &LateContext<'_>, decl: &FnDecl<'_>) {
                 && let predicates = cx.tcx.super_predicates_of(trait_def_id).predicates
                 && !predicates.is_empty() // If the trait has no supertrait, there is nothing to add.
             {
-                Some((bound.span(), path.args.map_or([].as_slice(), |a| a.args), predicates))
+                Some((bound.span(), path.args.map_or([].as_slice(), |a| a.args), predicates, trait_def_id))
             } else {
                 None
             }
@@ -135,11 +163,20 @@ fn check(cx: &LateContext<'_>, decl: &FnDecl<'_>) {
                 && let [.., path] = poly_trait.trait_ref.path.segments
                 && let implied_args = path.args.map_or([].as_slice(), |a| a.args)
                 && let Some(def_id) = poly_trait.trait_ref.path.res.opt_def_id()
-                && let Some(implied_by_span) = implied_bounds.iter().find_map(|&(span, implied_by_args, preds)| {
+                && let Some(implied_by_span) = implied_bounds
+                    .iter()
+                    .find_map(|&(span, implied_by_args, preds, implied_by_def_id)| {
                         preds.iter().find_map(|(clause, _)| {
                             if let ClauseKind::Trait(tr) = clause.kind().skip_binder()
                                 && tr.def_id() == def_id
-                            && is_same_generics(cx.tcx, tr.trait_ref.args, implied_by_args, implied_args)
+                                && is_same_generics(
+                                    cx.tcx,
+                                    tr.trait_ref.args,
+                                    implied_by_args,
+                                    implied_args,
+                                    implied_by_def_id,
+                                    def_id,
+                                )
                             {
                                 Some(span)
                             } else {

tests/ui/crashes/ice-11422.fixed

@@ -0,0 +1,25 @@
+#![warn(clippy::implied_bounds_in_impls)]
+
+use std::fmt::Debug;
+use std::ops::*;
+
+fn gen() -> impl PartialOrd + Debug {}
+
+struct Bar {}
+trait Foo<T = Self> {}
+trait FooNested<T = Option<Self>> {}
+impl Foo for Bar {}
+impl FooNested for Bar {}
+
+fn foo() -> impl Foo + FooNested {
+    Bar {}
+}
+
+fn test_impl_ops() -> impl Add + Sub + Mul + Div {
+    1
+}
+fn test_impl_assign_ops() -> impl AddAssign + SubAssign + MulAssign + DivAssign {
+    1
+}
+
+fn main() {}

tests/ui/crashes/ice-11422.rs

@@ -0,0 +1,25 @@
+#![warn(clippy::implied_bounds_in_impls)]
+
+use std::fmt::Debug;
+use std::ops::*;
+
+fn gen() -> impl PartialOrd + PartialEq + Debug {}
+
+struct Bar {}
+trait Foo<T = Self> {}
+trait FooNested<T = Option<Self>> {}
+impl Foo for Bar {}
+impl FooNested for Bar {}
+
+fn foo() -> impl Foo + FooNested {
+    Bar {}
+}
+
+fn test_impl_ops() -> impl Add + Sub + Mul + Div {
+    1
+}
+fn test_impl_assign_ops() -> impl AddAssign + SubAssign + MulAssign + DivAssign {
+    1
+}
+
+fn main() {}

tests/ui/crashes/ice-11422.stderr

@@ -0,0 +1,15 @@
+error: this bound is already specified as the supertrait of `PartialOrd`
+  --> $DIR/ice-11422.rs:6:31
+   |
+LL | fn gen() -> impl PartialOrd + PartialEq + Debug {}
+   |                               ^^^^^^^^^
+   |
+   = note: `-D clippy::implied-bounds-in-impls` implied by `-D warnings`
+help: try removing this bound
+   |
+LL - fn gen() -> impl PartialOrd + PartialEq + Debug {}
+LL + fn gen() -> impl PartialOrd + Debug {}
+   |
+
+error: aborting due to previous error
+

tests/ui/implied_bounds_in_impls.fixed

@@ -65,4 +65,22 @@ impl SomeTrait for SomeStruct {
     }
 }
 
+mod issue11422 {
+    use core::fmt::Debug;
+    // Some additional tests that would cause ICEs:
+
+    // `PartialOrd` has a default generic parameter and does not need to be explicitly specified.
+    // This needs special handling.
+    fn default_generic_param1() -> impl PartialOrd + Debug {}
+    fn default_generic_param2() -> impl PartialOrd + Debug {}
+
+    // Referring to `Self` in the supertrait clause needs special handling.
+    trait Trait1<X: ?Sized> {}
+    trait Trait2: Trait1<Self> {}
+    impl Trait1<()> for () {}
+    impl Trait2 for () {}
+
+    fn f() -> impl Trait1<()> + Trait2 {}
+}
+
 fn main() {}

tests/ui/implied_bounds_in_impls.rs

@@ -65,4 +65,22 @@ fn f() -> impl Deref + DerefMut<Target = u8> {
     }
 }
 
+mod issue11422 {
+    use core::fmt::Debug;
+    // Some additional tests that would cause ICEs:
+
+    // `PartialOrd` has a default generic parameter and does not need to be explicitly specified.
+    // This needs special handling.
+    fn default_generic_param1() -> impl PartialEq + PartialOrd + Debug {}
+    fn default_generic_param2() -> impl PartialOrd + PartialEq + Debug {}
+
+    // Referring to `Self` in the supertrait clause needs special handling.
+    trait Trait1<X: ?Sized> {}
+    trait Trait2: Trait1<Self> {}
+    impl Trait1<()> for () {}
+    impl Trait2 for () {}
+
+    fn f() -> impl Trait1<()> + Trait2 {}
+}
+
 fn main() {}

tests/ui/implied_bounds_in_impls.stderr

@@ -119,5 +119,29 @@ LL -     fn f() -> impl Deref + DerefMut<Target = u8> {
 LL +     fn f() -> impl DerefMut<Target = u8> {
    |
 
-error: aborting due to 10 previous errors
+error: this bound is already specified as the supertrait of `PartialOrd`
+  --> $DIR/implied_bounds_in_impls.rs:74:41
+   |
+LL |     fn default_generic_param1() -> impl PartialEq + PartialOrd + Debug {}
+   |                                         ^^^^^^^^^
+   |
+help: try removing this bound
+   |
+LL -     fn default_generic_param1() -> impl PartialEq + PartialOrd + Debug {}
+LL +     fn default_generic_param1() -> impl PartialOrd + Debug {}
+   |
+
+error: this bound is already specified as the supertrait of `PartialOrd`
+  --> $DIR/implied_bounds_in_impls.rs:75:54
+   |
+LL |     fn default_generic_param2() -> impl PartialOrd + PartialEq + Debug {}
+   |                                                      ^^^^^^^^^
+   |
+help: try removing this bound
+   |
+LL -     fn default_generic_param2() -> impl PartialOrd + PartialEq + Debug {}
+LL +     fn default_generic_param2() -> impl PartialOrd + Debug {}
+   |
+
+error: aborting due to 12 previous errors