Auto merge of #103690 - GuillaumeGomez:visibility-on-demand, r=notriddle

Make rustdoc Item::visibility computed on-demand

This is a take-over of #91408.

Helps with #90852 (needs to use `ty::Visibility` directly too).

cc `@camelid`
r? `@notriddle`

Author: bors

Diff for: src/librustdoc/clean/auto_trait.rs

@@ -118,7 +118,6 @@ where
         Some(Item {
             name: None,
             attrs: Default::default(),
-            visibility: Inherited,
             item_id: ItemId::Auto { trait_: trait_def_id, for_: item_def_id },
             kind: Box::new(ImplItem(Box::new(Impl {
                 unsafety: hir::Unsafety::Normal,
@@ -130,6 +129,7 @@ where
                 kind: ImplKind::Auto,
             }))),
             cfg: None,
+            inline_stmt_id: None,
         })
     }
 

Diff for: src/librustdoc/clean/blanket_impl.rs

@@ -97,7 +97,6 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
                 impls.push(Item {
                     name: None,
                     attrs: Default::default(),
-                    visibility: Inherited,
                     item_id: ItemId::Blanket { impl_id: impl_def_id, for_: item_def_id },
                     kind: Box::new(ImplItem(Box::new(Impl {
                         unsafety: hir::Unsafety::Normal,
@@ -128,6 +127,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
                         ))),
                     }))),
                     cfg: None,
+                    inline_stmt_id: None,
                 });
             }
         }

Diff for: src/librustdoc/clean/inline.rs

@@ -20,7 +20,7 @@ use crate::clean::{
     self, clean_fn_decl_from_did_and_sig, clean_generics, clean_impl_item, clean_middle_assoc_item,
     clean_middle_field, clean_middle_ty, clean_trait_ref_with_bindings, clean_ty,
     clean_ty_generics, clean_variant_def, clean_visibility, utils, Attributes, AttributesExt,
-    ImplKind, ItemId, Type, Visibility,
+    ImplKind, ItemId, Type,
 };
 use crate::core::DocContext;
 use crate::formats::item_type::ItemType;
@@ -152,18 +152,10 @@ pub(crate) fn try_inline(
 
     let (attrs, cfg) = merge_attrs(cx, Some(parent_module), load_attrs(cx, did), attrs);
     cx.inlined.insert(did.into());
-    let mut item = clean::Item::from_def_id_and_attrs_and_parts(
-        did,
-        Some(name),
-        kind,
-        Box::new(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 = clean_visibility(cx.tcx.visibility(import_def_id));
-    }
+    let mut item =
+        clean::Item::from_def_id_and_attrs_and_parts(did, Some(name), kind, Box::new(attrs), cfg);
+    // The visibility needs to reflect the one from the reexport and not from the "source" DefId.
+    item.inline_stmt_id = import_def_id;
     ret.push(item);
     Some(ret)
 }
@@ -239,13 +231,7 @@ pub(crate) fn build_external_trait(cx: &mut DocContext<'_>, did: DefId) -> clean
         .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, ..clean_middle_assoc_item(item, cx) }
-        })
+        .map(|item| clean_middle_assoc_item(item, cx))
         .collect();
 
     let predicates = cx.tcx.predicates_of(did);
@@ -559,7 +545,6 @@ pub(crate) fn build_impl(
             },
         })),
         Box::new(merged_attrs),
-        cx,
         cfg,
     ));
 }
@@ -607,7 +592,6 @@ fn build_module_items(
                     name: None,
                     attrs: Box::new(clean::Attributes::default()),
                     item_id: ItemId::Primitive(prim_ty, did.krate),
-                    visibility: clean::Public,
                     kind: Box::new(clean::ImportItem(clean::Import::new_simple(
                         item.ident.name,
                         clean::ImportSource {
@@ -626,6 +610,7 @@ fn build_module_items(
                         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)

Diff for: src/librustdoc/clean/mod.rs

@@ -1083,10 +1083,7 @@ fn clean_trait_item<'tcx>(trait_item: &hir::TraitItem<'tcx>, cx: &mut DocContext
                 TyAssocTypeItem(Box::new(generics), bounds)
             }
         };
-        let what_rustc_thinks =
-            Item::from_def_id_and_parts(local_did, Some(trait_item.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(trait_item.ident.name), inner, cx)
     })
 }
 
@@ -1117,18 +1114,7 @@ pub(crate) fn clean_impl_item<'tcx>(
             }
         };
 
-        let mut what_rustc_thinks =
-            Item::from_def_id_and_parts(local_did, Some(impl_.ident.name), inner, cx);
-
-        let impl_ref = cx.tcx.impl_trait_ref(cx.tcx.local_parent(impl_.owner_id.def_id));
-
-        // Trait impl items always inherit the impl's visibility --
-        // we don't want to show `pub`.
-        if impl_ref.is_some() {
-            what_rustc_thinks.visibility = Inherited;
-        }
-
-        what_rustc_thinks
+        Item::from_def_id_and_parts(local_did, Some(impl_.ident.name), inner, cx)
     })
 }
 
