Auto merge of #79372 - jyn514:more-cleanup, r=GuillaumeGomez

Cleanup more of rustdoc - Use `Item::from_def_id` for StructField - Use `from_def_id_and_parts` for primitives and keywords - Take `String` instead of `Symbol` in `from_def_id` - this avoids having to intern then immediately stringify the existing string. - Remove unused `get_stability` and `get_deprecation` - Remove unused `attrs` field from `primitives` - Remove unused `attrs` field from `keywords` This will probably conflict with #79335 and I would prefer for that PR to land first - I'm anxious for #77467 to land :) Makes #76998 easier to add. r? `@GuillaumeGomez`
rust-lang · Nov 27, 2020 · 6a88957 · 6a88957
2 parents 774bce7 + 09a3bc1
commit 6a88957
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 67 deletions.
diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs
@@ -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);
     ret.push(clean::Item { attrs, ..what_rustc_thinks });
     Some(ret)
 }

diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
@@ -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
         };
@@ -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,
@@ -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
         };
@@ -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,
@@ -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)
         })
     }
 }
@@ -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)
         })
     }
 }
@@ -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)
     }
 }
 
@@ -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,
         );
@@ -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,
         );
@@ -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)]
         })
     }
 }
@@ -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{}}}",

diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
@@ -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;
@@ -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>>>,
@@ -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
@@ -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
@@ -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),

diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs
@@ -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};
@@ -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)
         }));
     }
 
@@ -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>,

diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs
@@ -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);
         }
 

diff --git a/src/test/rustdoc-ui/coverage/exotic.stdout b/src/test/rustdoc-ui/coverage/exotic.stdout
@@ -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% |
 +-------------------------------------+------------+------------+------------+------------+