Skip to content

Commit

Permalink
Rollup merge of #95316 - fmease:rustdoc-discr-req-prov-assoc-consts-t…
Browse files Browse the repository at this point in the history
…ys, r=notriddle,GuillaumeGomez

Rustdoc: Discriminate required and provided associated constants and types

Currently, rustdoc merely separates required and provided associated _functions_ (i.e. methods). This PR extends this to constants (fixes #94652) and types. This makes the documentation of all three kinds of associated items more alike and consistent.

As an aside, associated types may actually be provided / have a default when users enable the unstable feature `associated_type_defaults`.

| Before | After |
|---|---|
| ![image](https://user-images.githubusercontent.com/14913065/160631832-d5862d13-b395-4d86-b45c-3873ffd4cd4e.png) | ![image](https://user-images.githubusercontent.com/14913065/160631903-33909a03-b6ee-4d75-9cbc-d188f7f8602e.png) |
| ![image](https://user-images.githubusercontent.com/14913065/160632173-040d4139-76f4-4410-851b-d8c1cef014d2.png) | ![image](https://user-images.githubusercontent.com/14913065/160632233-6fd3fe73-cadc-4291-b104-59d2e45366a6.png) |

### `clean::types::ItemKind` modification

* `ItemKind::TypedefItem(.., true)` → `ItemKind::AssocTypeItem(..)`
* `ItemKind::TypedefItem(.., false)` → `ItemKind::TypedefItem(..)`

Further, I added `ItemKind::TyAssoc{Const,Type}Item`, the “required” variant of `ItemKind::Assoc{Const,Type}Item`, analogous to `ItemKind::TyMethodItem` with `ItemKind::MethodItem`. These new variants don't contain new information really, they are just the result of me getting rid of the `Option<_>` field in `AssocConstItem` and `AssocTypeItem`.

**Goal**: Make associated items more consistent.
Originally I thought modifying `ItemKind` was necessary to achieve the new functionality of this PR but in retrospect, it does not. If you don't like the changes to `ItemKind`, I think I _can_ get rid of them.

This change is the root cause of those tiny changes in a lot of different files.

 ### Concerns and Open Questions

* **breaking changes** to hyperlinks: Some heading IDs change:
  * `associated-const` (sic!) -> `{provided,required}-associated-consts`
  * `associated-types` -> `{provided,required}-associated-types`
* **verbosity** of the headings _{Required,Provided} Associated {Constants,Types}_
* For some files, I am not sure if the changes I made are correct. So please take extra care when reviewing `conversions.rs` (conversion to JSON), `cache.rs`/`fold_item`, `stripper.rs`/`fold_item`, `check_doc_test_visibility.rs`/`should_have_doc_example`, `collect_intra_doc_links.rs`/`from_assoc_item`
* JSON output: I still map `AssocTypeItem`s to `Typedef` etc. (FIXME)
  • Loading branch information
Dylan-DPC authored Apr 12, 2022
2 parents 52ca603 + 8de453a commit 0ec00c0
Show file tree
Hide file tree
Showing 22 changed files with 401 additions and 243 deletions.
2 changes: 1 addition & 1 deletion src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ crate fn try_inline(
Res::Def(DefKind::TyAlias, did) => {
record_extern_fqn(cx, did, ItemType::Typedef);
build_impls(cx, Some(parent_module), did, attrs, &mut ret);
clean::TypedefItem(build_type_alias(cx, did), false)
clean::TypedefItem(build_type_alias(cx, did))
}
Res::Def(DefKind::Enum, did) => {
record_extern_fqn(cx, did, ItemType::Enum);
Expand Down
85 changes: 51 additions & 34 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -964,11 +964,11 @@ impl Clean<Item> for hir::TraitItem<'_> {
let local_did = self.def_id.to_def_id();
cx.with_param_env(local_did, |cx| {
let inner = match self.kind {
hir::TraitItemKind::Const(ref ty, default) => {
let default =
default.map(|e| ConstantKind::Local { def_id: local_did, body: e });
AssocConstItem(ty.clean(cx), default)
}
hir::TraitItemKind::Const(ref ty, Some(default)) => AssocConstItem(
ty.clean(cx),
ConstantKind::Local { def_id: local_did, body: default },
),
hir::TraitItemKind::Const(ref ty, None) => TyAssocConstItem(ty.clean(cx)),
hir::TraitItemKind::Fn(ref sig, hir::TraitFn::Provided(body)) => {
let m = clean_function(cx, sig, &self.generics, body);
MethodItem(m, None)
Expand All @@ -983,11 +983,19 @@ impl Clean<Item> for hir::TraitItem<'_> {
});
TyMethodItem(Function { decl, generics })
}
hir::TraitItemKind::Type(bounds, ref default) => {
hir::TraitItemKind::Type(bounds, Some(default)) => {
let generics = enter_impl_trait(cx, |cx| self.generics.clean(cx));
let bounds = bounds.iter().filter_map(|x| x.clean(cx)).collect();
let item_type = hir_ty_to_ty(cx.tcx, default).clean(cx);
AssocTypeItem(
Typedef { type_: default.clean(cx), generics, item_type: Some(item_type) },
bounds,
)
}
hir::TraitItemKind::Type(bounds, None) => {
let generics = enter_impl_trait(cx, |cx| self.generics.clean(cx));
let bounds = bounds.iter().filter_map(|x| x.clean(cx)).collect();
let default = default.map(|t| t.clean(cx));
AssocTypeItem(Box::new(generics), bounds, default)
TyAssocTypeItem(Box::new(generics), bounds)
}
};
let what_rustc_thinks =
Expand All @@ -1004,7 +1012,7 @@ impl Clean<Item> for hir::ImplItem<'_> {
cx.with_param_env(local_did, |cx| {
let inner = match self.kind {
hir::ImplItemKind::Const(ref ty, expr) => {
let default = Some(ConstantKind::Local { def_id: local_did, body: expr });
let default = ConstantKind::Local { def_id: local_did, body: expr };
AssocConstItem(ty.clean(cx), default)
}
hir::ImplItemKind::Fn(ref sig, body) => {
Expand All @@ -1016,7 +1024,10 @@ impl Clean<Item> for hir::ImplItem<'_> {
let type_ = hir_ty.clean(cx);
let generics = self.generics.clean(cx);
let item_type = hir_ty_to_ty(cx.tcx, hir_ty).clean(cx);
TypedefItem(Typedef { type_, generics, item_type: Some(item_type) }, true)
AssocTypeItem(
Typedef { type_, generics, item_type: Some(item_type) },
Vec::new(),
)
}
};

Expand All @@ -1041,13 +1052,17 @@ impl Clean<Item> for ty::AssocItem {
let tcx = cx.tcx;
let kind = match self.kind {
ty::AssocKind::Const => {
let ty = tcx.type_of(self.def_id);
let default = if self.defaultness.has_value() {
Some(ConstantKind::Extern { def_id: self.def_id })
} else {
None
let ty = tcx.type_of(self.def_id).clean(cx);

let provided = match self.container {
ty::ImplContainer(_) => true,
ty::TraitContainer(_) => self.defaultness.has_value(),
};
AssocConstItem(ty.clean(cx), default)
if provided {
AssocConstItem(ty, ConstantKind::Extern { def_id: self.def_id })
} else {
TyAssocConstItem(ty)
}
}
ty::AssocKind::Fn => {
let generics = clean_ty_generics(
Expand Down Expand Up @@ -1181,23 +1196,28 @@ impl Clean<Item> for ty::AssocItem {
None => bounds.push(GenericBound::maybe_sized(cx)),
}

let ty = if self.defaultness.has_value() {
Some(tcx.type_of(self.def_id))
if self.defaultness.has_value() {
AssocTypeItem(
Typedef {
type_: tcx.type_of(self.def_id).clean(cx),
generics,
// FIXME: should we obtain the Type from HIR and pass it on here?
item_type: None,
},
bounds,
)
} else {
None
};

AssocTypeItem(Box::new(generics), bounds, ty.map(|t| t.clean(cx)))
TyAssocTypeItem(Box::new(generics), bounds)
}
} else {
// FIXME: when could this happen? Associated items in inherent impls?
let type_ = tcx.type_of(self.def_id).clean(cx);
TypedefItem(
AssocTypeItem(
Typedef {
type_,
type_: tcx.type_of(self.def_id).clean(cx),
generics: Generics { params: Vec::new(), where_predicates: Vec::new() },
item_type: None,
},
true,
Vec::new(),
)
}
}
Expand Down Expand Up @@ -1837,14 +1857,11 @@ fn clean_maybe_renamed_item(
ItemKind::TyAlias(hir_ty, ref generics) => {
let rustdoc_ty = hir_ty.clean(cx);
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),
},
false,
)
TypedefItem(Typedef {
type_: rustdoc_ty,
generics: generics.clean(cx),
item_type: Some(ty),
})
}
ItemKind::Enum(ref def, ref generics) => EnumItem(Enum {
variants: def.variants.iter().map(|v| v.clean(cx)).collect(),
Expand Down
35 changes: 24 additions & 11 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -577,10 +577,16 @@ impl Item {
self.type_() == ItemType::Variant
}
crate fn is_associated_type(&self) -> bool {
self.type_() == ItemType::AssocType
matches!(&*self.kind, AssocTypeItem(..) | StrippedItem(box AssocTypeItem(..)))
}
crate fn is_ty_associated_type(&self) -> bool {
matches!(&*self.kind, TyAssocTypeItem(..) | StrippedItem(box TyAssocTypeItem(..)))
}
crate fn is_associated_const(&self) -> bool {
self.type_() == ItemType::AssocConst
matches!(&*self.kind, AssocConstItem(..) | StrippedItem(box AssocConstItem(..)))
}
crate fn is_ty_associated_const(&self) -> bool {
matches!(&*self.kind, TyAssocConstItem(..) | StrippedItem(box TyAssocConstItem(..)))
}
crate fn is_method(&self) -> bool {
self.type_() == ItemType::Method
Expand Down Expand Up @@ -726,17 +732,18 @@ crate enum ItemKind {
EnumItem(Enum),
FunctionItem(Function),
ModuleItem(Module),
TypedefItem(Typedef, bool /* is associated type */),
TypedefItem(Typedef),
OpaqueTyItem(OpaqueTy),
StaticItem(Static),
ConstantItem(Constant),
TraitItem(Trait),
TraitAliasItem(TraitAlias),
ImplItem(Impl),
/// A method signature only. Used for required methods in traits (ie,
/// non-default-methods).
/// A required method in a trait declaration meaning it's only a function signature.
TyMethodItem(Function),
/// A method with a body.
/// A method in a trait impl or a provided method in a trait declaration.
///
/// Compared to [TyMethodItem], it also contains a method body.
MethodItem(Function, Option<hir::Defaultness>),
StructFieldItem(Type),
VariantItem(Variant),
Expand All @@ -749,12 +756,16 @@ crate enum ItemKind {
MacroItem(Macro),
ProcMacroItem(ProcMacro),
PrimitiveItem(PrimitiveType),
AssocConstItem(Type, Option<ConstantKind>),
/// An associated item in a trait or trait impl.
/// A required associated constant in a trait declaration.
TyAssocConstItem(Type),
/// An associated associated constant in a trait impl or a provided one in a trait declaration.
AssocConstItem(Type, ConstantKind),
/// A required associated type in a trait declaration.
///
/// The bounds may be non-empty if there is a `where` clause.
/// The `Option<Type>` is the default concrete type (e.g. `trait Trait { type Target = usize; }`)
AssocTypeItem(Box<Generics>, Vec<GenericBound>, Option<Type>),
TyAssocTypeItem(Box<Generics>, Vec<GenericBound>),
/// An associated type in a trait impl or a provided one in a trait declaration.
AssocTypeItem(Typedef, Vec<GenericBound>),
/// An item that has been stripped by a rustdoc pass
StrippedItem(Box<ItemKind>),
KeywordItem(Symbol),
Expand All @@ -776,7 +787,7 @@ impl ItemKind {
ExternCrateItem { .. }
| ImportItem(_)
| FunctionItem(_)
| TypedefItem(_, _)
| TypedefItem(_)
| OpaqueTyItem(_)
| StaticItem(_)
| ConstantItem(_)
Expand All @@ -791,7 +802,9 @@ impl ItemKind {
| MacroItem(_)
| ProcMacroItem(_)
| PrimitiveItem(_)
| TyAssocConstItem(_)
| AssocConstItem(_, _)
| TyAssocTypeItem(..)
| AssocTypeItem(..)
| StrippedItem(_)
| KeywordItem(_) => [].iter(),
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ crate fn build_deref_target_impls(cx: &mut DocContext<'_>, items: &[Item], ret:

for item in items {
let target = match *item.kind {
ItemKind::TypedefItem(ref t, true) => &t.type_,
ItemKind::AssocTypeItem(ref t, _) => &t.type_,
_ => continue,
};

Expand Down
6 changes: 4 additions & 2 deletions src/librustdoc/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ crate trait DocFolder: Sized {
ExternCrateItem { src: _ }
| ImportItem(_)
| FunctionItem(_)
| TypedefItem(_, _)
| TypedefItem(_)
| OpaqueTyItem(_)
| StaticItem(_)
| ConstantItem(_)
Expand All @@ -85,7 +85,9 @@ crate trait DocFolder: Sized {
| MacroItem(_)
| ProcMacroItem(_)
| PrimitiveItem(_)
| AssocConstItem(_, _)
| TyAssocConstItem(..)
| AssocConstItem(..)
| TyAssocTypeItem(..)
| AssocTypeItem(..)
| KeywordItem(_) => kind,
}
Expand Down
11 changes: 7 additions & 4 deletions src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,14 +242,15 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
if let Some(ref s) = item.name {
let (parent, is_inherent_impl_item) = match *item.kind {
clean::StrippedItem(..) => ((None, None), false),
clean::AssocConstItem(..) | clean::TypedefItem(_, true)
clean::AssocConstItem(..) | clean::AssocTypeItem(..)
if self.cache.parent_is_trait_impl =>
{
// skip associated items in trait impls
((None, None), false)
}
clean::AssocTypeItem(..)
| clean::TyMethodItem(..)
clean::TyMethodItem(..)
| clean::TyAssocConstItem(..)
| clean::TyAssocTypeItem(..)
| clean::StructFieldItem(..)
| clean::VariantItem(..) => (
(
Expand All @@ -258,7 +259,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
),
false,
),
clean::MethodItem(..) | clean::AssocConstItem(..) => {
clean::MethodItem(..) | clean::AssocConstItem(..) | clean::AssocTypeItem(..) => {
if self.cache.parent_stack.is_empty() {
((None, None), false)
} else {
Expand Down Expand Up @@ -373,7 +374,9 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
| clean::TyMethodItem(..)
| clean::MethodItem(..)
| clean::StructFieldItem(..)
| clean::TyAssocConstItem(..)
| clean::AssocConstItem(..)
| clean::TyAssocTypeItem(..)
| clean::AssocTypeItem(..)
| clean::StrippedItem(..)
| clean::KeywordItem(..) => {
Expand Down
6 changes: 3 additions & 3 deletions src/librustdoc/formats/item_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::clean;
/// The search index uses item types encoded as smaller numbers which equal to
/// discriminants. JavaScript then is used to decode them into the original value.
/// Consequently, every change to this type should be synchronized to
/// the `itemTypes` mapping table in `html/static/main.js`.
/// the `itemTypes` mapping table in `html/static/js/search.js`.
///
/// In addition, code in `html::render` uses this enum to generate CSS classes, page prefixes, and
/// module headings. If you are adding to this enum and want to ensure that the sidebar also prints
Expand Down Expand Up @@ -89,8 +89,8 @@ impl<'a> From<&'a clean::Item> for ItemType {
clean::ForeignStaticItem(..) => ItemType::Static, // no ForeignStatic
clean::MacroItem(..) => ItemType::Macro,
clean::PrimitiveItem(..) => ItemType::Primitive,
clean::AssocConstItem(..) => ItemType::AssocConst,
clean::AssocTypeItem(..) => ItemType::AssocType,
clean::TyAssocConstItem(..) | clean::AssocConstItem(..) => ItemType::AssocConst,
clean::TyAssocTypeItem(..) | clean::AssocTypeItem(..) => ItemType::AssocType,
clean::ForeignTypeItem => ItemType::ForeignType,
clean::KeywordItem(..) => ItemType::Keyword,
clean::TraitAliasItem(..) => ItemType::TraitAlias,
Expand Down
15 changes: 15 additions & 0 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,21 @@ crate enum HrefError {
/// This item is known to rustdoc, but from a crate that does not have documentation generated.
///
/// This can only happen for non-local items.
///
/// # Example
///
/// Crate `a` defines a public trait and crate `b` – the target crate that depends on `a` –
/// implements it for a local type.
/// We document `b` but **not** `a` (we only _build_ the latter – with `rustc`):
///
/// ```sh
/// rustc a.rs --crate-type=lib
/// rustdoc b.rs --crate-type=lib --extern=a=liba.rlib
/// ```
///
/// Now, the associated items in the trait impl want to link to the corresponding item in the
/// trait declaration (see `html::render::assoc_href_attr`) but it's not available since their
/// *documentation (was) not built*.
DocumentationNotBuilt,
/// This can only happen for non-local items when `--document-private-items` is not passed.
Private,
Expand Down
6 changes: 4 additions & 2 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1452,8 +1452,10 @@ fn init_id_map() -> FxHashMap<String, usize> {
map.insert("trait-implementations".to_owned(), 1);
map.insert("synthetic-implementations".to_owned(), 1);
map.insert("blanket-implementations".to_owned(), 1);
map.insert("associated-types".to_owned(), 1);
map.insert("associated-const".to_owned(), 1);
map.insert("required-associated-types".to_owned(), 1);
map.insert("provided-associated-types".to_owned(), 1);
map.insert("provided-associated-consts".to_owned(), 1);
map.insert("required-associated-consts".to_owned(), 1);
map.insert("required-methods".to_owned(), 1);
map.insert("provided-methods".to_owned(), 1);
map.insert("implementors".to_owned(), 1);
Expand Down
Loading

0 comments on commit 0ec00c0

Please sign in to comment.