rustdoc: cross-crate re-exports: correctly render late-bound params in source order even if early-bound params are present

fmease · fmease · commit a8d869e1d151 · 2024-02-15T01:40:38.000+01:00
diff --git a/src/librustdoc/clean/auto_trait.rs b/src/librustdoc/clean/auto_trait.rs
@@ -334,7 +334,7 @@ where
                 match br {
                     // We only care about named late bound regions, as we need to add them
                     // to the 'for<>' section
-                    ty::BrNamed(_, name) => Some(GenericParamDef::lifetime(name)),
+                    ty::BrNamed(def_id, name) => Some(GenericParamDef::lifetime(def_id, name)),
                     _ => None,
                 }
             })
diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs
@@ -18,8 +18,8 @@ use rustc_span::hygiene::MacroKind;
 use rustc_span::symbol::{kw, sym, Symbol};
 
 use crate::clean::{
-    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,
+    self, clean_bound_vars, clean_generics, clean_impl_item, clean_middle_assoc_item,
+    clean_middle_field, clean_middle_ty, clean_poly_fn_sig, clean_trait_ref_with_bindings,
     clean_ty, clean_ty_alias_inner_type, clean_ty_generics, clean_variant_def, utils, Attributes,
     AttributesExt, ImplKind, ItemId, Type,
 };
@@ -72,7 +72,9 @@ pub(crate) fn try_inline(
         }
         Res::Def(DefKind::Fn, did) => {
             record_extern_fqn(cx, did, ItemType::Function);
-            cx.with_param_env(did, |cx| clean::FunctionItem(build_external_function(cx, did)))
+            cx.with_param_env(did, |cx| {
+                clean::enter_impl_trait(cx, |cx| clean::FunctionItem(build_function(cx, did)))
+            })
         }
         Res::Def(DefKind::Struct, did) => {
             record_extern_fqn(cx, did, ItemType::Struct);
@@ -274,18 +276,38 @@ pub(crate) fn build_external_trait(cx: &mut DocContext<'_>, did: DefId) -> clean
     clean::Trait { def_id: did, generics, items: trait_items, bounds: supertrait_bounds }
 }
 
-fn build_external_function<'tcx>(cx: &mut DocContext<'tcx>, did: DefId) -> Box<clean::Function> {
-    let sig = cx.tcx.fn_sig(did).instantiate_identity();
-    let predicates = cx.tcx.explicit_predicates_of(did);
+pub(crate) fn build_function<'tcx>(
+    cx: &mut DocContext<'tcx>,
+    def_id: DefId,
+) -> Box<clean::Function> {
+    let sig = cx.tcx.fn_sig(def_id).instantiate_identity();
+    // The generics need to be cleaned before the signature.
+    let mut generics =
+        clean_ty_generics(cx, cx.tcx.generics_of(def_id), cx.tcx.explicit_predicates_of(def_id));
+    let bound_vars = clean_bound_vars(sig.bound_vars());
+
+    // At the time of writing early & late-bound params are stored separately in rustc,
+    // namely in `generics.params` and `bound_vars` respectively.
+    //
+    // To reestablish the original source code order of the generic parameters, we
+    // need to manually sort them by their definition span after concatenation.
+    //
+    // See also:
+    // * https://rustc-dev-guide.rust-lang.org/bound-vars-and-params.html
+    // * https://rustc-dev-guide.rust-lang.org/what-does-early-late-bound-mean.html
+    let has_early_bound_params = !generics.params.is_empty();
+    let has_late_bound_params = !bound_vars.is_empty();
+    generics.params.extend(bound_vars);
+    if has_early_bound_params && has_late_bound_params {
+        // If this ever becomes a performances bottleneck either due to the sorting
+        // or due to the query calls, consider inserting the late-bound lifetime params
+        // right after the last early-bound lifetime param followed by only sorting
+        // the slice of lifetime params.
+        generics.params.sort_by_key(|param| cx.tcx.def_ident_span(param.def_id).unwrap());
+    }
+
+    let decl = clean_poly_fn_sig(cx, Some(def_id), sig);
 
-    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(clean_bound_vars(sig.bound_vars()));
-        let decl = clean_fn_decl_from_did_and_sig(cx, Some(did), sig);
-        (generics, decl)
-    });
     Box::new(clean::Function { decl, generics })
 }
 
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
@@ -525,7 +525,6 @@ fn clean_generic_param_def<'tcx>(
             (
                 def.name,
                 GenericParamDefKind::Type {
-                    did: def.def_id,
                     bounds: ThinVec::new(), // These are filled in from the where-clauses.
                     default: default.map(Box::new),
                     synthetic,
@@ -557,7 +556,7 @@ fn clean_generic_param_def<'tcx>(
         ),
     };
 
