Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup more of rustdoc #79372

Merged
merged 4 commits into from
Nov 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ crate fn try_inline(
let attrs = merge_attrs(cx, Some(parent_module), target_attrs, attrs_clone);

cx.renderinfo.borrow_mut().inlined.insert(did);
let what_rustc_thinks = clean::Item::from_def_id_and_parts(did, Some(name), kind, cx);
let what_rustc_thinks = clean::Item::from_def_id_and_parts(did, Some(name.clean(cx)), kind, cx);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't look deep but I wonder why do we need to have this .clean(cx) so many times unlike before?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the explanation in the commit message help? bbded0b

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, now I understand. I didn't know it is related to interning.

ret.push(clean::Item { attrs, ..what_rustc_thinks });
Some(ret)
}
Expand Down
42 changes: 20 additions & 22 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ impl Clean<ExternalCrate> for CrateNum {
}
}
}
return prim.map(|p| (def_id, p, attrs));
return prim.map(|p| (def_id, p));
}
None
};
Expand All @@ -144,9 +144,9 @@ impl Clean<ExternalCrate> for CrateNum {
hir::ItemKind::Use(ref path, hir::UseKind::Single)
if item.vis.node.is_pub() =>
{
as_primitive(path.res).map(|(_, prim, attrs)| {
as_primitive(path.res).map(|(_, prim)| {
// Pretend the primitive is local.
(cx.tcx.hir().local_def_id(id.id).to_def_id(), prim, attrs)
(cx.tcx.hir().local_def_id(id.id).to_def_id(), prim)
})
}
_ => None,
Expand Down Expand Up @@ -177,7 +177,7 @@ impl Clean<ExternalCrate> for CrateNum {
}
}
}
return keyword.map(|p| (def_id, p, attrs));
return keyword.map(|p| (def_id, p));
}
None
};
Expand All @@ -199,8 +199,8 @@ impl Clean<ExternalCrate> for CrateNum {
hir::ItemKind::Use(ref path, hir::UseKind::Single)
if item.vis.node.is_pub() =>
{
as_keyword(path.res).map(|(_, prim, attrs)| {
(cx.tcx.hir().local_def_id(id.id).to_def_id(), prim, attrs)
as_keyword(path.res).map(|(_, prim)| {
(cx.tcx.hir().local_def_id(id.id).to_def_id(), prim)
})
}
_ => None,
Expand Down Expand Up @@ -1099,7 +1099,7 @@ impl Clean<Item> for hir::TraitItem<'_> {
AssocTypeItem(bounds.clean(cx), default.clean(cx))
}
};
Item::from_def_id_and_parts(local_did, Some(self.ident.name), inner, cx)
Item::from_def_id_and_parts(local_did, Some(self.ident.name.clean(cx)), inner, cx)
})
}
}
Expand Down Expand Up @@ -1127,7 +1127,7 @@ impl Clean<Item> for hir::ImplItem<'_> {
TypedefItem(Typedef { type_, generics: Generics::default(), item_type }, true)
}
};
Item::from_def_id_and_parts(local_did, Some(self.ident.name), inner, cx)
Item::from_def_id_and_parts(local_did, Some(self.ident.name.clean(cx)), inner, cx)
})
}
}
Expand Down Expand Up @@ -1284,7 +1284,7 @@ impl Clean<Item> for ty::AssocItem {
}
};

Item::from_def_id_and_parts(self.def_id, Some(self.ident.name), kind, cx)
Item::from_def_id_and_parts(self.def_id, Some(self.ident.name.clean(cx)), kind, cx)
}
}

