From f141a524e8cf778756931ff8c776f4934eda1c58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Mon, 8 Jan 2024 22:37:42 +0100 Subject: [PATCH 1/6] Rename rustc_middle path cleaning functions The new names are consistent with the other rustc_middle cleaning functions. Regarding the local variable `ty_args`, it's used throughout the function and personally speaking its name isn't very legible, I trip up on it. --- src/librustdoc/clean/mod.rs | 26 +++++++++++++-------- src/librustdoc/clean/types.rs | 4 ++-- src/librustdoc/clean/utils.rs | 44 +++++++++++++++-------------------- 3 files changed, 37 insertions(+), 37 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 1a25d3f7993..520d60f5b9b 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -207,8 +207,13 @@ pub(crate) fn clean_trait_ref_with_bindings<'tcx>( span_bug!(cx.tcx.def_span(trait_ref.def_id()), "`TraitRef` had unexpected kind {kind:?}"); } inline::record_extern_fqn(cx, trait_ref.def_id(), kind); - let path = - external_path(cx, trait_ref.def_id(), true, bindings, trait_ref.map_bound(|tr| tr.args)); + let path = clean_middle_path( + cx, + trait_ref.def_id(), + true, + bindings, + trait_ref.map_bound(|tr| tr.args), + ); debug!(?trait_ref); @@ -467,7 +472,7 @@ fn projection_to_path_segment<'tcx>( PathSegment { name: item.name, args: GenericArgs::AngleBracketed { - args: ty_args_to_args( + args: clean_middle_generic_args( cx, ty.map_bound(|ty| &ty.args[generics.parent_count..]), false, @@ -2096,12 +2101,12 @@ pub(crate) fn clean_middle_ty<'tcx>( AdtKind::Enum => ItemType::Enum, }; inline::record_extern_fqn(cx, did, kind); - let path = external_path(cx, did, false, ThinVec::new(), bound_ty.rebind(args)); + let path = clean_middle_path(cx, did, false, ThinVec::new(), bound_ty.rebind(args)); Type::Path { path } } ty::Foreign(did) => { inline::record_extern_fqn(cx, did, ItemType::ForeignType); - let path = external_path( + let path = clean_middle_path( cx, did, false, @@ -2132,7 +2137,7 @@ pub(crate) fn clean_middle_ty<'tcx>( let mut bounds = dids .map(|did| { let empty = ty::Binder::dummy(ty::GenericArgs::empty()); - let path = external_path(cx, did, false, ThinVec::new(), empty); + let path = clean_middle_path(cx, did, false, ThinVec::new(), empty); inline::record_extern_fqn(cx, did, ItemType::Trait); PolyTrait { trait_: path, generic_params: Vec::new() } }) @@ -2171,7 +2176,7 @@ pub(crate) fn clean_middle_ty<'tcx>( .collect(); let late_bound_regions = late_bound_regions.into_iter().collect(); - let path = external_path(cx, did, false, bindings, args); + let path = clean_middle_path(cx, did, false, bindings, args); bounds.insert(0, PolyTrait { trait_: path, generic_params: late_bound_regions }); DynTrait(bounds, lifetime) @@ -2193,7 +2198,7 @@ pub(crate) fn clean_middle_ty<'tcx>( assoc: PathSegment { name: cx.tcx.associated_item(def_id).name, args: GenericArgs::AngleBracketed { - args: ty_args_to_args( + args: clean_middle_generic_args( cx, alias_ty.map_bound(|ty| ty.args.as_slice()), true, @@ -2213,7 +2218,7 @@ pub(crate) fn clean_middle_ty<'tcx>( if cx.tcx.features().lazy_type_alias { // Weak type alias `data` represents the `type X` in `type X = Y`. If we need `Y`, // we need to use `type_of`. - let path = external_path( + let path = clean_middle_path( cx, data.def_id, false, @@ -2243,7 +2248,8 @@ pub(crate) fn clean_middle_ty<'tcx>( 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) { - let path = external_path(cx, def_id, false, ThinVec::new(), bound_ty.rebind(args)); + let path = + clean_middle_path(cx, def_id, false, ThinVec::new(), bound_ty.rebind(args)); Type::Path { path } } else { *cx.current_type_aliases.entry(def_id).or_insert(0) += 1; diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 179f37e6d96..90eb783c7ca 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -36,7 +36,7 @@ use rustc_target::spec::abi::Abi; use crate::clean::cfg::Cfg; -use crate::clean::external_path; +use crate::clean::clean_middle_path; use crate::clean::inline::{self, print_inlined_const}; use crate::clean::utils::{is_literal_expr, print_evaluated_const}; use crate::core::DocContext; @@ -1258,7 +1258,7 @@ pub(crate) fn maybe_sized(cx: &mut DocContext<'_>) -> GenericBound { fn sized_with(cx: &mut DocContext<'_>, modifier: hir::TraitBoundModifier) -> GenericBound { let did = cx.tcx.require_lang_item(LangItem::Sized, None); let empty = ty::Binder::dummy(ty::GenericArgs::empty()); - let path = external_path(cx, did, false, ThinVec::new(), empty); + let path = clean_middle_path(cx, did, false, ThinVec::new(), empty); inline::record_extern_fqn(cx, did, ItemType::Trait); GenericBound::TraitBound(PolyTrait { trait_: path, generic_params: Vec::new() }, modifier) } diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index bdfda07be09..cb5cfef6382 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -75,13 +75,13 @@ pub(crate) fn krate(cx: &mut DocContext<'_>) -> Crate { Crate { module, external_traits: cx.external_traits.clone() } } -pub(crate) fn ty_args_to_args<'tcx>( +pub(crate) fn clean_middle_generic_args<'tcx>( cx: &mut DocContext<'tcx>, - ty_args: ty::Binder<'tcx, &'tcx [ty::GenericArg<'tcx>]>, + args: ty::Binder<'tcx, &'tcx [ty::GenericArg<'tcx>]>, has_self: bool, owner: DefId, ) -> Vec { - if ty_args.skip_binder().is_empty() { + if args.skip_binder().is_empty() { // Fast path which avoids executing the query `generics_of`. return Vec::new(); } @@ -90,9 +90,9 @@ pub(crate) fn ty_args_to_args<'tcx>( let mut elision_has_failed_once_before = false; let offset = if has_self { 1 } else { 0 }; - let mut args = Vec::with_capacity(ty_args.skip_binder().len().saturating_sub(offset)); + let mut clean_args = Vec::with_capacity(args.skip_binder().len().saturating_sub(offset)); - let ty_arg_to_arg = |(index, arg): (usize, &ty::GenericArg<'tcx>)| match arg.unpack() { + let clean_arg = |(index, arg): (usize, &ty::GenericArg<'tcx>)| match arg.unpack() { GenericArgKind::Lifetime(lt) => { Some(GenericArg::Lifetime(clean_middle_region(lt).unwrap_or(Lifetime::elided()))) } @@ -101,10 +101,9 @@ pub(crate) fn ty_args_to_args<'tcx>( if !elision_has_failed_once_before && let Some(default) = params[index].default_value(cx.tcx) { - let default = - ty_args.map_bound(|args| default.instantiate(cx.tcx, args).expect_ty()); + let default = args.map_bound(|args| default.instantiate(cx.tcx, args).expect_ty()); - if can_elide_generic_arg(ty_args.rebind(ty), default) { + if can_elide_generic_arg(args.rebind(ty), default) { return None; } @@ -112,15 +111,10 @@ pub(crate) fn ty_args_to_args<'tcx>( } Some(GenericArg::Type(clean_middle_ty( - ty_args.rebind(ty), + args.rebind(ty), cx, None, - Some(crate::clean::ContainerTy::Regular { - ty: owner, - args: ty_args, - has_self, - arg: index, - }), + Some(crate::clean::ContainerTy::Regular { ty: owner, args, has_self, arg: index }), ))) } GenericArgKind::Const(ct) => { @@ -133,22 +127,22 @@ pub(crate) fn ty_args_to_args<'tcx>( && let Some(default) = params[index].default_value(cx.tcx) { let default = - ty_args.map_bound(|args| default.instantiate(cx.tcx, args).expect_const()); + args.map_bound(|args| default.instantiate(cx.tcx, args).expect_const()); - if can_elide_generic_arg(ty_args.rebind(ct), default) { + if can_elide_generic_arg(args.rebind(ct), default) { return None; } elision_has_failed_once_before = true; } - Some(GenericArg::Const(Box::new(clean_middle_const(ty_args.rebind(ct), cx)))) + Some(GenericArg::Const(Box::new(clean_middle_const(args.rebind(ct), cx)))) } }; - args.extend(ty_args.skip_binder().iter().enumerate().rev().filter_map(ty_arg_to_arg)); - args.reverse(); - args + clean_args.extend(args.skip_binder().iter().enumerate().rev().filter_map(clean_arg)); + clean_args.reverse(); + clean_args } /// Check if the generic argument `actual` coincides with the `default` and can therefore be elided. @@ -192,14 +186,14 @@ fn can_elide_generic_arg<'tcx, Term>( actual.skip_binder() == default.skip_binder() } -fn external_generic_args<'tcx>( +fn clean_middle_generic_args_with_bindings<'tcx>( cx: &mut DocContext<'tcx>, did: DefId, has_self: bool, bindings: ThinVec, ty_args: ty::Binder<'tcx, GenericArgsRef<'tcx>>, ) -> GenericArgs { - let args = ty_args_to_args(cx, ty_args.map_bound(|args| &args[..]), has_self, did); + let args = clean_middle_generic_args(cx, ty_args.map_bound(|args| &args[..]), has_self, did); if cx.tcx.fn_trait_kind_from_def_id(did).is_some() { let ty = ty_args @@ -225,7 +219,7 @@ fn external_generic_args<'tcx>( } } -pub(super) fn external_path<'tcx>( +pub(super) fn clean_middle_path<'tcx>( cx: &mut DocContext<'tcx>, did: DefId, has_self: bool, @@ -238,7 +232,7 @@ pub(super) fn external_path<'tcx>( res: Res::Def(def_kind, did), segments: thin_vec![PathSegment { name, - args: external_generic_args(cx, did, has_self, bindings, args), + args: clean_middle_generic_args_with_bindings(cx, did, has_self, bindings, args), }], } } From 5b32681db1704d99cb9aef11903362c70c2ac988 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Mon, 8 Jan 2024 23:28:30 +0100 Subject: [PATCH 2/6] Offset args of trait object types when cleaning --- src/librustdoc/clean/mod.rs | 14 ++++---------- src/librustdoc/clean/utils.rs | 31 +++++++++++++++++++++++++------ 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 520d60f5b9b..9cc2d9cecb7 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1988,8 +1988,9 @@ pub(crate) enum ContainerTy<'tcx> { Ref(ty::Region<'tcx>), Regular { ty: DefId, + /// The arguments *have* to contain an arg for the self type if the corresponding generics + /// contain a self type. args: ty::Binder<'tcx, &'tcx [ty::GenericArg<'tcx>]>, - has_self: bool, arg: usize, }, } @@ -1998,7 +1999,7 @@ impl<'tcx> ContainerTy<'tcx> { fn object_lifetime_default(self, tcx: TyCtxt<'tcx>) -> ObjectLifetimeDefault<'tcx> { match self { Self::Ref(region) => ObjectLifetimeDefault::Arg(region), - Self::Regular { ty: container, args, has_self, arg: index } => { + Self::Regular { ty: container, args, arg: index } => { let (DefKind::Struct | DefKind::Union | DefKind::Enum @@ -2011,14 +2012,7 @@ fn object_lifetime_default(self, tcx: TyCtxt<'tcx>) -> ObjectLifetimeDefault<'tc let generics = tcx.generics_of(container); debug_assert_eq!(generics.parent_count, 0); - // If the container is a trait object type, the arguments won't contain the self type but the - // generics of the corresponding trait will. In such a case, offset the index by one. - // For comparison, if the container is a trait inside a bound, the arguments do contain the - // self type. - let offset = - if !has_self && generics.parent.is_none() && generics.has_self { 1 } else { 0 }; - let param = generics.params[index + offset].def_id; - + let param = generics.params[index].def_id; let default = tcx.object_lifetime_default(param); match default { rbv::ObjectLifetimeDefault::Param(lifetime) => { diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index cb5cfef6382..aadc308d5ec 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -78,7 +78,7 @@ pub(crate) fn krate(cx: &mut DocContext<'_>) -> Crate { pub(crate) fn clean_middle_generic_args<'tcx>( cx: &mut DocContext<'tcx>, args: ty::Binder<'tcx, &'tcx [ty::GenericArg<'tcx>]>, - has_self: bool, + mut has_self: bool, owner: DefId, ) -> Vec { if args.skip_binder().is_empty() { @@ -86,12 +86,30 @@ pub(crate) fn clean_middle_generic_args<'tcx>( return Vec::new(); } - let params = &cx.tcx.generics_of(owner).params; + let generics = cx.tcx.generics_of(owner); let mut elision_has_failed_once_before = false; let offset = if has_self { 1 } else { 0 }; let mut clean_args = Vec::with_capacity(args.skip_binder().len().saturating_sub(offset)); + // If the container is a trait object type, the arguments won't contain the self type but the + // generics of the corresponding trait will. In such a case, prepend a dummy self type in order + // to align the arguments and parameters for the iteration below and to enable us to correctly + // instantiate the generic parameter default later. + let args = if !has_self && generics.parent.is_none() && generics.has_self { + has_self = true; + // FIXME(fmease): Don't arena-allocate the args (blocked on further refactorings)! + args.map_bound(|args| { + &*cx.tcx.arena.alloc_from_iter( + [cx.tcx.types.trait_object_dummy_self.into()] + .into_iter() + .chain(args.iter().copied()), + ) + }) + } else { + args + }; + let clean_arg = |(index, arg): (usize, &ty::GenericArg<'tcx>)| match arg.unpack() { GenericArgKind::Lifetime(lt) => { Some(GenericArg::Lifetime(clean_middle_region(lt).unwrap_or(Lifetime::elided()))) @@ -99,7 +117,7 @@ pub(crate) fn clean_middle_generic_args<'tcx>( GenericArgKind::Type(_) if has_self && index == 0 => None, GenericArgKind::Type(ty) => { if !elision_has_failed_once_before - && let Some(default) = params[index].default_value(cx.tcx) + && let Some(default) = generics.param_at(index, cx.tcx).default_value(cx.tcx) { let default = args.map_bound(|args| default.instantiate(cx.tcx, args).expect_ty()); @@ -114,17 +132,18 @@ pub(crate) fn clean_middle_generic_args<'tcx>( args.rebind(ty), cx, None, - Some(crate::clean::ContainerTy::Regular { ty: owner, args, has_self, arg: index }), + Some(crate::clean::ContainerTy::Regular { ty: owner, args, arg: index }), ))) } GenericArgKind::Const(ct) => { - if let ty::GenericParamDefKind::Const { is_host_effect: true, .. } = params[index].kind + if let ty::GenericParamDefKind::Const { is_host_effect: true, .. } = + generics.param_at(index, cx.tcx).kind { return None; } if !elision_has_failed_once_before - && let Some(default) = params[index].default_value(cx.tcx) + && let Some(default) = generics.param_at(index, cx.tcx).default_value(cx.tcx) { let default = args.map_bound(|args| default.instantiate(cx.tcx, args).expect_const()); From 17ec134fa421dd152df12063e45afe355e585d57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Mon, 8 Jan 2024 22:31:50 +0100 Subject: [PATCH 3/6] Update tests --- .../auxiliary/default-generic-args.rs | 9 ++++-- .../auxiliary/u_default_generic_args.rs | 1 + .../inline_cross/default-generic-args.rs | 32 ++++++++++++------- 3 files changed, 29 insertions(+), 13 deletions(-) create mode 100644 tests/rustdoc/inline_cross/auxiliary/u_default_generic_args.rs diff --git a/tests/rustdoc/inline_cross/auxiliary/default-generic-args.rs b/tests/rustdoc/inline_cross/auxiliary/default-generic-args.rs index 1e31f18927e..c2c94817fad 100644 --- a/tests/rustdoc/inline_cross/auxiliary/default-generic-args.rs +++ b/tests/rustdoc/inline_cross/auxiliary/default-generic-args.rs @@ -40,6 +40,11 @@ pub type M0 = Multi; -pub trait Trait<'a, T = &'a ()> {} +pub trait Trait0<'a, T = &'a ()> {} +pub type D0 = dyn for<'a> Trait0<'a>; -pub type F = dyn for<'a> Trait<'a>; +// Regression test for issue #119529. +pub trait Trait1 {} +pub type D1 = dyn Trait1; +pub type D2 = dyn Trait1<(), K>; +pub type D3 = dyn Trait1; diff --git a/tests/rustdoc/inline_cross/auxiliary/u_default_generic_args.rs b/tests/rustdoc/inline_cross/auxiliary/u_default_generic_args.rs new file mode 100644 index 00000000000..a742dd7d865 --- /dev/null +++ b/tests/rustdoc/inline_cross/auxiliary/u_default_generic_args.rs @@ -0,0 +1 @@ +pub use default_generic_args::*; diff --git a/tests/rustdoc/inline_cross/default-generic-args.rs b/tests/rustdoc/inline_cross/default-generic-args.rs index c9a87a19901..775bf041532 100644 --- a/tests/rustdoc/inline_cross/default-generic-args.rs +++ b/tests/rustdoc/inline_cross/default-generic-args.rs @@ -79,15 +79,14 @@ pub use default_generic_args::P2; // @has user/type.A0.html -// Ensure that we elide generic arguments that are alpha-equivalent to their respective -// generic parameter (modulo substs) (#1): -// @has - '//*[@class="rust item-decl"]//code' "Alpha" +// @has - '//*[@class="rust item-decl"]//code' "Alpha;" pub use default_generic_args::A0; // @has user/type.A1.html -// Ensure that we elide generic arguments that are alpha-equivalent to their respective -// generic parameter (modulo substs) (#1): -// @has - '//*[@class="rust item-decl"]//code' "Alpha" +// Demonstrates that we currently don't elide generic arguments that are alpha-equivalent to their +// respective generic parameter (after instantiation) for perf reasons (it would require us to +// create an inference context). +// @has - '//*[@class="rust item-decl"]//code' "Alpha fn(_: &'arbitrary ())>" pub use default_generic_args::A1; // @has user/type.M0.html @@ -97,8 +96,19 @@ // @has - '//*[@class="rust item-decl"]//code' "Multi" pub use default_generic_args::M0; -// @has user/type.F.html -// FIXME: Ideally, we would elide `&'a ()` but `'a` is an escaping bound var which we can't reason -// about at the moment since we don't keep track of bound vars. -// @has - '//*[@class="rust item-decl"]//code' "dyn for<'a> Trait<'a, &'a ()>" -pub use default_generic_args::F; +// @has user/type.D0.html +// @has - '//*[@class="rust item-decl"]//code' "dyn for<'a> Trait0<'a>" +pub use default_generic_args::D0; + +// Regression test for issue #119529. +// Check that we correctly elide def ty&const args inside trait object types. + +// @has user/type.D1.html +// @has - '//*[@class="rust item-decl"]//code' "dyn Trait1" +pub use default_generic_args::D1; +// @has user/type.D2.html +// @has - '//*[@class="rust item-decl"]//code' "dyn Trait1<(), K>" +pub use default_generic_args::D2; +// @has user/type.D3.html +// @has - '//*[@class="rust item-decl"]//code' "dyn Trait1;" +pub use default_generic_args::D3; From 4b859e68aa9454612a22c65f91c100fc7c920af4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Tue, 9 Jan 2024 13:56:28 +0100 Subject: [PATCH 4/6] Don't arena-allocate extended generic args --- src/librustdoc/clean/mod.rs | 12 ++++----- src/librustdoc/clean/utils.rs | 47 +++++++++++++++++++---------------- 2 files changed, 31 insertions(+), 28 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 9cc2d9cecb7..ab5cf549905 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1908,7 +1908,7 @@ fn normalize<'tcx>( fn clean_trait_object_lifetime_bound<'tcx>( region: ty::Region<'tcx>, - container: Option>, + container: Option>, preds: &'tcx ty::List>, tcx: TyCtxt<'tcx>, ) -> Option { @@ -1937,7 +1937,7 @@ fn clean_trait_object_lifetime_bound<'tcx>( fn can_elide_trait_object_lifetime_bound<'tcx>( region: ty::Region<'tcx>, - container: Option>, + container: Option>, preds: &'tcx ty::List>, tcx: TyCtxt<'tcx>, ) -> bool { @@ -1984,18 +1984,18 @@ fn can_elide_trait_object_lifetime_bound<'tcx>( } #[derive(Debug)] -pub(crate) enum ContainerTy<'tcx> { +pub(crate) enum ContainerTy<'a, 'tcx> { Ref(ty::Region<'tcx>), Regular { ty: DefId, /// The arguments *have* to contain an arg for the self type if the corresponding generics /// contain a self type. - args: ty::Binder<'tcx, &'tcx [ty::GenericArg<'tcx>]>, + args: ty::Binder<'tcx, &'a [ty::GenericArg<'tcx>]>, arg: usize, }, } -impl<'tcx> ContainerTy<'tcx> { +impl<'tcx> ContainerTy<'_, 'tcx> { fn object_lifetime_default(self, tcx: TyCtxt<'tcx>) -> ObjectLifetimeDefault<'tcx> { match self { Self::Ref(region) => ObjectLifetimeDefault::Arg(region), @@ -2044,7 +2044,7 @@ pub(crate) fn clean_middle_ty<'tcx>( bound_ty: ty::Binder<'tcx, Ty<'tcx>>, cx: &mut DocContext<'tcx>, parent_def_id: Option, - container: Option>, + container: Option>, ) -> Type { let bound_ty = normalize(cx, bound_ty).unwrap_or(bound_ty); match *bound_ty.skip_binder().kind() { diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index aadc308d5ec..e957cf1f2f5 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -81,7 +81,8 @@ pub(crate) fn clean_middle_generic_args<'tcx>( mut has_self: bool, owner: DefId, ) -> Vec { - if args.skip_binder().is_empty() { + let (args, bound_vars) = (args.skip_binder(), args.bound_vars()); + if args.is_empty() { // Fast path which avoids executing the query `generics_of`. return Vec::new(); } @@ -90,7 +91,7 @@ pub(crate) fn clean_middle_generic_args<'tcx>( let mut elision_has_failed_once_before = false; let offset = if has_self { 1 } else { 0 }; - let mut clean_args = Vec::with_capacity(args.skip_binder().len().saturating_sub(offset)); + let mut clean_args = Vec::with_capacity(args.len().saturating_sub(offset)); // If the container is a trait object type, the arguments won't contain the self type but the // generics of the corresponding trait will. In such a case, prepend a dummy self type in order @@ -98,16 +99,13 @@ pub(crate) fn clean_middle_generic_args<'tcx>( // instantiate the generic parameter default later. let args = if !has_self && generics.parent.is_none() && generics.has_self { has_self = true; - // FIXME(fmease): Don't arena-allocate the args (blocked on further refactorings)! - args.map_bound(|args| { - &*cx.tcx.arena.alloc_from_iter( - [cx.tcx.types.trait_object_dummy_self.into()] - .into_iter() - .chain(args.iter().copied()), - ) - }) + [cx.tcx.types.trait_object_dummy_self.into()] + .into_iter() + .chain(args.iter().copied()) + .collect::>() + .into() } else { - args + std::borrow::Cow::from(args) }; let clean_arg = |(index, arg): (usize, &ty::GenericArg<'tcx>)| match arg.unpack() { @@ -116,12 +114,13 @@ pub(crate) fn clean_middle_generic_args<'tcx>( } GenericArgKind::Type(_) if has_self && index == 0 => None, GenericArgKind::Type(ty) => { + let ty = ty::Binder::bind_with_vars(ty, bound_vars); + if !elision_has_failed_once_before && let Some(default) = generics.param_at(index, cx.tcx).default_value(cx.tcx) { - let default = args.map_bound(|args| default.instantiate(cx.tcx, args).expect_ty()); - - if can_elide_generic_arg(args.rebind(ty), default) { + let default = default.instantiate(cx.tcx, args.as_ref()).expect_ty(); + if can_elide_generic_arg(ty, ty.rebind(default)) { return None; } @@ -129,10 +128,14 @@ pub(crate) fn clean_middle_generic_args<'tcx>( } Some(GenericArg::Type(clean_middle_ty( - args.rebind(ty), + ty, cx, None, - Some(crate::clean::ContainerTy::Regular { ty: owner, args, arg: index }), + Some(crate::clean::ContainerTy::Regular { + ty: owner, + args: ty.rebind(args.as_ref()), + arg: index, + }), ))) } GenericArgKind::Const(ct) => { @@ -142,24 +145,24 @@ pub(crate) fn clean_middle_generic_args<'tcx>( return None; } + let ct = ty::Binder::bind_with_vars(ct, bound_vars); + if !elision_has_failed_once_before && let Some(default) = generics.param_at(index, cx.tcx).default_value(cx.tcx) { - let default = - args.map_bound(|args| default.instantiate(cx.tcx, args).expect_const()); - - if can_elide_generic_arg(args.rebind(ct), default) { + let default = default.instantiate(cx.tcx, args.as_ref()).expect_const(); + if can_elide_generic_arg(ct, ct.rebind(default)) { return None; } elision_has_failed_once_before = true; } - Some(GenericArg::Const(Box::new(clean_middle_const(args.rebind(ct), cx)))) + Some(GenericArg::Const(Box::new(clean_middle_const(ct, cx)))) } }; - clean_args.extend(args.skip_binder().iter().enumerate().rev().filter_map(clean_arg)); + clean_args.extend(args.iter().enumerate().rev().filter_map(clean_arg)); clean_args.reverse(); clean_args } From aa1a5e3b6f0cc1711937b5ee29ac22eedb75c797 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Tue, 9 Jan 2024 14:18:41 +0100 Subject: [PATCH 5/6] Simplify elision of default generic arguments --- src/librustdoc/clean/utils.rs | 88 ++++++++++++++++------------------- 1 file changed, 41 insertions(+), 47 deletions(-) diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index e957cf1f2f5..0e4b00b0976 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -16,9 +16,10 @@ use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE}; use rustc_metadata::rendered_const; use rustc_middle::mir; +use rustc_middle::ty::TypeVisitableExt; use rustc_middle::ty::{self, GenericArgKind, GenericArgsRef, TyCtxt}; -use rustc_middle::ty::{TypeVisitable, TypeVisitableExt}; use rustc_span::symbol::{kw, sym, Symbol}; +use std::assert_matches::debug_assert_matches; use std::fmt::Write as _; use std::mem; use std::sync::LazyLock as Lazy; @@ -108,57 +109,46 @@ pub(crate) fn clean_middle_generic_args<'tcx>( std::borrow::Cow::from(args) }; - let clean_arg = |(index, arg): (usize, &ty::GenericArg<'tcx>)| match arg.unpack() { - GenericArgKind::Lifetime(lt) => { - Some(GenericArg::Lifetime(clean_middle_region(lt).unwrap_or(Lifetime::elided()))) + let clean_arg = |(index, &arg): (usize, &ty::GenericArg<'tcx>)| { + // Elide the self type. + if has_self && index == 0 { + return None; } - GenericArgKind::Type(_) if has_self && index == 0 => None, - GenericArgKind::Type(ty) => { - let ty = ty::Binder::bind_with_vars(ty, bound_vars); - if !elision_has_failed_once_before - && let Some(default) = generics.param_at(index, cx.tcx).default_value(cx.tcx) - { - let default = default.instantiate(cx.tcx, args.as_ref()).expect_ty(); - if can_elide_generic_arg(ty, ty.rebind(default)) { - return None; - } + // Elide internal host effect args. + let param = generics.param_at(index, cx.tcx); + if param.is_host_effect() { + return None; + } - elision_has_failed_once_before = true; + let arg = ty::Binder::bind_with_vars(arg, bound_vars); + + // Elide arguments that coincide with their default. + if !elision_has_failed_once_before && let Some(default) = param.default_value(cx.tcx) { + let default = default.instantiate(cx.tcx, args.as_ref()); + if can_elide_generic_arg(arg, arg.rebind(default)) { + return None; } + elision_has_failed_once_before = true; + } - Some(GenericArg::Type(clean_middle_ty( - ty, + match arg.skip_binder().unpack() { + GenericArgKind::Lifetime(lt) => { + Some(GenericArg::Lifetime(clean_middle_region(lt).unwrap_or(Lifetime::elided()))) + } + GenericArgKind::Type(ty) => Some(GenericArg::Type(clean_middle_ty( + arg.rebind(ty), cx, None, Some(crate::clean::ContainerTy::Regular { ty: owner, - args: ty.rebind(args.as_ref()), + args: arg.rebind(args.as_ref()), arg: index, }), - ))) - } - GenericArgKind::Const(ct) => { - if let ty::GenericParamDefKind::Const { is_host_effect: true, .. } = - generics.param_at(index, cx.tcx).kind - { - return None; + ))), + GenericArgKind::Const(ct) => { + Some(GenericArg::Const(Box::new(clean_middle_const(arg.rebind(ct), cx)))) } - - let ct = ty::Binder::bind_with_vars(ct, bound_vars); - - if !elision_has_failed_once_before - && let Some(default) = generics.param_at(index, cx.tcx).default_value(cx.tcx) - { - let default = default.instantiate(cx.tcx, args.as_ref()).expect_const(); - if can_elide_generic_arg(ct, ct.rebind(default)) { - return None; - } - - elision_has_failed_once_before = true; - } - - Some(GenericArg::Const(Box::new(clean_middle_const(ct, cx)))) } }; @@ -172,13 +162,17 @@ pub(crate) fn clean_middle_generic_args<'tcx>( /// This uses a very conservative approach for performance and correctness reasons, meaning for /// several classes of terms it claims that they cannot be elided even if they theoretically could. /// This is absolutely fine since it mostly concerns edge cases. -fn can_elide_generic_arg<'tcx, Term>( - actual: ty::Binder<'tcx, Term>, - default: ty::Binder<'tcx, Term>, -) -> bool -where - Term: Eq + TypeVisitable>, -{ +fn can_elide_generic_arg<'tcx>( + actual: ty::Binder<'tcx, ty::GenericArg<'tcx>>, + default: ty::Binder<'tcx, ty::GenericArg<'tcx>>, +) -> bool { + debug_assert_matches!( + (actual.skip_binder().unpack(), default.skip_binder().unpack()), + (ty::GenericArgKind::Lifetime(_), ty::GenericArgKind::Lifetime(_)) + | (ty::GenericArgKind::Type(_), ty::GenericArgKind::Type(_)) + | (ty::GenericArgKind::Const(_), ty::GenericArgKind::Const(_)) + ); + // In practice, we shouldn't have any inference variables at this point. // However to be safe, we bail out if we do happen to stumble upon them. if actual.has_infer() || default.has_infer() { From 2d010bc634ea1c8ad0005db2a5c9791fdc5c2454 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Tue, 9 Jan 2024 14:28:35 +0100 Subject: [PATCH 6/6] Move variables closer to their usage sites --- src/librustdoc/clean/utils.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index 0e4b00b0976..437517598ac 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -88,16 +88,11 @@ pub(crate) fn clean_middle_generic_args<'tcx>( return Vec::new(); } - let generics = cx.tcx.generics_of(owner); - let mut elision_has_failed_once_before = false; - - let offset = if has_self { 1 } else { 0 }; - let mut clean_args = Vec::with_capacity(args.len().saturating_sub(offset)); - // If the container is a trait object type, the arguments won't contain the self type but the // generics of the corresponding trait will. In such a case, prepend a dummy self type in order // to align the arguments and parameters for the iteration below and to enable us to correctly // instantiate the generic parameter default later. + let generics = cx.tcx.generics_of(owner); let args = if !has_self && generics.parent.is_none() && generics.has_self { has_self = true; [cx.tcx.types.trait_object_dummy_self.into()] @@ -109,6 +104,7 @@ pub(crate) fn clean_middle_generic_args<'tcx>( std::borrow::Cow::from(args) }; + let mut elision_has_failed_once_before = false; let clean_arg = |(index, &arg): (usize, &ty::GenericArg<'tcx>)| { // Elide the self type. if has_self && index == 0 { @@ -152,6 +148,8 @@ pub(crate) fn clean_middle_generic_args<'tcx>( } }; + let offset = if has_self { 1 } else { 0 }; + let mut clean_args = Vec::with_capacity(args.len().saturating_sub(offset)); clean_args.extend(args.iter().enumerate().rev().filter_map(clean_arg)); clean_args.reverse(); clean_args