From a8d869e1d151d70137af8e7a03155cb6ac93139c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Tue, 13 Feb 2024 11:52:27 +0100 Subject: [PATCH] rustdoc: cross-crate re-exports: correctly render late-bound params in source order even if early-bound params are present --- src/librustdoc/clean/auto_trait.rs | 2 +- src/librustdoc/clean/inline.rs | 50 +++++++++---- src/librustdoc/clean/mod.rs | 71 +++++++++---------- src/librustdoc/clean/types.rs | 7 +- src/librustdoc/json/conversions.rs | 25 +++---- .../early-late-bound-lifetime-params.rs | 17 +++++ .../early-late-bound-lifetime-params.rs | 17 +++++ 7 files changed, 119 insertions(+), 70 deletions(-) create mode 100644 tests/rustdoc/inline_cross/auxiliary/early-late-bound-lifetime-params.rs create mode 100644 tests/rustdoc/inline_cross/early-late-bound-lifetime-params.rs diff --git a/src/librustdoc/clean/auto_trait.rs b/src/librustdoc/clean/auto_trait.rs index 9de547ba6dcef..8cc4201c3fc25 100644 --- 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 index 23a059c6e0302..e2f2c9a5e5663 100644 --- 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 { - 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 { + 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 index f4527d1e55e85..c4a9243816e5c 100644 --- 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, 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 index 96b4d1a45f6ea..9b7ec6e109a79 100644 --- 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 }, - Type { did: DefId, bounds: ThinVec, default: Option>, synthetic: bool }, + Type { bounds: ThinVec, default: Option>, synthetic: bool }, // Option> makes this type smaller than `Option` would. Const { ty: Box, default: Option>, 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 index 32d7a80863d6d..cb50a27326fe5 100644 --- a/src/librustdoc/json/conversions.rs +++ b/src/librustdoc/json/conversions.rs @@ -456,7 +456,7 @@ impl FromWithTcx 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 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 new file mode 100644 index 0000000000000..70f7a84a8dcc9 --- /dev/null +++ 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 new file mode 100644 index 0000000000000..09cc8a79072b9 --- /dev/null +++ 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;