Expand Down Expand Up @@ -1769,7 +1769,7 @@ impl Clean<Item> for ty::FieldDef {
fn clean(&self, cx: &DocContext<'_>) -> Item {
let what_rustc_thinks = Item::from_def_id_and_parts(
self.did,
Some(self.ident.name),
Some(self.ident.name.clean(cx)),
StructFieldItem(cx.tcx.type_of(self.did).clean(cx)),
cx,
);
Expand Down Expand Up @@ -1844,22 +1844,20 @@ impl Clean<Item> for ty::VariantDef {
fields: self
.fields
.iter()
.map(|field| Item {
source: cx.tcx.def_span(field.did).clean(cx),
name: Some(field.ident.name.clean(cx)),
attrs: cx.tcx.get_attrs(field.did).clean(cx),
visibility: Visibility::Inherited,
def_id: field.did,
stability: get_stability(cx, field.did),
deprecation: get_deprecation(cx, field.did),
kind: StructFieldItem(cx.tcx.type_of(field.did).clean(cx)),
.map(|field| {
let name = Some(field.ident.name.clean(cx));
let kind = StructFieldItem(cx.tcx.type_of(field.did).clean(cx));
let what_rustc_thinks =
Item::from_def_id_and_parts(field.did, name, kind, cx);
// don't show `pub` for fields, which are always public
Item { visibility: Visibility::Inherited, ..what_rustc_thinks }
})
.collect(),
}),
};
let what_rustc_thinks = Item::from_def_id_and_parts(
self.def_id,
Some(self.ident.name),
Some(self.ident.name.clean(cx)),
VariantItem(Variant { kind }),
cx,
);
Expand Down Expand Up @@ -2057,7 +2055,7 @@ impl Clean<Vec<Item>> for (&hir::Item<'_>, Option<Ident>) {
_ => unreachable!("not yet converted"),
};

vec![Item::from_def_id_and_parts(def_id, Some(name), kind, cx)]
vec![Item::from_def_id_and_parts(def_id, Some(name.clean(cx)), kind, cx)]
})
}
}
Expand Down Expand Up @@ -2319,7 +2317,7 @@ impl Clean<Item> for doctree::Macro {
fn clean(&self, cx: &DocContext<'_>) -> Item {
Item::from_def_id_and_parts(
self.def_id,
Some(self.name),
Some(self.name.clean(cx)),
MacroItem(Macro {
source: format!(
"macro_rules! {} {{\n{}}}",
Expand Down
20 changes: 12 additions & 8 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use crate::clean::cfg::Cfg;
use crate::clean::external_path;
use crate::clean::inline;
use crate::clean::types::Type::{QPath, ResolvedPath};
use crate::clean::Clean;
use crate::core::DocContext;
use crate::doctree;
use crate::formats::cache::cache;
Expand All @@ -54,7 +55,7 @@ crate struct Crate {
crate src: FileName,
crate module: Option<Item>,
crate externs: Vec<(CrateNum, ExternalCrate)>,
crate primitives: Vec<(DefId, PrimitiveType, Attributes)>,
crate primitives: Vec<(DefId, PrimitiveType)>,
// These are later on moved into `CACHEKEY`, leaving the map empty.
// Only here so that they can be filtered through the rustdoc passes.
crate external_traits: Rc<RefCell<FxHashMap<DefId, Trait>>>,
Expand All @@ -67,8 +68,8 @@ crate struct ExternalCrate {
crate name: String,
crate src: FileName,
crate attrs: Attributes,
crate primitives: Vec<(DefId, PrimitiveType, Attributes)>,
crate keywords: Vec<(DefId, String, Attributes)>,
crate primitives: Vec<(DefId, PrimitiveType)>,
crate keywords: Vec<(DefId, String)>,
}

/// Anything with a source location and set of attributes and, optionally, a
Expand Down Expand Up @@ -120,17 +121,20 @@ impl Item {
kind: ItemKind,
cx: &DocContext<'_>,
) -> Item {
Item::from_def_id_and_parts(cx.tcx.hir().local_def_id(hir_id).to_def_id(), name, kind, cx)
Item::from_def_id_and_parts(
cx.tcx.hir().local_def_id(hir_id).to_def_id(),
name.clean(cx),
kind,
cx,
)
}

pub fn from_def_id_and_parts(
def_id: DefId,
name: Option<Symbol>,
name: Option<String>,
kind: ItemKind,
cx: &DocContext<'_>,
) -> Item {
use super::Clean;

debug!("name={:?}, def_id={:?}", name, def_id);

// `span_if_local()` lies about functions and only gives the span of the function signature
Expand All @@ -145,7 +149,7 @@ impl Item {
Item {
def_id,
kind,
name: name.clean(cx),
name,
source: source.clean(cx),
attrs: cx.tcx.get_attrs(def_id).clean(cx),
visibility: cx.tcx.visibility(def_id).clean(cx),
Expand Down
45 changes: 13 additions & 32 deletions src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
use crate::clean::auto_trait::AutoTraitFinder;
use crate::clean::blanket_impl::BlanketImplFinder;
use crate::clean::{
inline, Clean, Crate, Deprecation, ExternalCrate, FnDecl, FnRetTy, Generic, GenericArg,
GenericArgs, GenericBound, Generics, GetDefId, ImportSource, Item, ItemKind, Lifetime,
MacroKind, Path, PathSegment, Primitive, PrimitiveType, ResolvedPath, Span, Type, TypeBinding,
TypeKind, Visibility, WherePredicate,
inline, Clean, Crate, ExternalCrate, FnDecl, FnRetTy, Generic, GenericArg, GenericArgs,
GenericBound, Generics, GetDefId, ImportSource, Item, ItemKind, Lifetime, MacroKind, Path,
PathSegment, Primitive, PrimitiveType, ResolvedPath, Type, TypeBinding, TypeKind,
WherePredicate,
};
use crate::core::DocContext;

use itertools::Itertools;
use rustc_attr::Stability;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
Expand Down Expand Up @@ -66,25 +65,16 @@ crate fn krate(mut cx: &mut DocContext<'_>) -> Crate {
ItemKind::ModuleItem(ref mut m) => m,
_ => unreachable!(),
};
m.items.extend(primitives.iter().map(|&(def_id, prim, ref attrs)| Item {
source: Span::empty(),
name: Some(prim.to_url_str().to_string()),
attrs: attrs.clone(),
visibility: Visibility::Public,
stability: get_stability(cx, def_id),
deprecation: get_deprecation(cx, def_id),
def_id,
kind: ItemKind::PrimitiveItem(prim),
m.items.extend(primitives.iter().map(|&(def_id, prim)| {
Item::from_def_id_and_parts(
def_id,
Some(prim.to_url_str().to_owned()),
ItemKind::PrimitiveItem(prim),
cx,
)
}));
m.items.extend(keywords.into_iter().map(|(def_id, kw, attrs)| Item {
source: Span::empty(),
name: Some(kw.clone()),
attrs,
visibility: Visibility::Public,
stability: get_stability(cx, def_id),
deprecation: get_deprecation(cx, def_id),
def_id,
kind: ItemKind::KeywordItem(kw),
m.items.extend(keywords.into_iter().map(|(def_id, kw)| {
Item::from_def_id_and_parts(def_id, Some(kw.clone()), ItemKind::KeywordItem(kw), cx)
}));
}

Expand All @@ -101,15 +91,6 @@ crate fn krate(mut cx: &mut DocContext<'_>) -> Crate {
}
}

// extract the stability index for a node from tcx, if possible
crate fn get_stability(cx: &DocContext<'_>, def_id: DefId) -> Option<Stability> {
cx.tcx.lookup_stability(def_id).cloned()
}

crate fn get_deprecation(cx: &DocContext<'_>, def_id: DefId) -> Option<Deprecation> {
cx.tcx.lookup_deprecation(def_id).clean(cx)
}

fn external_generic_args(
cx: &DocContext<'_>,
trait_did: Option<DefId>,
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,11 @@ impl Cache {
// Favor linking to as local extern as possible, so iterate all crates in
// reverse topological order.
for &(_, ref e) in krate.externs.iter().rev() {
for &(def_id, prim, _) in &e.primitives {
for &(def_id, prim) in &e.primitives {
cache.primitive_locations.insert(prim, def_id);
}
}
for &(def_id, prim, _) in &krate.primitives {
for &(def_id, prim) in &krate.primitives {
cache.primitive_locations.insert(prim, def_id);
}

Expand Down
3 changes: 1 addition & 2 deletions src/test/rustdoc-ui/coverage/exotic.stdout
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
+-------------------------------------+------------+------------+------------+------------+
| File | Documented | Percentage | Examples | Percentage |
+-------------------------------------+------------+------------+------------+------------+
| ...st/rustdoc-ui/coverage/exotic.rs | 1 | 100.0% | 0 | 0.0% |
| <anon> | 2 | 100.0% | 0 | 0.0% |
| ...st/rustdoc-ui/coverage/exotic.rs | 3 | 100.0% | 0 | 0.0% |
+-------------------------------------+------------+------------+------------+------------+
| Total | 3 | 100.0% | 0 | 0.0% |
+-------------------------------------+------------+------------+------------+------------+