Rollup merge of rust-lang#116388 - fmease:rustdoc-fix-n-clean-up-x-crate-higher-ranked-params, r=notriddle

rustdoc: fix & clean up handling of cross-crate higher-ranked parameters

Preparatory work for the refactoring planned in rust-lang#113015 (for correctness & maintainability).

---

1. Render the higher-ranked parameters of cross-crate function pointer types **(*)**.
2. Replace occurrences of `collect_referenced_late_bound_regions()` (CRLBR) with `bound_vars()`.
  The former is quite problematic and the use of the latter allows us to yank a lot of hacky code **(†)**
  as you can tell from the diff! :)
3. Add support for cross-crate higher-ranked types (`#![feature(non_lifetime_binders)]`).
  We were previously ICE'ing on them (see `inline_cross/non_lifetime_binders.rs`).

---

**(*)**: Extracted from test `inline_cross/fn-type.rs`:

```diff
- fn(_: &'z fn(_: &'b str), _: &'a ()) -> &'a ()
+ for<'z, 'a, '_unused> fn(_: &'z for<'b> fn(_: &'b str), _: &'a ()) -> &'a ()
```

**(†)**: It returns an `FxHashSet` which isn't *predictable* or *stable* wrt. source code (`.rmeta`) changes. To elaborate, the ordering of late-bound regions doesn't necessarily reflect the ordering found in the source code. It does seem to be stable across compilations but modifying the source code of the to-be-documented crates (like adding or renaming items) may result in a different order:

<details><summary>Example</summary>

Let's assume that we're documenting the cross-crate re-export of `produce` from the code below. On `master`, rustdoc would render the list of binders as `for<'x, 'y, 'z>`. However, once you add back the functions `a`–`l`, it would be rendered as `for<'z, 'y, 'x>` (reverse order)! Results may vary. `bound_vars()` fixes this as it returns them in source order.

```rs
// pub fn a() {}
// pub fn b() {}
// pub fn c() {}
// pub fn d() {}
// pub fn e() {}
// pub fn f() {}
// pub fn g() {}
// pub fn h() {}
// pub fn i() {}
// pub fn j() {}
// pub fn k() {}
// pub fn l() {}

pub fn produce() -> impl for<'x, 'y, 'z> Trait<'z, 'y, 'x> {}

pub trait Trait<'a, 'b, 'c> {}

impl Trait<'_, '_, '_> for () {}
```

</details>

Further, as the name suggests, CRLBR only collects *referenced* regions and thus we drop unused binders. `bound_vars()` contains unused binders on the other hand. Let's stay closer to the source where possible and keep unused binders.

Lastly, using `bound_vars()` allows us to get rid of

* the deduplication and alphabetical sorting hack in `simplify.rs`
* the weird field `bound_params` on `EqPredicate`

both of which were introduced by me in rust-lang#102707 back when I didn't know better.

To illustrate, let's look at the cross-crate bound `T: for<'a, 'b> Trait<A<'a> = (), B<'b> = ()>`.

* With CRLBR + `EqPredicate.bound_params`, *before* bounds simplification we would have the bounds `T: Trait`, `for<'a> <T as Trait>::A<'a> == ()` and `for<'b> <T as Trait>::B<'b> == ()` which required us to merge `for<>`, `for<'a>` and `for<'b>` into `for<'a, 'b>` in a deterministic manner and without introducing duplicate binders.
* With `bound_vars()`, we now have the bounds `for<'a, b> T: Trait`, `<T as Trait>::A<'a> == ()` and `<T as Trait>::B<'b> == ()` before bound simplification similar to rustc itself. This obviously no longer requires any funny merging of `for<>`s. On top of that `for<'a, 'b>` is guaranteed to be in source order.

Author: matthiaskrgr

src/librustdoc/clean/auto_trait.rs

@@ -551,8 +551,8 @@ where
                 WherePredicate::RegionPredicate { lifetime, bounds } => {
                     lifetime_to_bounds.entry(lifetime).or_default().extend(bounds);
                 }
