Calculate visibility on-demand, not up-front

- lots of staring really hard at the existing code to make sure the
  behavior matches
- store `inline_stmt_id` and `crate_stmt_id` to be able to get their visibility (rustdoc stores the IDs of the item definition in `clean::Item`, not the re-export)

Author: jyn514

src/librustdoc/clean/auto_trait.rs

@@ -112,7 +112,6 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {
         Some(Item {
             name: None,
             attrs: Default::default(),
-            visibility: Inherited,
             def_id: ItemId::Auto { trait_: trait_def_id, for_: item_def_id },
             kind: box ImplItem(Impl {
                 unsafety: hir::Unsafety::Normal,
@@ -124,6 +123,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {
                 kind: ImplKind::Auto,
             }),
             cfg: None,
+            inline_stmt_id: None,
         })
     }
 

src/librustdoc/clean/blanket_impl.rs

@@ -103,7 +103,6 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
                 impls.push(Item {
                     name: None,
                     attrs: Default::default(),
-                    visibility: Inherited,
                     def_id: ItemId::Blanket { impl_id: impl_def_id, for_: item_def_id },
                     kind: box ImplItem(Impl {
                         unsafety: hir::Unsafety::Normal,
@@ -127,6 +126,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
                         kind: ImplKind::Blanket(box trait_ref.self_ty().clean(self.cx)),
                     }),
                     cfg: None,
+                    inline_stmt_id: None,
                 });
             }
         }

src/librustdoc/clean/inline.rs

@@ -20,7 +20,7 @@ use crate::clean::{
 use crate::core::DocContext;
 use crate::formats::item_type::ItemType;
 
-use super::{Clean, Visibility};
+use super::Clean;
 
 type Attrs<'hir> = rustc_middle::ty::Attributes<'hir>;
 
@@ -125,11 +125,8 @@ crate fn try_inline(
     let (attrs, cfg) = merge_attrs(cx, Some(parent_module), load_attrs(cx, did), attrs_clone);
     cx.inlined.insert(did.into());
     let mut item =
-        clean::Item::from_def_id_and_attrs_and_parts(did, Some(name), kind, box attrs, cx, cfg);
-    if let Some(import_def_id) = import_def_id {
-        // The visibility needs to reflect the one from the reexport and not from the "source" DefId.
-        item.visibility = cx.tcx.visibility(import_def_id).clean(cx);
-    }
+        clean::Item::from_def_id_and_attrs_and_parts(did, Some(name), kind, box attrs, cfg);
+    item.inline_stmt_id = import_def_id;
     ret.push(item);
     Some(ret)
 }
@@ -194,18 +191,8 @@ crate fn record_extern_fqn(cx: &mut DocContext<'_>, did: DefId, kind: ItemType)
 }
 
 crate fn build_external_trait(cx: &mut DocContext<'_>, did: DefId) -> clean::Trait {
-    let trait_items = cx
-        .tcx
-        .associated_items(did)
-        .in_definition_order()
-        .map(|item| {
-            // When building an external trait, the cleaned trait will have all items public,
-            // which causes methods to have a `pub` prefix, which is invalid since items in traits
-            // can not have a visibility prefix. Thus we override the visibility here manually.
-            // See https://github.com/rust-lang/rust/issues/81274
-            clean::Item { visibility: Visibility::Inherited, ..item.clean(cx) }
-        })
-        .collect();
+    let trait_items =
+        cx.tcx.associated_items(did).in_definition_order().map(|item| item.clean(cx)).collect();
 
     let predicates = cx.tcx.predicates_of(did);
     let generics = (cx.tcx.generics_of(did), predicates).clean(cx);
@@ -497,7 +484,6 @@ crate fn build_impl(
             kind: ImplKind::Normal,
         }),
         box merged_attrs,
-        cx,
         cfg,
     ));
 }
