Auto merge of #80099 - jyn514:visibility-on-demand, r=GuillaumeGomez

Remove `DefPath` from `Visibility` and calculate it on demand

Depends on #80090 and should not be merged before. Helps with https://github.com/rust-lang/rust/issues/79103 and https://github.com/rust-lang/rust/issues/76382.

cc https://github.com/rust-lang/rust/pull/80014#issuecomment-746810284 - `@nnethercote` I figured it out! It was simpler than I expected :)

This brings the size of `clean::Visibility` down from 40 bytes to 8.

Note that this does *not* remove `clean::Visibility`, even though it's now basically the same as `ty::Visibility`, because the `Invsible` variant means something different from `Inherited` and I thought it would be be confusing to merge the two. See the new comments on `impl Clean for ty::Visibility` for details.

src/librustdoc/clean/mod.rs

@@ -1777,25 +1777,28 @@ fn clean(&self, cx: &DocContext<'_>) -> 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),
         }
     }
 }
@@ -2296,14 +2299,14 @@ fn clean(&self, cx: &DocContext<'_>) -> Item {
             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()

src/librustdoc/clean/types.rs

@@ -1573,11 +1573,11 @@ fn from(prim_ty: hir::PrimTy) -> PrimitiveType {
     }
 }
 
-#[derive(Clone, Debug)]
+#[derive(Copy, Clone, Debug)]
 crate enum Visibility {
     Public,
     Inherited,
-    Restricted(DefId, rustc_hir::definitions::DefPath),
+    Restricted(DefId),
 }
 
 impl Visibility {

src/librustdoc/html/format.rs

@@ -11,6 +11,7 @@
 
 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");

src/librustdoc/html/render/mod.rs

@@ -2157,14 +2157,14 @@ fn cmp(
                     Some(ref src) => write!(
                         w,
                         "<tr><td><code>{}extern crate {} as {};",
-                        myitem.visibility.print_with_space(),
+                        myitem.visibility.print_with_space(cx.tcx()),
                         anchor(myitem.def_id, &*src.as_str()),
                         name
                     ),
                     None => write!(
                         w,
                         "<tr><td><code>{}extern crate {};",
-                        myitem.visibility.print_with_space(),
+                        myitem.visibility.print_with_space(cx.tcx()),
                         anchor(myitem.def_id, &*name.as_str())
                     ),
                 }
@@ -2175,7 +2175,7 @@ fn cmp(
                 write!(
                     w,
                     "<tr><td><code>{}{}</code></td></tr>",
-                    myitem.visibility.print_with_space(),
+                    myitem.visibility.print_with_space(cx.tcx()),
                     import.print()
                 );
             }
@@ -2392,7 +2392,7 @@ fn item_constant(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, c: &clean::
     write!(
         w,
         "{vis}const {name}: {typ}",
-        vis = it.visibility.print_with_space(),
+        vis = it.visibility.print_with_space(cx.tcx()),
         name = it.name.as_ref().unwrap(),
         typ = c.type_.print(),
     );
@@ -2426,7 +2426,7 @@ fn item_static(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::St
     write!(
         w,
         "{vis}static {mutability}{name}: {typ}</pre>",
-        vis = it.visibility.print_with_space(),
+        vis = it.visibility.print_with_space(cx.tcx()),
         mutability = s.mutability.print_with_space(),
         name = it.name.as_ref().unwrap(),
         typ = s.type_.print()
@@ -2437,7 +2437,7 @@ fn item_static(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::St
 fn item_function(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, f: &clean::Function) {
     let header_len = format!(
         "{}{}{}{}{:#}fn {}{:#}",
-        it.visibility.print_with_space(),
+        it.visibility.print_with_space(cx.tcx()),
         f.header.constness.print_with_space(),
         f.header.asyncness.print_with_space(),
         f.header.unsafety.print_with_space(),
@@ -2452,7 +2452,7 @@ fn item_function(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, f: &clean::
         w,
         "{vis}{constness}{asyncness}{unsafety}{abi}fn \
          {name}{generics}{decl}{spotlight}{where_clause}</pre>",
-        vis = it.visibility.print_with_space(),
+        vis = it.visibility.print_with_space(cx.tcx()),
         constness = f.header.constness.print_with_space(),
         asyncness = f.header.asyncness.print_with_space(),
         unsafety = f.header.unsafety.print_with_space(),
@@ -2578,7 +2578,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra
         write!(
             w,
             "{}{}{}trait {}{}{}",
-            it.visibility.print_with_space(),
+            it.visibility.print_with_space(cx.tcx()),
             t.unsafety.print_with_space(),
             if t.is_auto { "auto " } else { "" },
             it.name.as_ref().unwrap(),
@@ -2598,21 +2598,21 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra
             // FIXME: we should be using a derived_id for the Anchors here
             write!(w, "{{\n");
             for t in &types {
-                render_assoc_item(w, t, AssocItemLink::Anchor(None), ItemType::Trait);
+                render_assoc_item(w, t, AssocItemLink::Anchor(None), ItemType::Trait, cx);
                 write!(w, ";\n");
             }
             if !types.is_empty() && !consts.is_empty() {
                 w.write_str("\n");
             }
             for t in &consts {
-                render_assoc_item(w, t, AssocItemLink::Anchor(None), ItemType::Trait);
+                render_assoc_item(w, t, AssocItemLink::Anchor(None), ItemType::Trait, cx);
                 write!(w, ";\n");
             }
             if !consts.is_empty() && !required.is_empty() {
                 w.write_str("\n");
             }
             for (pos, m) in required.iter().enumerate() {
-                render_assoc_item(w, m, AssocItemLink::Anchor(None), ItemType::Trait);
+                render_assoc_item(w, m, AssocItemLink::Anchor(None), ItemType::Trait, cx);
                 write!(w, ";\n");
 
                 if pos < required.len() - 1 {
@@ -2623,7 +2623,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra
                 w.write_str("\n");
             }
             for (pos, m) in provided.iter().enumerate() {
-                render_assoc_item(w, m, AssocItemLink::Anchor(None), ItemType::Trait);
+                render_assoc_item(w, m, AssocItemLink::Anchor(None), ItemType::Trait, cx);
                 match m.kind {
                     clean::MethodItem(ref inner, _)
                         if !inner.generics.where_predicates.is_empty() =>
@@ -2672,7 +2672,7 @@ fn trait_item(
         let item_type = m.type_();
         let id = cx.derive_id(format!("{}.{}", item_type, name));
         write!(w, "<h3 id=\"{id}\" class=\"method\"><code>", id = id,);
-        render_assoc_item(w, m, AssocItemLink::Anchor(Some(&id)), ItemType::Impl);
+        render_assoc_item(w, m, AssocItemLink::Anchor(Some(&id)), ItemType::Impl, cx);
         write!(w, "</code>");
         render_stability_since(w, m, t, cx.tcx());
         write_srclink(cx, m, w, cache);
@@ -2890,12 +2890,13 @@ fn assoc_const(
     _default: Option<&String>,
     link: AssocItemLink<'_>,
     extra: &str,
+    cx: &Context<'_>,
 ) {
     write!(
         w,
         "{}{}const <a href=\"{}\" class=\"constant\"><b>{}</b></a>: {}",
         extra,
-        it.visibility.print_with_space(),
+        it.visibility.print_with_space(cx.tcx()),
         naive_assoc_href(it, link),
         it.name.as_ref().unwrap(),
         ty.print()
@@ -2983,6 +2984,7 @@ fn render_assoc_item(
     item: &clean::Item,
     link: AssocItemLink<'_>,
     parent: ItemType,
+    cx: &Context<'_>,
 ) {
     fn method(
         w: &mut Buffer,
@@ -2992,6 +2994,7 @@ fn method(
         d: &clean::FnDecl,
         link: AssocItemLink<'_>,
         parent: ItemType,
+        cx: &Context<'_>,
     ) {
         let name = meth.name.as_ref().unwrap();
         let anchor = format!("#{}.{}", meth.type_(), name);
@@ -3012,7 +3015,7 @@ fn method(
         };
         let mut header_len = format!(
             "{}{}{}{}{}{:#}fn {}{:#}",
-            meth.visibility.print_with_space(),
+            meth.visibility.print_with_space(cx.tcx()),
             header.constness.print_with_space(),
             header.asyncness.print_with_space(),
             header.unsafety.print_with_space(),
@@ -3034,7 +3037,7 @@ fn method(
             "{}{}{}{}{}{}{}fn <a href=\"{href}\" class=\"fnname\">{name}</a>\
              {generics}{decl}{spotlight}{where_clause}",
             if parent == ItemType::Trait { "    " } else { "" },
-            meth.visibility.print_with_space(),
+            meth.visibility.print_with_space(cx.tcx()),
             header.constness.print_with_space(),
             header.asyncness.print_with_space(),
             header.unsafety.print_with_space(),
@@ -3050,9 +3053,11 @@ fn method(
     }
     match item.kind {
         clean::StrippedItem(..) => {}
-        clean::TyMethodItem(ref m) => method(w, item, m.header, &m.generics, &m.decl, link, parent),
+        clean::TyMethodItem(ref m) => {
+            method(w, item, m.header, &m.generics, &m.decl, link, parent, cx)
+        }
         clean::MethodItem(ref m, _) => {
-            method(w, item, m.header, &m.generics, &m.decl, link, parent)
+            method(w, item, m.header, &m.generics, &m.decl, link, parent, cx)
         }
         clean::AssocConstItem(ref ty, ref default) => assoc_const(
             w,
@@ -3061,6 +3066,7 @@ fn method(
             default.as_ref(),
             link,
             if parent == ItemType::Trait { "    " } else { "" },
+            cx,
         ),
         clean::AssocTypeItem(ref bounds, ref default) => assoc_type(
             w,
@@ -3084,7 +3090,7 @@ fn item_struct(
     wrap_into_docblock(w, |w| {
         write!(w, "<pre class=\"rust struct\">");
         render_attributes(w, it, true);
-        render_struct(w, it, Some(&s.generics), s.struct_type, &s.fields, "", true);
+        render_struct(w, it, Some(&s.generics), s.struct_type, &s.fields, "", true, cx);
         write!(w, "</pre>")
     });
 
@@ -3134,7 +3140,7 @@ fn item_union(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::Uni
     wrap_into_docblock(w, |w| {
         write!(w, "<pre class=\"rust union\">");
         render_attributes(w, it, true);
-        render_union(w, it, Some(&s.generics), &s.fields, "", true);
+        render_union(w, it, Some(&s.generics), &s.fields, "", true, cx);
         write!(w, "</pre>")
     });
 
@@ -3183,7 +3189,7 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum
         write!(
             w,
             "{}enum {}{}{}",
-            it.visibility.print_with_space(),
+            it.visibility.print_with_space(cx.tcx()),
             it.name.as_ref().unwrap(),
             e.generics.print(),
             WhereClause { gens: &e.generics, indent: 0, end_newline: true }
@@ -3209,7 +3215,7 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum
                             write!(w, ")");
                         }
                         clean::VariantKind::Struct(ref s) => {
-                            render_struct(w, v, None, s.struct_type, &s.fields, "    ", false);
+                            render_struct(w, v, None, s.struct_type, &s.fields, "    ", false, cx);
                         }
                     },
                     _ => unreachable!(),
@@ -3353,11 +3359,12 @@ fn render_struct(
     fields: &[clean::Item],
     tab: &str,
     structhead: bool,
+    cx: &Context<'_>,
 ) {
     write!(
         w,
         "{}{}{}",
-        it.visibility.print_with_space(),
+        it.visibility.print_with_space(cx.tcx()),
         if structhead { "struct " } else { "" },
         it.name.as_ref().unwrap()
     );
@@ -3377,7 +3384,7 @@ fn render_struct(
                         w,
                         "\n{}    {}{}: {},",
                         tab,
-                        field.visibility.print_with_space(),
+                        field.visibility.print_with_space(cx.tcx()),
                         field.name.as_ref().unwrap(),
                         ty.print()
                     );
@@ -3406,7 +3413,7 @@ fn render_struct(
                 match field.kind {
                     clean::StrippedItem(box clean::StructFieldItem(..)) => write!(w, "_"),
                     clean::StructFieldItem(ref ty) => {
-                        write!(w, "{}{}", field.visibility.print_with_space(), ty.print())
+                        write!(w, "{}{}", field.visibility.print_with_space(cx.tcx()), ty.print())
                     }
                     _ => unreachable!(),
                 }
@@ -3434,11 +3441,12 @@ fn render_union(
     fields: &[clean::Item],
     tab: &str,
     structhead: bool,
+    cx: &Context<'_>,
 ) {
     write!(
         w,
         "{}{}{}",
-        it.visibility.print_with_space(),
+        it.visibility.print_with_space(cx.tcx()),
         if structhead { "union " } else { "" },
         it.name.as_ref().unwrap()
     );
@@ -3453,7 +3461,7 @@ fn render_union(
             write!(
                 w,
                 "    {}{}: {},\n{}",
-                field.visibility.print_with_space(),
+                field.visibility.print_with_space(cx.tcx()),
                 field.name.as_ref().unwrap(),
                 ty.print(),
                 tab
@@ -3845,7 +3853,7 @@ fn doc_impl_item(
                     let id = cx.derive_id(format!("{}.{}", item_type, name));
                     write!(w, "<h4 id=\"{}\" class=\"{}{}\">", id, item_type, extra_class);
                     write!(w, "<code>");
-                    render_assoc_item(w, item, link.anchor(&id), ItemType::Impl);
+                    render_assoc_item(w, item, link.anchor(&id), ItemType::Impl, cx);
                     write!(w, "</code>");
                     render_stability_since_raw(
                         w,
@@ -3867,7 +3875,7 @@ fn doc_impl_item(
             clean::AssocConstItem(ref ty, ref default) => {
                 let id = cx.derive_id(format!("{}.{}", item_type, name));
                 write!(w, "<h4 id=\"{}\" class=\"{}{}\"><code>", id, item_type, extra_class);
-                assoc_const(w, item, ty, default.as_ref(), link.anchor(&id), "");
+                assoc_const(w, item, ty, default.as_ref(), link.anchor(&id), "", cx);
                 write!(w, "</code>");
                 render_stability_since_raw(
                     w,
@@ -4092,7 +4100,7 @@ fn item_foreign_type(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, cache:
     write!(
         w,
         "    {}type {};\n}}</pre>",
-        it.visibility.print_with_space(),
+        it.visibility.print_with_space(cx.tcx()),
         it.name.as_ref().unwrap(),
     );
 

src/librustdoc/json/conversions.rs

@@ -26,7 +26,7 @@ pub(super) fn convert_item(&self, item: clean::Item) -> Option<Item> {
                 crate_id: def_id.krate.as_u32(),
                 name: name.map(|sym| sym.to_string()),
                 source: self.convert_span(source),
-                visibility: visibility.into(),
+                visibility: self.convert_visibility(visibility),
                 docs: attrs.collapsed_doc_value().unwrap_or_default(),
                 links: attrs
                     .links
@@ -66,31 +66,29 @@ fn convert_span(&self, span: clean::Span) -> Option<Span> {
             _ => None,
         }
     }
-}
-
-impl From<rustc_attr::Deprecation> for Deprecation {
-    fn from(deprecation: rustc_attr::Deprecation) -> Self {
-        #[rustfmt::skip]
-        let rustc_attr::Deprecation { since, note, is_since_rustc_version: _, suggestion: _ } = deprecation;
-        Deprecation { since: since.map(|s| s.to_string()), note: note.map(|s| s.to_string()) }
-    }
-}
 
-impl From<clean::Visibility> for Visibility {
-    fn from(v: clean::Visibility) -> Self {
+    fn convert_visibility(&self, v: clean::Visibility) -> Visibility {
         use clean::Visibility::*;
         match v {
             Public => Visibility::Public,
             Inherited => Visibility::Default,
-            Restricted(did, _) if did.index == CRATE_DEF_INDEX => Visibility::Crate,
-            Restricted(did, path) => Visibility::Restricted {
+            Restricted(did) if did.index == CRATE_DEF_INDEX => Visibility::Crate,
+            Restricted(did) => Visibility::Restricted {
                 parent: did.into(),
-                path: path.to_string_no_crate_verbose(),
+                path: self.tcx.def_path(did).to_string_no_crate_verbose(),
             },
         }
     }
 }
 
+impl From<rustc_attr::Deprecation> for Deprecation {
+    fn from(deprecation: rustc_attr::Deprecation) -> Self {
+        #[rustfmt::skip]
+        let rustc_attr::Deprecation { since, note, is_since_rustc_version: _, suggestion: _ } = deprecation;
+        Deprecation { since: since.map(|s| s.to_string()), note: note.map(|s| s.to_string()) }
+    }
+}
+
 impl From<clean::GenericArgs> for GenericArgs {
     fn from(args: clean::GenericArgs) -> Self {
         use clean::GenericArgs::*;