-                WherePredicate::EqPredicate { lhs, rhs, bound_params } => {
-                    match *lhs {
+                WherePredicate::EqPredicate { lhs, rhs } => {
+                    match lhs {
                         Type::QPath(box QPathData {
                             ref assoc,
                             ref self_type,
@@ -590,14 +590,13 @@ where
                                 GenericArgs::AngleBracketed { ref mut bindings, .. } => {
                                     bindings.push(TypeBinding {
                                         assoc: assoc.clone(),
-                                        kind: TypeBindingKind::Equality { term: *rhs },
+                                        kind: TypeBindingKind::Equality { term: rhs },
                                     });
                                 }
                                 GenericArgs::Parenthesized { .. } => {
                                     existing_predicates.push(WherePredicate::EqPredicate {
                                         lhs: lhs.clone(),
                                         rhs,
-                                        bound_params,
                                     });
                                     continue; // If something other than a Fn ends up
                                     // with parentheses, leave it alone

src/librustdoc/clean/inline.rs

@@ -18,9 +18,9 @@ use rustc_span::hygiene::MacroKind;
 use rustc_span::symbol::{kw, sym, Symbol};
 
 use crate::clean::{
-    self, clean_fn_decl_from_did_and_sig, clean_generics, clean_impl_item, clean_middle_assoc_item,
-    clean_middle_field, clean_middle_ty, clean_trait_ref_with_bindings, clean_ty,
-    clean_ty_alias_inner_type, clean_ty_generics, clean_variant_def, utils, Attributes,
+    self, clean_bound_vars, clean_fn_decl_from_did_and_sig, clean_generics, clean_impl_item,
+    clean_middle_assoc_item, clean_middle_field, clean_middle_ty, clean_trait_ref_with_bindings,
+    clean_ty, clean_ty_alias_inner_type, clean_ty_generics, clean_variant_def, utils, Attributes,
     AttributesExt, ImplKind, ItemId, Type,
 };
 use crate::core::DocContext;
@@ -239,20 +239,13 @@ pub(crate) fn build_external_trait(cx: &mut DocContext<'_>, did: DefId) -> clean
 
 fn build_external_function<'tcx>(cx: &mut DocContext<'tcx>, did: DefId) -> Box<clean::Function> {
     let sig = cx.tcx.fn_sig(did).instantiate_identity();
-
-    let late_bound_regions = sig.bound_vars().into_iter().filter_map(|var| match var {
-        ty::BoundVariableKind::Region(ty::BrNamed(_, name)) if name != kw::UnderscoreLifetime => {
-            Some(clean::GenericParamDef::lifetime(name))
-        }
-        _ => None,
-    });
-
     let predicates = cx.tcx.explicit_predicates_of(did);
+
     let (generics, decl) = clean::enter_impl_trait(cx, |cx| {
         // NOTE: generics need to be cleaned before the decl!
         let mut generics = clean_ty_generics(cx, cx.tcx.generics_of(did), predicates);
         // FIXME: This does not place parameters in source order (late-bound ones come last)
-        generics.params.extend(late_bound_regions);
+        generics.params.extend(clean_bound_vars(sig.bound_vars()));
         let decl = clean_fn_decl_from_did_and_sig(cx, Some(did), sig);
         (generics, decl)
     });

src/librustdoc/clean/mod.rs

@@ -232,20 +232,11 @@ fn clean_poly_trait_ref_with_bindings<'tcx>(
     poly_trait_ref: ty::PolyTraitRef<'tcx>,
     bindings: ThinVec<TypeBinding>,
 ) -> GenericBound {
-    // collect any late bound regions
-    let late_bound_regions: Vec<_> = cx
-        .tcx
-        .collect_referenced_late_bound_regions(&poly_trait_ref)
-        .into_iter()
-        .filter_map(|br| match br {
-            ty::BrNamed(_, name) if br.is_named() => Some(GenericParamDef::lifetime(name)),
-            _ => None,
-        })
-        .collect();
-
-    let trait_ = clean_trait_ref_with_bindings(cx, poly_trait_ref, bindings);
     GenericBound::TraitBound(
-        PolyTrait { trait_, generic_params: late_bound_regions },
+        PolyTrait {
+            trait_: clean_trait_ref_with_bindings(cx, poly_trait_ref, bindings),
+            generic_params: clean_bound_vars(poly_trait_ref.bound_vars()),
+        },
         hir::TraitBoundModifier::None,
     )
 }
@@ -338,9 +329,8 @@ fn clean_where_predicate<'tcx>(
         },
 
         hir::WherePredicate::EqPredicate(ref wrp) => WherePredicate::EqPredicate {
-            lhs: Box::new(clean_ty(wrp.lhs_ty, cx)),
-            rhs: Box::new(clean_ty(wrp.rhs_ty, cx).into()),
-            bound_params: Vec::new(),
+            lhs: clean_ty(wrp.lhs_ty, cx),
+            rhs: clean_ty(wrp.rhs_ty, cx).into(),
         },
     })
 }
@@ -436,20 +426,9 @@ fn clean_projection_predicate<'tcx>(
     pred: ty::Binder<'tcx, ty::ProjectionPredicate<'tcx>>,
     cx: &mut DocContext<'tcx>,
 ) -> WherePredicate {
-    let late_bound_regions = cx
-        .tcx
-        .collect_referenced_late_bound_regions(&pred)
-        .into_iter()
-        .filter_map(|br| match br {
-            ty::BrNamed(_, name) if br.is_named() => Some(GenericParamDef::lifetime(name)),
-            _ => None,
-        })
-        .collect();
-
     WherePredicate::EqPredicate {
-        lhs: Box::new(clean_projection(pred.map_bound(|p| p.projection_ty), cx, None)),
-        rhs: Box::new(clean_middle_term(pred.map_bound(|p| p.term), cx)),
-        bound_params: late_bound_regions,
+        lhs: clean_projection(pred.map_bound(|p| p.projection_ty), cx, None),
+        rhs: clean_middle_term(pred.map_bound(|p| p.term), cx),
     }
 }
 
@@ -703,8 +682,8 @@ pub(crate) fn clean_generics<'tcx>(
                     }
                 }
             }