@@ -527,7 +513,6 @@ fn build_module(
                     name: None,
                     attrs: box clean::Attributes::default(),
                     def_id: ItemId::Primitive(prim_ty, did.krate),
-                    visibility: clean::Public,
                     kind: box clean::ImportItem(clean::Import::new_simple(
                         item.ident.name,
                         clean::ImportSource {
@@ -546,6 +531,7 @@ fn build_module(
                         true,
                     )),
                     cfg: None,
+                    inline_stmt_id: None,
                 });
             } else if let Some(i) = try_inline(cx, did, None, res, item.ident.name, None, visited) {
                 items.extend(i)

src/librustdoc/clean/mod.rs

@@ -909,10 +909,7 @@ impl Clean<Item> for hir::TraitItem<'_> {
                     AssocTypeItem(bounds, default)
                 }
             };
-            let what_rustc_thinks =
-                Item::from_def_id_and_parts(local_did, Some(self.ident.name), inner, cx);
-            // Trait items always inherit the trait's visibility -- we don't want to show `pub`.
-            Item { visibility: Inherited, ..what_rustc_thinks }
+            Item::from_def_id_and_parts(local_did, Some(self.ident.name), inner, cx)
         })
     }
 }
@@ -948,20 +945,7 @@ impl Clean<Item> for hir::ImplItem<'_> {
                 }
             };
 
-            let what_rustc_thinks =
-                Item::from_def_id_and_parts(local_did, Some(self.ident.name), inner, cx);
-            let parent_item = cx.tcx.hir().expect_item(cx.tcx.hir().get_parent_did(self.hir_id()));
-            if let hir::ItemKind::Impl(impl_) = &parent_item.kind {
-                if impl_.of_trait.is_some() {
-                    // Trait impl items always inherit the impl's visibility --
-                    // we don't want to show `pub`.
-                    Item { visibility: Inherited, ..what_rustc_thinks }
-                } else {
-                    what_rustc_thinks
-                }
-            } else {
-                panic!("found impl item with non-impl parent {:?}", parent_item);
-            }
+            Item::from_def_id_and_parts(local_did, Some(self.ident.name), inner, cx)
         })
     }
 }
@@ -1570,14 +1554,7 @@ impl Clean<Item> for ty::FieldDef {
 }
 
 fn clean_field(def_id: DefId, name: Symbol, ty: Type, cx: &mut DocContext<'_>) -> Item {
-    let what_rustc_thinks =
-        Item::from_def_id_and_parts(def_id, Some(name), StructFieldItem(ty), cx);
-    if is_field_vis_inherited(cx.tcx, def_id) {
-        // Variant fields inherit their enum's visibility.
-        Item { visibility: Visibility::Inherited, ..what_rustc_thinks }
-    } else {
-        what_rustc_thinks
-    }
+    Item::from_def_id_and_parts(def_id, Some(name), StructFieldItem(ty), cx)
 }
 
 fn is_field_vis_inherited(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
@@ -1592,17 +1569,21 @@ fn is_field_vis_inherited(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
     }
 }
 
+fn clean_vis(vis: ty::Visibility) -> Visibility {
+    match vis {
+        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.
+        // `Item::visibility()` overrides `tcx.visibility` explicitly to make sure this distinction is captured.
+        ty::Visibility::Invisible => Visibility::Inherited,
+        ty::Visibility::Restricted(module) => Visibility::Restricted(module),
+    }
+}
+
 impl Clean<Visibility> for ty::Visibility {
     fn clean(&self, _cx: &mut 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.
-            // 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),
-        }
+        clean_vis(*self)
     }
 }
 
@@ -1635,10 +1616,7 @@ impl Clean<Item> for ty::VariantDef {
                 fields: self.fields.iter().map(|field| field.clean(cx)).collect(),
             }),
         };
-        let what_rustc_thinks =
-            Item::from_def_id_and_parts(self.def_id, Some(self.ident.name), VariantItem(kind), cx);
-        // don't show `pub` for variants, which always inherit visibility
-        Item { visibility: Inherited, ..what_rustc_thinks }
+        Item::from_def_id_and_parts(self.def_id, Some(self.ident.name), VariantItem(kind), cx)
     }
 }
 
