Remove DefPath from Visibility and calculate it on demand

rust-lang-ci · Dec 18, 2020 · a2fb4b9 · a2fb4b9
1 parent d8d3ab9
commit a2fb4b9
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 61 deletions.
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
@@ -1775,25 +1775,28 @@ impl Clean<Visibility> for hir::Visibility<'_> {
             hir::VisibilityKind::Inherited => Visibility::Inherited,
             hir::VisibilityKind::Crate(_) => {
                 let krate = DefId::local(CRATE_DEF_INDEX);
-                Visibility::Restricted(krate, cx.tcx.def_path(krate))
+                Visibility::Restricted(krate)
             }
             hir::VisibilityKind::Restricted { ref path, .. } => {
                 let path = path.clean(cx);
                 let did = register_res(cx, path.res);
-                Visibility::Restricted(did, cx.tcx.def_path(did))
+                Visibility::Restricted(did)
             }
         }
     }
 }
 
 impl Clean<Visibility> for ty::Visibility {
-    fn clean(&self, cx: &DocContext<'_>) -> Visibility {
+    fn clean(&self, _cx: &DocContext<'_>) -> Visibility {
         match *self {
             ty::Visibility::Public => Visibility::Public,
+            // NOTE: this is not quite right: `ty` uses `Invisible` to mean 'private',
+            // while rustdoc really does mean inherited. That means that for enum variants, such as
+            // `pub enum E { V }`, `V` will be marked as `Public` by `ty`, but as `Inherited` by rustdoc.
+            // This is the main reason `impl Clean for hir::Visibility` still exists; various parts of clean
+            // override `tcx.visibility` explicitly to make sure this distinction is captured.
             ty::Visibility::Invisible => Visibility::Inherited,
-            ty::Visibility::Restricted(module) => {
-                Visibility::Restricted(module, cx.tcx.def_path(module))
-            }
+            ty::Visibility::Restricted(module) => Visibility::Restricted(module),
         }
     }
 }
@@ -2303,14 +2306,14 @@ impl Clean<Item> for (&hir::MacroDef<'_>, Option<Symbol>) {
             if matchers.len() <= 1 {
                 format!(
                     "{}macro {}{} {{\n    ...\n}}",
-                    vis.print_with_space(),
+                    vis.print_with_space(cx.tcx),
                     name,
                     matchers.iter().map(|span| span.to_src(cx)).collect::<String>(),
                 )
             } else {
                 format!(
                     "{}macro {} {{\n{}}}",
-                    vis.print_with_space(),
+                    vis.print_with_space(cx.tcx),
                     name,
                     matchers
                         .iter()

diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
@@ -1569,11 +1569,11 @@ impl From<hir::PrimTy> for PrimitiveType {
     }
 }
 
-#[derive(Clone, Debug)]
+#[derive(Copy, Clone, Debug)]
 crate enum Visibility {
     Public,
     Inherited,
-    Restricted(DefId, rustc_hir::definitions::DefPath),
+    Restricted(DefId),
 }
 
 impl Visibility {

diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs
@@ -11,6 +11,7 @@ use std::fmt;
 
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir as hir;
+use rustc_middle::ty::TyCtxt;
 use rustc_span::def_id::{DefId, CRATE_DEF_INDEX};
 use rustc_target::spec::abi::Abi;
 
@@ -1084,18 +1085,18 @@ impl Function<'_> {
 }
 
 impl clean::Visibility {
-    crate fn print_with_space(&self) -> impl fmt::Display + '_ {
+    crate fn print_with_space<'tcx>(self, tcx: TyCtxt<'tcx>) -> impl fmt::Display + 'tcx {
         use rustc_span::symbol::kw;
 
-        display_fn(move |f| match *self {
+        display_fn(move |f| match self {
             clean::Public => f.write_str("pub "),
             clean::Inherited => Ok(()),
-            // If this is `pub(crate)`, `path` will be empty.
-            clean::Visibility::Restricted(did, _) if did.index == CRATE_DEF_INDEX => {
+            clean::Visibility::Restricted(did) if did.index == CRATE_DEF_INDEX => {
                 write!(f, "pub(crate) ")
             }
-            clean::Visibility::Restricted(did, ref path) => {
+            clean::Visibility::Restricted(did) => {
                 f.write_str("pub(")?;
+                let path = tcx.def_path(did);
                 debug!("path={:?}", path);
                 let first_name =
                     path.data[0].data.get_opt_name().expect("modules are always named");