-            WherePredicate::EqPredicate { lhs, rhs, bound_params } => {
-                eq_predicates.push(WherePredicate::EqPredicate { lhs, rhs, bound_params });
+            WherePredicate::EqPredicate { lhs, rhs } => {
+                eq_predicates.push(WherePredicate::EqPredicate { lhs, rhs });
             }
         }
     }
@@ -798,11 +777,9 @@ fn clean_ty_generics<'tcx>(
         })
         .collect::<ThinVec<GenericParamDef>>();
 
-    // param index -> [(trait DefId, associated type name & generics, term, higher-ranked params)]
-    let mut impl_trait_proj = FxHashMap::<
-        u32,
-        Vec<(DefId, PathSegment, ty::Binder<'_, ty::Term<'_>>, Vec<GenericParamDef>)>,
-    >::default();
+    // param index -> [(trait DefId, associated type name & generics, term)]
+    let mut impl_trait_proj =
+        FxHashMap::<u32, Vec<(DefId, PathSegment, ty::Binder<'_, ty::Term<'_>>)>>::default();
 
     let where_predicates = preds
         .predicates
@@ -854,11 +831,6 @@ fn clean_ty_generics<'tcx>(
                         trait_did,
                         name,
                         proj.map_bound(|p| p.term),
-                        pred.get_bound_params()
-                            .into_iter()
-                            .flatten()
-                            .cloned()
-                            .collect(),
                     ));
                 }
 