@@ -1798,10 +1776,7 @@ impl Clean<Vec<Item>> for (&hir::Item<'_>, Option<Symbol>) {
 impl Clean<Item> for hir::Variant<'_> {
     fn clean(&self, cx: &mut DocContext<'_>) -> Item {
         let kind = VariantItem(self.data.clean(cx));
-        let what_rustc_thinks =
-            Item::from_hir_id_and_parts(self.id, Some(self.ident.name), kind, cx);
-        // don't show `pub` for variants, which are always public
-        Item { visibility: Inherited, ..what_rustc_thinks }
+        Item::from_hir_id_and_parts(self.id, Some(self.ident.name), kind, cx)
     }
 }
 
@@ -1887,9 +1862,9 @@ fn clean_extern_crate(
         name: Some(name),
         attrs: box attrs.clean(cx),
         def_id: crate_def_id.into(),
-        visibility: ty_vis.clean(cx),
-        kind: box ExternCrateItem { src: orig_name },
+        kind: box ExternCrateItem { src: orig_name, crate_stmt_id: krate.def_id },
         cfg: attrs.cfg(cx.tcx, &cx.cache.hidden_cfg),
+        inline_stmt_id: None,
     }]
 }
 

src/librustdoc/clean/types.rs

@@ -18,7 +18,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::thin_vec::ThinVec;
 use rustc_hir as hir;
 use rustc_hir::def::{CtorKind, DefKind, Res};
-use rustc_hir::def_id::{CrateNum, DefId, DefIndex, CRATE_DEF_INDEX, LOCAL_CRATE};
+use rustc_hir::def_id::{CrateNum, DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE};
 use rustc_hir::lang_items::LangItem;
 use rustc_hir::{BodyId, Mutability};
 use rustc_index::vec::IndexVec;
@@ -32,10 +32,10 @@ use rustc_target::abi::VariantIdx;
 use rustc_target::spec::abi::Abi;
 
 use crate::clean::cfg::Cfg;
-use crate::clean::external_path;
 use crate::clean::inline::{self, print_inlined_const};
 use crate::clean::utils::{is_literal_expr, print_const_expr, print_evaluated_const};
 use crate::clean::Clean;
+use crate::clean::{external_path, is_field_vis_inherited};
 use crate::core::DocContext;
 use crate::formats::cache::Cache;
 use crate::formats::item_type::ItemType;
@@ -348,12 +348,12 @@ crate struct Item {
     /// Optional because not every item has a name, e.g. impls.
     crate name: Option<Symbol>,
     crate attrs: Box<Attributes>,
-    crate visibility: Visibility,
     /// Information about this item that is specific to what kind of item it is.
     /// E.g., struct vs enum vs function.
     crate kind: Box<ItemKind>,
     crate def_id: ItemId,
-
+    /// If this item was inlined, the DefId of the `use` statement.
+    crate inline_stmt_id: Option<DefId>,
     crate cfg: Option<Arc<Cfg>>,
 }
 
@@ -388,6 +388,42 @@ impl Item {
         self.def_id.as_def_id().map(|did| tcx.get_attrs(did).inner_docs()).unwrap_or(false)
     }
 