-    GenericParamDef { name, kind }
+    GenericParamDef { name, def_id: def.def_id, kind }
 }
 
 fn clean_generic_param<'tcx>(
@@ -596,7 +595,6 @@ fn clean_generic_param<'tcx>(
             (
                 param.name.ident().name,
                 GenericParamDefKind::Type {
-                    did: param.def_id.to_def_id(),
                     bounds,
                     default: default.map(|t| clean_ty(t, cx)).map(Box::new),
                     synthetic,
@@ -614,7 +612,7 @@ fn clean_generic_param<'tcx>(
         ),
     };
 
-    GenericParamDef { name, kind }
+    GenericParamDef { name, def_id: param.def_id.to_def_id(), kind }
 }
 
 /// Synthetic type-parameters are inserted after normal ones.
@@ -646,8 +644,8 @@ pub(crate) fn clean_generics<'tcx>(
             let param = clean_generic_param(cx, Some(gens), param);
             match param.kind {
                 GenericParamDefKind::Lifetime { .. } => unreachable!(),
-                GenericParamDefKind::Type { did, ref bounds, .. } => {
-                    cx.impl_trait_bounds.insert(did.into(), bounds.to_vec());
+                GenericParamDefKind::Type { ref bounds, .. } => {
+                    cx.impl_trait_bounds.insert(param.def_id.into(), bounds.to_vec());
                 }
                 GenericParamDefKind::Const { .. } => unreachable!(),
             }
@@ -1064,8 +1062,11 @@ fn clean_fn_decl_legacy_const_generics(func: &mut Function, attrs: &[ast::Attrib
             match literal.kind {
                 ast::LitKind::Int(a, _) => {
                     let gen = func.generics.params.remove(0);
-                    if let GenericParamDef { name, kind: GenericParamDefKind::Const { ty, .. } } =
-                        gen
+                    if let GenericParamDef {
+                        name,
+                        kind: GenericParamDefKind::Const { ty, .. },
+                        ..
+                    } = gen
                     {
                         func.decl
                             .inputs
@@ -1169,7 +1170,7 @@ fn clean_fn_decl_with_args<'tcx>(
     FnDecl { inputs: args, output, c_variadic: decl.c_variadic }
 }
 
-fn clean_fn_decl_from_did_and_sig<'tcx>(
+fn clean_poly_fn_sig<'tcx>(
     cx: &mut DocContext<'tcx>,
     did: Option<DefId>,
     sig: ty::PolyFnSig<'tcx>,
@@ -1359,16 +1360,7 @@ pub(crate) fn clean_middle_assoc_item<'tcx>(
             }
         }
         ty::AssocKind::Fn => {
-            let sig = tcx.fn_sig(assoc_item.def_id).instantiate_identity();
-            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(clean_bound_vars(sig.bound_vars()));
-
-            let mut decl = clean_fn_decl_from_did_and_sig(cx, Some(assoc_item.def_id), sig);
+            let mut item = inline::build_function(cx, assoc_item.def_id);
 
             if assoc_item.fn_has_self_parameter {
                 let self_ty = match assoc_item.container {
@@ -1377,12 +1369,13 @@ pub(crate) fn clean_middle_assoc_item<'tcx>(
                     }
                     ty::TraitContainer => tcx.types.self_param,
                 };
-                let self_arg_ty = sig.input(0).skip_binder();
+                let self_arg_ty =
+                    tcx.fn_sig(assoc_item.def_id).instantiate_identity().input(0).skip_binder();
                 if self_arg_ty == self_ty {
-                    decl.inputs.values[0].type_ = Generic(kw::SelfUpper);
+                    item.decl.inputs.values[0].type_ = Generic(kw::SelfUpper);
                 } else if let ty::Ref(_, ty, _) = *self_arg_ty.kind() {
                     if ty == self_ty {
-                        match decl.inputs.values[0].type_ {
+                        match item.decl.inputs.values[0].type_ {
                             BorrowedRef { ref mut type_, .. } => **type_ = Generic(kw::SelfUpper),
                             _ => unreachable!(),
                         }
@@ -1399,9 +1392,9 @@ pub(crate) fn clean_middle_assoc_item<'tcx>(
                     ty::ImplContainer => Some(assoc_item.defaultness(tcx)),
                     ty::TraitContainer => None,
                 };
-                MethodItem(Box::new(Function { generics, decl }), defaultness)
+                MethodItem(item, defaultness)
             } else {
-                TyMethodItem(Box::new(Function { generics, decl }))
+                TyMethodItem(item)
             }
         }
         ty::AssocKind::Type => {
@@ -2109,7 +2102,7 @@ pub(crate) fn clean_middle_ty<'tcx>(
         ty::FnDef(..) | ty::FnPtr(_) => {
             // 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 decl = clean_poly_fn_sig(cx, None, sig);
             let generic_params = clean_bound_vars(sig.bound_vars());
 
             BareFunction(Box::new(BareFunctionDecl {
@@ -2192,10 +2185,10 @@ pub(crate) fn clean_middle_ty<'tcx>(
                 .iter()
                 .flat_map(|pred| pred.bound_vars())
                 .filter_map(|var| match var {
-                    ty::BoundVariableKind::Region(ty::BrNamed(_, name))
+                    ty::BoundVariableKind::Region(ty::BrNamed(def_id, name))
                         if name != kw::UnderscoreLifetime =>
                     {
-                        Some(GenericParamDef::lifetime(name))
+                        Some(GenericParamDef::lifetime(def_id, name))
                     }
                     _ => None,
                 })
@@ -3167,20 +3160,22 @@ fn clean_bound_vars<'tcx>(
     bound_vars
         .into_iter()
         .filter_map(|var| match var {
-            ty::BoundVariableKind::Region(ty::BrNamed(_, name))
+            ty::BoundVariableKind::Region(ty::BrNamed(def_id, name))
                 if name != kw::UnderscoreLifetime =>
             {
-                Some(GenericParamDef::lifetime(name))
+                Some(GenericParamDef::lifetime(def_id, name))
+            }
+            ty::BoundVariableKind::Ty(ty::BoundTyKind::Param(def_id, name)) => {
+                Some(GenericParamDef {
+                    name,
+                    def_id,
+                    kind: GenericParamDefKind::Type {
+                        bounds: ThinVec::new(),
+                        default: None,
+                        synthetic: false,
+                    },
+                })
             }
-            ty::BoundVariableKind::Ty(ty::BoundTyKind::Param(did, name)) => Some(GenericParamDef {
-                name,
-                kind: GenericParamDefKind::Type {
-                    did,
-                    bounds: ThinVec::new(),
-                    default: None,
-                    synthetic: false,
-                },
-            }),
             // FIXME(non_lifetime_binders): Support higher-ranked const parameters.
             ty::BoundVariableKind::Const => None,
             _ => None,
diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
@@ -1326,7 +1326,7 @@ impl WherePredicate {
 #[derive(Clone, PartialEq, Eq, Debug, Hash)]
 pub(crate) enum GenericParamDefKind {
     Lifetime { outlives: ThinVec<Lifetime> },
-    Type { did: DefId, bounds: ThinVec<GenericBound>, default: Option<Box<Type>>, synthetic: bool },
+    Type { bounds: ThinVec<GenericBound>, default: Option<Box<Type>>, synthetic: bool },
     // Option<Box<String>> makes this type smaller than `Option<String>` would.
     Const { ty: Box<Type>, default: Option<Box<String>>, is_host_effect: bool },
 }
@@ -1340,12 +1340,13 @@ impl GenericParamDefKind {
 #[derive(Clone, PartialEq, Eq, Debug, Hash)]
 pub(crate) struct GenericParamDef {
     pub(crate) name: Symbol,
+    pub(crate) def_id: DefId,
     pub(crate) kind: GenericParamDefKind,
 }
 
 impl GenericParamDef {
-    pub(crate) fn lifetime(name: Symbol) -> Self {
-        Self { name, kind: GenericParamDefKind::Lifetime { outlives: ThinVec::new() } }
+    pub(crate) fn lifetime(def_id: DefId, name: Symbol) -> Self {
+        Self { name, def_id, kind: GenericParamDefKind::Lifetime { outlives: ThinVec::new() } }
     }
 
     pub(crate) fn is_synthetic_param(&self) -> bool {
diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs
@@ -456,7 +456,7 @@ impl FromWithTcx<clean::GenericParamDefKind> for GenericParamDefKind {
             Lifetime { outlives } => GenericParamDefKind::Lifetime {
                 outlives: outlives.into_iter().map(convert_lifetime).collect(),
             },
-            Type { did: _, bounds, default, synthetic } => GenericParamDefKind::Type {
+            Type { bounds, default, synthetic } => GenericParamDefKind::Type {
                 bounds: bounds.into_tcx(tcx),
                 default: default.map(|x| (*x).into_tcx(tcx)),
                 synthetic,
@@ -486,19 +486,16 @@ impl FromWithTcx<clean::WherePredicate> for WherePredicate {
                                     outlives: outlives.iter().map(|lt| lt.0.to_string()).collect(),
                                 }
                             }
-                            clean::GenericParamDefKind::Type {
-                                did: _,
-                                bounds,
-                                default,
-                                synthetic,
-                            } => GenericParamDefKind::Type {
-                                bounds: bounds
-                                    .into_iter()
-                                    .map(|bound| bound.into_tcx(tcx))
-                                    .collect(),
-                                default: default.map(|ty| (*ty).into_tcx(tcx)),
-                                synthetic,
-                            },
+                            clean::GenericParamDefKind::Type { bounds, default, synthetic } => {
+                                GenericParamDefKind::Type {
+                                    bounds: bounds
+                                        .into_iter()
+                                        .map(|bound| bound.into_tcx(tcx))
+                                        .collect(),
+                                    default: default.map(|ty| (*ty).into_tcx(tcx)),
+                                    synthetic,
+                                }
+                            }
                             clean::GenericParamDefKind::Const {
                                 ty,
                                 default,
diff --git a/tests/rustdoc/inline_cross/auxiliary/early-late-bound-lifetime-params.rs b/tests/rustdoc/inline_cross/auxiliary/early-late-bound-lifetime-params.rs
@@ -0,0 +1,17 @@
+// Here, `'a` and `'c` are late-bound and `'b`, `'d`, `T` and `N` are early-bound.
+
+pub fn f<'a, 'b, 'c, 'd, T, const N: usize>(_: impl Copy)
+where
+    'b:,
+    'd:,
+{}
+
+pub struct Ty;
+
+impl Ty {
+    pub fn f<'a, 'b, 'c, 'd, T, const N: usize>(_: impl Copy)
+    where
+        'b:,
+        'd:,
+    {}
+}
diff --git a/tests/rustdoc/inline_cross/early-late-bound-lifetime-params.rs b/tests/rustdoc/inline_cross/early-late-bound-lifetime-params.rs
@@ -0,0 +1,17 @@
+// Check that we correctly render late-bound lifetime params in source order
+// even if early-bound generic params are present.
+//
+// For context, at the time of writing early- and late-bound params are stored
+// separately in rustc and therefore rustdoc needs to manually merge them.
+
+#![crate_name = "usr"]
+// aux-crate:dep=early-late-bound-lifetime-params.rs
+// edition:2021
+
+// @has usr/fn.f.html
+// @has - '//pre[@class="rust item-decl"]' "fn f<'a, 'b, 'c, 'd, T, const N: usize>(_: impl Copy)"
+pub use dep::f;
+
+// @has usr/struct.Ty.html
+// @has - '//*[@id="method.f"]' "fn f<'a, 'b, 'c, 'd, T, const N: usize>(_: impl Copy)"
+pub use dep::Ty;

Original file line number	Diff line number	Diff line change
`@@ -334,7 +334,7 @@ where`
`334`	`334`	`match br {`
`335`	`335`	`// We only care about named late bound regions, as we need to add them`
`336`	`336`	`// to the 'for<>' section`
`337`		`- ty::BrNamed(_, name) => Some(GenericParamDef::lifetime(name)),`
	`337`	`+ ty::BrNamed(def_id, name) => Some(GenericParamDef::lifetime(def_id, name)),`
`338`	`338`	`_ => None,`
`339`	`339`	`}`
`340`	`340`	`})`