From a786eaac1f0272417fcc023516d09393e109a7ea Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 3 Jan 2021 15:38:46 -0500 Subject: [PATCH 1/2] Simplify rustdoc handling of type aliases for associated types The logic was very hard to follow before. --- src/librustdoc/clean/inline.rs | 20 +++----------------- src/librustdoc/clean/mod.rs | 22 +++++++++++----------- src/librustdoc/clean/types.rs | 4 ++++ src/librustdoc/html/render/mod.rs | 3 +++ src/test/rustdoc/deref-typedef.rs | 19 ++++++++++++++++--- 5 files changed, 37 insertions(+), 31 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index c168c56d30d0d..810aca3e15c70 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -261,26 +261,12 @@ fn build_union(cx: &DocContext<'_>, did: DefId) -> clean::Union { fn build_type_alias(cx: &DocContext<'_>, did: DefId) -> clean::Typedef { let predicates = cx.tcx.explicit_predicates_of(did); + let type_ = cx.tcx.type_of(did).clean(cx); clean::Typedef { - type_: cx.tcx.type_of(did).clean(cx), + type_: type_.clone(), generics: (cx.tcx.generics_of(did), predicates).clean(cx), - item_type: build_type_alias_type(cx, did), - } -} - -fn build_type_alias_type(cx: &DocContext<'_>, did: DefId) -> Option { - let type_ = cx.tcx.type_of(did).clean(cx); - type_.def_id().and_then(|did| build_ty(cx, did)) -} - -crate fn build_ty(cx: &DocContext<'_>, did: DefId) -> Option { - match cx.tcx.def_kind(did) { - DefKind::Struct | DefKind::Union | DefKind::Enum | DefKind::Const | DefKind::Static => { - Some(cx.tcx.type_of(did).clean(cx)) - } - DefKind::TyAlias => build_type_alias_type(cx, did), - _ => None, + item_type: Some(type_), } } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index f4eb1924e6f7e..1029cb083603c 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1118,10 +1118,10 @@ impl Clean for hir::ImplItem<'_> { } MethodItem(m, Some(self.defaultness)) } - hir::ImplItemKind::TyAlias(ref ty) => { - let type_ = ty.clean(cx); - let item_type = type_.def_id().and_then(|did| inline::build_ty(cx, did)); - TypedefItem(Typedef { type_, generics: Generics::default(), item_type }, true) + hir::ImplItemKind::TyAlias(ref hir_ty) => { + let type_ = hir_ty.clean(cx); + let item_type = hir_ty_to_ty(cx.tcx, hir_ty).clean(cx); + TypedefItem(Typedef { type_, generics: Generics::default(), item_type: Some(item_type) }, true) } }; Item::from_def_id_and_parts(local_did, Some(self.ident.name), inner, cx) @@ -1267,13 +1267,13 @@ impl Clean for ty::AssocItem { AssocTypeItem(bounds, ty.clean(cx)) } else { + // FIXME: when could this happen? ASsociated items in inherent impls? let type_ = cx.tcx.type_of(self.def_id).clean(cx); - let item_type = type_.def_id().and_then(|did| inline::build_ty(cx, did)); TypedefItem( Typedef { - type_, + type_: type_.clone(), generics: Generics { params: Vec::new(), where_predicates: Vec::new() }, - item_type, + item_type: Some(type_), }, true, ) @@ -1986,11 +1986,11 @@ impl Clean> for (&hir::Item<'_>, Option) { bounds: ty.bounds.clean(cx), generics: ty.generics.clean(cx), }), - ItemKind::TyAlias(ty, ref generics) => { - let rustdoc_ty = ty.clean(cx); - let item_type = rustdoc_ty.def_id().and_then(|did| inline::build_ty(cx, did)); + ItemKind::TyAlias(hir_ty, ref generics) => { + let rustdoc_ty = hir_ty.clean(cx); + let ty = hir_ty_to_ty(cx.tcx, hir_ty); TypedefItem( - Typedef { type_: rustdoc_ty, generics: generics.clean(cx), item_type }, + Typedef { type_: rustdoc_ty, generics: generics.clean(cx), item_type: Some(ty.clean(cx)) }, false, ) } diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 0d33bc9afd5ec..be07444275e6a 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -334,6 +334,10 @@ crate enum ItemKind { ProcMacroItem(ProcMacro), PrimitiveItem(PrimitiveType), AssocConstItem(Type, Option), + /// An associated item in a trait or trait impl. + /// + /// The bounds may be non-empty if there is a `where` clause. + /// The `Option` is the default concrete type (e.g. `trait Trait { type Target = usize; }`) AssocTypeItem(Vec, Option), /// An item that has been stripped by a rustdoc pass StrippedItem(Box), diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index c19262b72cf72..0430caa75bd29 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -4308,6 +4308,7 @@ fn sidebar_assoc_items(it: &clean::Item) -> String { .filter(|i| i.inner_impl().trait_.is_some()) .find(|i| i.inner_impl().trait_.def_id() == c.deref_trait_did) { + debug!("found Deref: {:?}", impl_); if let Some((target, real_target)) = impl_.inner_impl().items.iter().find_map(|item| match *item.kind { clean::TypedefItem(ref t, true) => Some(match *t { @@ -4317,6 +4318,7 @@ fn sidebar_assoc_items(it: &clean::Item) -> String { _ => None, }) { + debug!("found target, real_target: {:?} {:?}", target, real_target); let deref_mut = v .iter() .filter(|i| i.inner_impl().trait_.is_some()) @@ -4328,6 +4330,7 @@ fn sidebar_assoc_items(it: &clean::Item) -> String { .and_then(|prim| c.primitive_locations.get(&prim).cloned())) .and_then(|did| c.impls.get(&did)); if let Some(impls) = inner_impl { + debug!("found inner_impl: {:?}", impls); out.push_str(""); out.push_str(&format!( "Methods from {}<Target={}>", diff --git a/src/test/rustdoc/deref-typedef.rs b/src/test/rustdoc/deref-typedef.rs index 770f8d7289c3b..e2dac2cf417da 100644 --- a/src/test/rustdoc/deref-typedef.rs +++ b/src/test/rustdoc/deref-typedef.rs @@ -1,18 +1,27 @@ #![crate_name = "foo"] // @has 'foo/struct.Bar.html' -// @has '-' '//*[@id="deref-methods"]' 'Methods from Deref' +// @has '-' '//*[@id="deref-methods"]' 'Methods from Deref' // @has '-' '//*[@class="impl-items"]//*[@id="method.foo_a"]' 'pub fn foo_a(&self)' // @has '-' '//*[@class="impl-items"]//*[@id="method.foo_b"]' 'pub fn foo_b(&self)' // @has '-' '//*[@class="impl-items"]//*[@id="method.foo_c"]' 'pub fn foo_c(&self)' -// @has '-' '//*[@class="sidebar-title"]' 'Methods from Deref' +// @has '-' '//*[@class="impl-items"]//*[@id="method.foo_j"]' 'pub fn foo_j(&self)' +// @has '-' '//*[@class="sidebar-title"]' 'Methods from Deref' // @has '-' '//*[@class="sidebar-links"]/a[@href="#method.foo_a"]' 'foo_a' // @has '-' '//*[@class="sidebar-links"]/a[@href="#method.foo_b"]' 'foo_b' // @has '-' '//*[@class="sidebar-links"]/a[@href="#method.foo_c"]' 'foo_c' +// @has '-' '//*[@class="sidebar-links"]/a[@href="#method.foo_j"]' 'foo_j' pub struct FooA; pub type FooB = FooA; pub type FooC = FooB; +pub type FooD = FooC; +pub type FooE = FooD; +pub type FooF = FooE; +pub type FooG = FooF; +pub type FooH = FooG; +pub type FooI = FooH; +pub type FooJ = FooI; impl FooA { pub fn foo_a(&self) {} @@ -26,8 +35,12 @@ impl FooC { pub fn foo_c(&self) {} } +impl FooJ { + pub fn foo_j(&self) {} +} + pub struct Bar; impl std::ops::Deref for Bar { - type Target = FooC; + type Target = FooJ; fn deref(&self) -> &Self::Target { unimplemented!() } } From 24ef94593c84084be03e6461b702178523767cf7 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 3 Jan 2021 16:08:21 -0500 Subject: [PATCH 2/2] Don't clone `type_` unnecessarily --- src/librustdoc/clean/inline.rs | 4 ++-- src/librustdoc/clean/mod.rs | 21 ++++++++++++++++----- src/librustdoc/clean/types.rs | 7 ++++++- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 810aca3e15c70..ed972cc16e954 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -264,9 +264,9 @@ fn build_type_alias(cx: &DocContext<'_>, did: DefId) -> clean::Typedef { let type_ = cx.tcx.type_of(did).clean(cx); clean::Typedef { - type_: type_.clone(), + type_, generics: (cx.tcx.generics_of(did), predicates).clean(cx), - item_type: Some(type_), + item_type: None, } } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 1029cb083603c..227c62537a023 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1121,7 +1121,14 @@ impl Clean for hir::ImplItem<'_> { hir::ImplItemKind::TyAlias(ref hir_ty) => { let type_ = hir_ty.clean(cx); let item_type = hir_ty_to_ty(cx.tcx, hir_ty).clean(cx); - TypedefItem(Typedef { type_, generics: Generics::default(), item_type: Some(item_type) }, true) + TypedefItem( + Typedef { + type_, + generics: Generics::default(), + item_type: Some(item_type), + }, + true, + ) } }; Item::from_def_id_and_parts(local_did, Some(self.ident.name), inner, cx) @@ -1271,9 +1278,9 @@ impl Clean for ty::AssocItem { let type_ = cx.tcx.type_of(self.def_id).clean(cx); TypedefItem( Typedef { - type_: type_.clone(), + type_, generics: Generics { params: Vec::new(), where_predicates: Vec::new() }, - item_type: Some(type_), + item_type: None, }, true, ) @@ -1988,9 +1995,13 @@ impl Clean> for (&hir::Item<'_>, Option) { }), ItemKind::TyAlias(hir_ty, ref generics) => { let rustdoc_ty = hir_ty.clean(cx); - let ty = hir_ty_to_ty(cx.tcx, hir_ty); + let ty = hir_ty_to_ty(cx.tcx, hir_ty).clean(cx); TypedefItem( - Typedef { type_: rustdoc_ty, generics: generics.clean(cx), item_type: Some(ty.clean(cx)) }, + Typedef { + type_: rustdoc_ty, + generics: generics.clean(cx), + item_type: Some(ty), + }, false, ) } diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index be07444275e6a..be63a9c9ab7ce 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1732,7 +1732,12 @@ crate struct PathSegment { crate struct Typedef { crate type_: Type, crate generics: Generics, - // Type of target item. + /// `type_` can come from either the HIR or from metadata. If it comes from HIR, it may be a type + /// alias instead of the final type. This will always have the final type, regardless of whether + /// `type_` came from HIR or from metadata. + /// + /// If `item_type.is_none()`, `type_` is guarenteed to come from metadata (and therefore hold the + /// final type). crate item_type: Option, }