+    crate fn visibility(&self, tcx: TyCtxt<'_>) -> Visibility {
+        let mut def_id = match self.def_id {
+            // Anything but DefId *shouldn't* matter, but return a reasonable value anyway.
+            ItemId::Auto { .. } | ItemId::Blanket { .. } => return Visibility::Inherited,
+            ItemId::Primitive(..) => return Visibility::Public,
+            ItemId::DefId(did) => did,
+        };
+        match *self.kind {
+            // Variant fields inherit their enum's visibility.
+            StructFieldItem(..) if is_field_vis_inherited(tcx, def_id) => {
+                return Visibility::Inherited;
+            }
+            // Variants always inherit visibility
+            VariantItem(..) => return Visibility::Inherited,
+            // Return the visibility of `extern crate` statement, not the crate itself (which will always be public).
+            ExternCrateItem { crate_stmt_id, .. } => def_id = crate_stmt_id.to_def_id(),
+            // Trait items inherit the trait's visibility
+            AssocConstItem(..) | AssocTypeItem(..) | TyMethodItem(..) | MethodItem(..) => {
+                let is_trait_item = match tcx.associated_item(def_id).container {
+                    ty::TraitContainer(_) => true,
+                    ty::ImplContainer(impl_id) => tcx.impl_trait_ref(impl_id).is_some(),
+                };
+                if is_trait_item {
+                    return Visibility::Inherited;
+                }
+            }
+            _ => {}
+        }
+        // If this item was inlined, show the visibility of the `use` statement, not the definition.
+        let def_id = match self.inline_stmt_id {
+            Some(inlined) => inlined,
+            None => def_id,
+        };
+        super::clean_vis(tcx.visibility(def_id))
+    }
+
     crate fn span(&self, tcx: TyCtxt<'_>) -> Span {
         let kind = match &*self.kind {
             ItemKind::StrippedItem(k) => k,
@@ -443,7 +479,6 @@ impl Item {
             name,
             kind,
             box ast_attrs.clean(cx),
-            cx,
             ast_attrs.cfg(cx.tcx, &cx.cache.hidden_cfg),
         )
     }
@@ -453,19 +488,11 @@ impl Item {
         name: Option<Symbol>,
         kind: ItemKind,
         attrs: Box<Attributes>,
-        cx: &mut DocContext<'_>,
         cfg: Option<Arc<Cfg>>,
     ) -> Item {
         trace!("name={:?}, def_id={:?}", name, def_id);
 
-        Item {
-            def_id: def_id.into(),
-            kind: box kind,
-            name,
-            attrs,
-            visibility: cx.tcx.visibility(def_id).clean(cx),
-            cfg,
-        }
+        Item { def_id: def_id.into(), kind: box kind, name, attrs, cfg, inline_stmt_id: None }
     }
 
     /// Finds all `doc` attributes as NameValues and returns their corresponding values, joined
@@ -640,6 +667,8 @@ crate enum ItemKind {
     ExternCrateItem {
         /// The crate's name, *not* the name it's imported as.
         src: Option<Symbol>,
+        /// The id of the `extern crate` statement, not the crate itself.
+        crate_stmt_id: LocalDefId,
     },
     ImportItem(Import),
     StructItem(Struct),

src/librustdoc/fold.rs

@@ -68,7 +68,7 @@ crate trait DocFolder: Sized {
                 }
                 Variant::CLike => VariantItem(Variant::CLike),
             },
-            ExternCrateItem { src: _ }
+            ExternCrateItem { .. }
             | ImportItem(_)
             | FunctionItem(_)
             | TypedefItem(_, _)

src/librustdoc/html/render/mod.rs

@@ -771,7 +771,7 @@ fn assoc_const(
         w,
         "{}{}const <a href=\"{}\" class=\"constant\">{}</a>: {}",
         extra,
-        it.visibility.print_with_space(it.def_id, cx),
+        it.visibility(cx.tcx()).print_with_space(it.def_id, cx),
         naive_assoc_href(it, link, cx),
         it.name.as_ref().unwrap(),
         ty.print(cx)
@@ -892,7 +892,7 @@ fn render_assoc_item(
                 }
             }
         };
-        let vis = meth.visibility.print_with_space(meth.def_id, cx).to_string();
+        let vis = meth.visibility(cx.tcx()).print_with_space(meth.def_id, cx).to_string();
         let constness =
             print_constness_with_space(&header.constness, meth.const_stability(cx.tcx()));
         let asyncness = header.asyncness.print_with_space();