@@ -894,9 +866,9 @@ fn clean_ty_generics<'tcx>(
 
         let crate::core::ImplTraitParam::ParamIndex(idx) = param else { unreachable!() };
         if let Some(proj) = impl_trait_proj.remove(&idx) {
-            for (trait_did, name, rhs, bound_params) in proj {
+            for (trait_did, name, rhs) in proj {
                 let rhs = clean_middle_term(rhs, cx);
-                simplify::merge_bounds(cx, &mut bounds, bound_params, trait_did, name, &rhs);
+                simplify::merge_bounds(cx, &mut bounds, trait_did, name, &rhs);
             }
         }
 
@@ -1363,23 +1335,13 @@ pub(crate) fn clean_middle_assoc_item<'tcx>(
         }
         ty::AssocKind::Fn => {
             let sig = tcx.fn_sig(assoc_item.def_id).instantiate_identity();
-
-            let late_bound_regions = sig.bound_vars().into_iter().filter_map(|var| match var {
-                ty::BoundVariableKind::Region(ty::BrNamed(_, name))
-                    if name != kw::UnderscoreLifetime =>
-                {
-                    Some(GenericParamDef::lifetime(name))
-                }
-                _ => None,
-            });
-
             let mut generics = clean_ty_generics(
                 cx,
                 tcx.generics_of(assoc_item.def_id),
                 tcx.explicit_predicates_of(assoc_item.def_id),
             );
             // FIXME: This does not place parameters in source order (late-bound ones come last)
-            generics.params.extend(late_bound_regions);
+            generics.params.extend(clean_bound_vars(sig.bound_vars()));
 
             let mut decl = clean_fn_decl_from_did_and_sig(cx, Some(assoc_item.def_id), sig);
 
@@ -2115,9 +2077,11 @@ pub(crate) fn clean_middle_ty<'tcx>(
             // FIXME: should we merge the outer and inner binders somehow?
             let sig = bound_ty.skip_binder().fn_sig(cx.tcx);
             let decl = clean_fn_decl_from_did_and_sig(cx, None, sig);
+            let generic_params = clean_bound_vars(sig.bound_vars());
+
             BareFunction(Box::new(BareFunctionDecl {
                 unsafety: sig.unsafety(),
-                generic_params: Vec::new(),
+                generic_params,
                 decl,
                 abi: sig.abi(),
             }))
@@ -2193,8 +2157,8 @@ pub(crate) fn clean_middle_ty<'tcx>(
 
             let late_bound_regions: FxIndexSet<_> = obj
                 .iter()
-                .flat_map(|pb| pb.bound_vars())
-                .filter_map(|br| match br {
+                .flat_map(|pred| pred.bound_vars())
+                .filter_map(|var| match var {
                     ty::BoundVariableKind::Region(ty::BrNamed(_, name))
                         if name != kw::UnderscoreLifetime =>
                     {
@@ -2268,6 +2232,11 @@ pub(crate) fn clean_middle_ty<'tcx>(
             }
         }
 
+        ty::Bound(_, ref ty) => match ty.kind {
+            ty::BoundTyKind::Param(_, name) => Generic(name),
+            ty::BoundTyKind::Anon => panic!("unexpected anonymous bound type variable"),
+        },
+
         ty::Alias(ty::Opaque, ty::AliasTy { def_id, args, .. }) => {
             // If it's already in the same alias, don't get an infinite loop.
             if cx.current_type_aliases.contains_key(&def_id) {
@@ -2296,7 +2265,6 @@ pub(crate) fn clean_middle_ty<'tcx>(
 
         ty::Closure(..) => panic!("Closure"),
         ty::Generator(..) => panic!("Generator"),
-        ty::Bound(..) => panic!("Bound"),
         ty::Placeholder(..) => panic!("Placeholder"),
         ty::GeneratorWitness(..) => panic!("GeneratorWitness"),
         ty::Infer(..) => panic!("Infer"),
@@ -3127,3 +3095,30 @@ fn clean_type_binding<'tcx>(
         },
     }
 }
+
+fn clean_bound_vars<'tcx>(
+    bound_vars: &'tcx ty::List<ty::BoundVariableKind>,
+) -> Vec<GenericParamDef> {
+    bound_vars
+        .into_iter()
+        .filter_map(|var| match var {
+            ty::BoundVariableKind::Region(ty::BrNamed(_, name))
+                if name != kw::UnderscoreLifetime =>
+            {
+                Some(GenericParamDef::lifetime(name))
+            }
+            ty::BoundVariableKind::Ty(ty::BoundTyKind::Param(did, name)) => Some(GenericParamDef {
+                name,
+                kind: GenericParamDefKind::Type {
+                    did,
+                    bounds: Vec::new(),
+                    default: None,
+                    synthetic: false,
+                },
+            }),
+            // FIXME(non_lifetime_binders): Support higher-ranked const parameters.
+            ty::BoundVariableKind::Const => None,
+            _ => None,
+        })
+        .collect()
+}

src/librustdoc/clean/simplify.rs

@@ -40,18 +40,18 @@ pub(crate) fn where_clauses(cx: &DocContext<'_>, clauses: Vec<WP>) -> ThinVec<WP
             WP::RegionPredicate { lifetime, bounds } => {
                 lifetimes.push((lifetime, bounds));
             }
-            WP::EqPredicate { lhs, rhs, bound_params } => equalities.push((lhs, rhs, bound_params)),
+            WP::EqPredicate { lhs, rhs } => equalities.push((lhs, rhs)),
         }
     }
 
     // Look for equality predicates on associated types that can be merged into
     // general bound predicates.
-    equalities.retain(|(lhs, rhs, bound_params)| {
+    equalities.retain(|(lhs, rhs)| {
         let Some((ty, trait_did, name)) = lhs.projection() else {
             return true;
         };
         let Some((bounds, _)) = tybounds.get_mut(ty) else { return true };
-        merge_bounds(cx, bounds, bound_params.clone(), trait_did, name, rhs)
+        merge_bounds(cx, bounds, trait_did, name, rhs)
     });
 
     // And finally, let's reassemble everything
@@ -64,18 +64,13 @@ pub(crate) fn where_clauses(cx: &DocContext<'_>, clauses: Vec<WP>) -> ThinVec<WP
         bounds,
         bound_params,
     }));
-    clauses.extend(equalities.into_iter().map(|(lhs, rhs, bound_params)| WP::EqPredicate {
-        lhs,
-        rhs,
-        bound_params,
-    }));
+    clauses.extend(equalities.into_iter().map(|(lhs, rhs)| WP::EqPredicate { lhs, rhs }));
     clauses
 }
 
 pub(crate) fn merge_bounds(
     cx: &clean::DocContext<'_>,
     bounds: &mut Vec<clean::GenericBound>,
-    mut bound_params: Vec<clean::GenericParamDef>,
     trait_did: DefId,
     assoc: clean::PathSegment,
     rhs: &clean::Term,
@@ -93,12 +88,6 @@ pub(crate) fn merge_bounds(
         }
         let last = trait_ref.trait_.segments.last_mut().expect("segments were empty");
 
-        trait_ref.generic_params.append(&mut bound_params);
-        // Sort parameters (likely) originating from a hashset alphabetically to
-        // produce predictable output (and to allow for full deduplication).
-        trait_ref.generic_params.sort_unstable_by(|p, q| p.name.as_str().cmp(q.name.as_str()));
-        trait_ref.generic_params.dedup_by_key(|p| p.name);
-
         match last.args {
             PP::AngleBracketed { ref mut bindings, .. } => {
                 bindings.push(clean::TypeBinding {

src/librustdoc/clean/types.rs

@@ -1289,7 +1289,7 @@ impl Lifetime {
 pub(crate) enum WherePredicate {
     BoundPredicate { ty: Type, bounds: Vec<GenericBound>, bound_params: Vec<GenericParamDef> },
     RegionPredicate { lifetime: Lifetime, bounds: Vec<GenericBound> },
-    EqPredicate { lhs: Box<Type>, rhs: Box<Term>, bound_params: Vec<GenericParamDef> },
+    EqPredicate { lhs: Type, rhs: Term },
 }
 
 impl WherePredicate {
@@ -1300,15 +1300,6 @@ impl WherePredicate {
             _ => None,
         }
     }
-
-    pub(crate) fn get_bound_params(&self) -> Option<&[GenericParamDef]> {
-        match self {
-            Self::BoundPredicate { bound_params, .. } | Self::EqPredicate { bound_params, .. } => {
-                Some(bound_params)
-            }
-            _ => None,
-        }
-    }
 }
 
 #[derive(Clone, PartialEq, Eq, Debug, Hash)]

src/librustdoc/html/format.rs

@@ -326,8 +326,7 @@ pub(crate) fn print_where_clause<'a, 'tcx: 'a>(
                         bounds_display.truncate(bounds_display.len() - " + ".len());
                         write!(f, "{}: {bounds_display}", lifetime.print())
                     }
-                    // FIXME(fmease): Render bound params.
-                    clean::WherePredicate::EqPredicate { lhs, rhs, bound_params: _ } => {
+                    clean::WherePredicate::EqPredicate { lhs, rhs } => {
                         if f.alternate() {
                             write!(f, "{:#} == {:#}", lhs.print(cx), rhs.print(cx))
                         } else {

src/librustdoc/json/conversions.rs

@@ -506,9 +506,8 @@ impl FromWithTcx<clean::WherePredicate> for WherePredicate {
                 lifetime: convert_lifetime(lifetime),
                 bounds: bounds.into_tcx(tcx),
             },
-            // FIXME(fmease): Convert bound parameters as well.
-            EqPredicate { lhs, rhs, bound_params: _ } => {
-                WherePredicate::EqPredicate { lhs: (*lhs).into_tcx(tcx), rhs: (*rhs).into_tcx(tcx) }
+            EqPredicate { lhs, rhs } => {
+                WherePredicate::EqPredicate { lhs: lhs.into_tcx(tcx), rhs: rhs.into_tcx(tcx) }
             }
         }
     }

tests/rustdoc/inline_cross/auxiliary/fn-type.rs

@@ -0,0 +1 @@
+pub type F = for<'z, 'a, '_unused> fn(&'z for<'b> fn(&'b str), &'a ()) -> &'a ();

tests/rustdoc/inline_cross/auxiliary/impl_trait_aux.rs

@@ -15,7 +15,7 @@ pub fn func4<T: Iterator<Item = impl Clone>>(_x: T) {}
 
 pub fn func5(
     _f: impl for<'any> Fn(&'any str, &'any str) -> bool + for<'r> Other<T<'r> = ()>,
-    _a: impl for<'alpha, 'beta> Auxiliary<'alpha, Item<'beta> = fn(&'beta ())>,
+    _a: impl for<'beta, 'alpha, '_gamma> Auxiliary<'alpha, Item<'beta> = fn(&'beta ())>,
 ) {}
 
 pub trait Other {

tests/rustdoc/inline_cross/auxiliary/non_lifetime_binders.rs

@@ -0,0 +1,10 @@
+#![feature(non_lifetime_binders)]
+
+pub trait Trait<T> {}
+
+pub fn f(_: impl for<T> Trait<T>) {}
+
+pub fn g<T>(_: T)
+where
+    T: for<U> Trait<U>,
+{}