@@ -1340,18 +1326,7 @@ pub(crate) fn clean_middle_assoc_item<'tcx>(
         }
     };
 
-    let mut what_rustc_thinks =
-        Item::from_def_id_and_parts(assoc_item.def_id, Some(assoc_item.name), kind, cx);
-
-    let impl_ref = tcx.impl_trait_ref(tcx.parent(assoc_item.def_id));
-
-    // Trait impl items always inherit the impl's visibility --
-    // we don't want to show `pub`.
-    if impl_ref.is_some() {
-        what_rustc_thinks.visibility = Visibility::Inherited;
-    }
-
-    what_rustc_thinks
+    Item::from_def_id_and_parts(assoc_item.def_id, Some(assoc_item.name), kind, cx)
 }
 
 fn clean_qpath<'tcx>(hir_ty: &hir::Ty<'tcx>, cx: &mut DocContext<'tcx>) -> Type {
@@ -1821,23 +1796,7 @@ pub(crate) fn clean_field_with_def_id(
     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
-    }
-}
-
-fn is_field_vis_inherited(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
-    let parent = tcx.parent(def_id);
-    match tcx.def_kind(parent) {
-        DefKind::Struct | DefKind::Union => false,
-        DefKind::Variant => true,
-        parent_kind => panic!("unexpected parent kind: {:?}", parent_kind),
-    }
+    Item::from_def_id_and_parts(def_id, Some(name), StructFieldItem(ty), cx)
 }
 
 pub(crate) fn clean_visibility(vis: ty::Visibility<DefId>) -> Visibility {
@@ -1861,10 +1820,7 @@ pub(crate) fn clean_variant_def<'tcx>(variant: &ty::VariantDef, cx: &mut DocCont
             fields: variant.fields.iter().map(|field| clean_middle_field(field, cx)).collect(),
         }),
     };
-    let what_rustc_thinks =
-        Item::from_def_id_and_parts(variant.def_id, Some(variant.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(variant.def_id, Some(variant.name), VariantItem(kind), cx)
 }
 
 fn clean_variant_data<'tcx>(
@@ -2038,10 +1994,7 @@ fn clean_maybe_renamed_item<'tcx>(
 
 fn clean_variant<'tcx>(variant: &hir::Variant<'tcx>, cx: &mut DocContext<'tcx>) -> Item {
     let kind = VariantItem(clean_variant_data(&variant.data, &variant.disr_expr, cx));
-    let what_rustc_thinks =
-        Item::from_hir_id_and_parts(variant.id, Some(variant.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(variant.id, Some(variant.ident.name), kind, cx)
 }
 
 fn clean_impl<'tcx>(
@@ -2114,6 +2067,7 @@ fn clean_extern_crate<'tcx>(
                 }
         });
 
+    let krate_owner_def_id = krate.owner_id.to_def_id();
     if please_inline {
         let mut visited = FxHashSet::default();
 
@@ -2122,7 +2076,7 @@ fn clean_extern_crate<'tcx>(
         if let Some(items) = inline::try_inline(
             cx,
             cx.tcx.parent_module(krate.hir_id()).to_def_id(),
-            Some(krate.owner_id.to_def_id()),
+            Some(krate_owner_def_id),
             res,
             name,
             Some(attrs),
@@ -2137,9 +2091,9 @@ fn clean_extern_crate<'tcx>(
         name: Some(name),
         attrs: Box::new(Attributes::from_ast(attrs)),
         item_id: crate_def_id.into(),
-        visibility: clean_visibility(ty_vis),
         kind: Box::new(ExternCrateItem { src: orig_name }),
         cfg: attrs.cfg(cx.tcx, &cx.cache.hidden_cfg),
+        inline_stmt_id: Some(krate_owner_def_id),
     }]
 }
 

Diff for: src/librustdoc/clean/types.rs

@@ -24,7 +24,7 @@ use rustc_hir::{BodyId, Mutability};
 use rustc_hir_analysis::check::intrinsic::intrinsic_operation_unsafety;
 use rustc_index::vec::IndexVec;
 use rustc_middle::ty::fast_reject::SimplifiedType;
-use rustc_middle::ty::{self, TyCtxt};
+use rustc_middle::ty::{self, DefIdTree, TyCtxt};
 use rustc_session::Session;
 use rustc_span::hygiene::MacroKind;
 use rustc_span::source_map::DUMMY_SP;
@@ -348,12 +348,12 @@ pub(crate) struct Item {
     /// Optional because not every item has a name, e.g. impls.
     pub(crate) name: Option<Symbol>,
     pub(crate) attrs: Box<Attributes>,
-    pub(crate) visibility: Visibility,
     /// Information about this item that is specific to what kind of item it is.
     /// E.g., struct vs enum vs function.
     pub(crate) kind: Box<ItemKind>,
     pub(crate) item_id: ItemId,
-
+    /// This is the `DefId` of the `use` statement if the item was inlined.
+    pub(crate) inline_stmt_id: Option<DefId>,
     pub(crate) cfg: Option<Arc<Cfg>>,
 }
 
@@ -364,9 +364,7 @@ impl fmt::Debug for Item {
         let alternate = f.alternate();
         // hand-picked fields that don't bloat the logs too much
         let mut fmt = f.debug_struct("Item");
-        fmt.field("name", &self.name)
-            .field("visibility", &self.visibility)
-            .field("item_id", &self.item_id);
+        fmt.field("name", &self.name).field("item_id", &self.item_id);
         // allow printing the full item if someone really wants to
         if alternate {
             fmt.field("attrs", &self.attrs).field("kind", &self.kind).field("cfg", &self.cfg);
@@ -388,6 +386,15 @@ pub(crate) fn rustc_span(def_id: DefId, tcx: TyCtxt<'_>) -> Span {
     ))
 }
 
+fn is_field_vis_inherited(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
+    let parent = tcx.parent(def_id);
+    match tcx.def_kind(parent) {
+        DefKind::Struct | DefKind::Union => false,
+        DefKind::Variant => true,
+        parent_kind => panic!("unexpected parent kind: {:?}", parent_kind),
+    }
+}
+
 impl Item {
     pub(crate) fn stability<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Option<Stability> {
         self.item_id.as_def_id().and_then(|did| tcx.lookup_stability(did))
@@ -462,7 +469,6 @@ impl Item {
             name,
             kind,
             Box::new(Attributes::from_ast(ast_attrs)),
-            cx,
             ast_attrs.cfg(cx.tcx, &cx.cache.hidden_cfg),
         )
     }
@@ -472,21 +478,18 @@ impl Item {
         name: Option<Symbol>,
         kind: ItemKind,
         attrs: Box<Attributes>,
-        cx: &mut DocContext<'_>,
         cfg: Option<Arc<Cfg>>,
     ) -> Item {
         trace!("name={:?}, def_id={:?} cfg={:?}", name, def_id, cfg);
 
-        // Primitives and Keywords are written in the source code as private modules.
-        // The modules need to be private so that nobody actually uses them, but the
-        // keywords and primitives that they are documenting are public.
-        let visibility = if matches!(&kind, ItemKind::KeywordItem | ItemKind::PrimitiveItem(..)) {
-            Visibility::Public
-        } else {
-            clean_visibility(cx.tcx.visibility(def_id))
-        };
-
-        Item { item_id: def_id.into(), kind: Box::new(kind), name, attrs, visibility, cfg }
+        Item {
+            item_id: def_id.into(),
+            kind: Box::new(kind),
+            name,
+            attrs,
+            cfg,
+            inline_stmt_id: None,
+        }
     }
 
     /// Finds all `doc` attributes as NameValues and returns their corresponding values, joined
@@ -702,6 +705,51 @@ impl Item {
         };
         Some(header)
     }
+
+    pub(crate) fn visibility(&self, tcx: TyCtxt<'_>) -> Visibility {
+        let def_id = match self.item_id {
+            // Anything but DefId *shouldn't* matter, but return a reasonable value anyway.
+            ItemId::Auto { .. } | ItemId::Blanket { .. } => return Visibility::Inherited,
+            // Primitives and Keywords are written in the source code as private modules.
+            // The modules need to be private so that nobody actually uses them, but the
+            // keywords and primitives that they are documenting are public.
+            ItemId::Primitive(..) => return Visibility::Public,
+            ItemId::DefId(def_id) => def_id,
+        };
+
+        match *self.kind {
+            // Explication on `ItemId::Primitive` just above.
+            ItemKind::KeywordItem | ItemKind::PrimitiveItem(_) => return Visibility::Public,
+            // 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,
+            // Trait items inherit the trait's visibility
+            AssocConstItem(..) | TyAssocConstItem(..) | AssocTypeItem(..) | TyAssocTypeItem(..)
+            | TyMethodItem(..) | MethodItem(..) => {
+                let assoc_item = tcx.associated_item(def_id);
+                let is_trait_item = match assoc_item.container {
+                    ty::TraitContainer => true,
+                    ty::ImplContainer => {
+                        // Trait impl items always inherit the impl's visibility --
+                        // we don't want to show `pub`.
+                        tcx.impl_trait_ref(tcx.parent(assoc_item.def_id)).is_some()
+                    }
+                };
+                if is_trait_item {
+                    return Visibility::Inherited;
+                }
+            }
+            _ => {}
+        }
+        let def_id = match self.inline_stmt_id {
+            Some(inlined) => inlined,
+            None => def_id,
+        };
+        clean_visibility(tcx.visibility(def_id))
+    }
 }
 
 #[derive(Clone, Debug)]