Auto merge of #103010 - petrochenkov:effvisdoc, r=GuillaumeGomez

rustdoc: Simplify modifications of effective visibility table

It is now obvious that rustdoc only calls `set_access_level` with foreign def ids and `AccessLevel::Public`.

The second commit makes one more step and separates effective visibilities coming from rustc from similar data collected by rustdoc for extern `DefId`s.
The original table is no longer modified and now only contains local def ids as populated by rustc.

cc https://github.com/rust-lang/rust/pull/102026 `@Bryanskiy`

compiler/rustc_middle/src/middle/privacy.rs

@@ -6,8 +6,7 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_macros::HashStable;
 use rustc_query_system::ich::StableHashingContext;
-use rustc_span::def_id::{DefId, LocalDefId};
-use std::hash::Hash;
+use rustc_span::def_id::LocalDefId;
 
 /// Represents the levels of effective visibility an item can have.
 ///
@@ -75,33 +74,33 @@ impl EffectiveVisibility {
 }
 
 /// Holds a map of effective visibilities for reachable HIR nodes.
-#[derive(Debug, Clone)]
-pub struct EffectiveVisibilities<Id = LocalDefId> {
-    map: FxHashMap<Id, EffectiveVisibility>,
+#[derive(Default, Clone, Debug)]
+pub struct EffectiveVisibilities {
+    map: FxHashMap<LocalDefId, EffectiveVisibility>,
 }
 
-impl<Id: Hash + Eq + Copy> EffectiveVisibilities<Id> {
-    pub fn is_public_at_level(&self, id: Id, level: Level) -> bool {
+impl EffectiveVisibilities {
+    pub fn is_public_at_level(&self, id: LocalDefId, level: Level) -> bool {
         self.effective_vis(id)
             .map_or(false, |effective_vis| effective_vis.is_public_at_level(level))
     }
 
     /// See `Level::Reachable`.
-    pub fn is_reachable(&self, id: Id) -> bool {
+    pub fn is_reachable(&self, id: LocalDefId) -> bool {
         self.is_public_at_level(id, Level::Reachable)
     }
 
     /// See `Level::Reexported`.
-    pub fn is_exported(&self, id: Id) -> bool {
+    pub fn is_exported(&self, id: LocalDefId) -> bool {
         self.is_public_at_level(id, Level::Reexported)
     }
 
     /// See `Level::Direct`.
-    pub fn is_directly_public(&self, id: Id) -> bool {
+    pub fn is_directly_public(&self, id: LocalDefId) -> bool {
         self.is_public_at_level(id, Level::Direct)
     }
 
-    pub fn public_at_level(&self, id: Id) -> Option<Level> {
+    pub fn public_at_level(&self, id: LocalDefId) -> Option<Level> {
         self.effective_vis(id).and_then(|effective_vis| {
             for level in Level::all_levels() {
                 if effective_vis.is_public_at_level(level) {
@@ -112,24 +111,17 @@ impl<Id: Hash + Eq + Copy> EffectiveVisibilities<Id> {
         })
     }
 
-    pub fn effective_vis(&self, id: Id) -> Option<&EffectiveVisibility> {
+    pub fn effective_vis(&self, id: LocalDefId) -> Option<&EffectiveVisibility> {
         self.map.get(&id)
     }
 
-    pub fn iter(&self) -> impl Iterator<Item = (&Id, &EffectiveVisibility)> {
+    pub fn iter(&self) -> impl Iterator<Item = (&LocalDefId, &EffectiveVisibility)> {
         self.map.iter()
     }
 
-    pub fn map_id<OutId: Hash + Eq + Copy>(
-        &self,
-        f: impl Fn(Id) -> OutId,
-    ) -> EffectiveVisibilities<OutId> {
-        EffectiveVisibilities { map: self.map.iter().map(|(k, v)| (f(*k), *v)).collect() }
-    }
-
     pub fn set_public_at_level(
         &mut self,
-        id: Id,
+        id: LocalDefId,
         default_vis: impl FnOnce() -> Visibility,
         level: Level,
     ) {
@@ -144,23 +136,21 @@ impl<Id: Hash + Eq + Copy> EffectiveVisibilities<Id> {
         }
         self.map.insert(id, effective_vis);
     }
-}
 
-impl<Id: Hash + Eq + Copy + Into<DefId>> EffectiveVisibilities<Id> {
     // `parent_id` is not necessarily a parent in source code tree,
     // it is the node from which the maximum effective visibility is inherited.
     pub fn update(
         &mut self,
-        id: Id,
+        id: LocalDefId,
         nominal_vis: Visibility,
         default_vis: impl FnOnce() -> Visibility,
-        parent_id: Id,
+        parent_id: LocalDefId,
         level: Level,
         tree: impl DefIdTree,
     ) -> bool {
         let mut changed = false;
         let mut current_effective_vis = self.effective_vis(id).copied().unwrap_or_else(|| {
-            if id.into().is_crate_root() {
+            if id.is_top_level_module() {
                 EffectiveVisibility::from_vis(Visibility::Public)
             } else {
                 EffectiveVisibility::from_vis(default_vis())
@@ -204,12 +194,6 @@ impl<Id: Hash + Eq + Copy + Into<DefId>> EffectiveVisibilities<Id> {
     }
 }
 
-impl<Id> Default for EffectiveVisibilities<Id> {
-    fn default() -> Self {
-        EffectiveVisibilities { map: Default::default() }
-    }
-}
-
 impl<'a> HashStable<StableHashingContext<'a>> for EffectiveVisibilities {
     fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
         let EffectiveVisibilities { ref map } = *self;

src/librustdoc/clean/blanket_impl.rs

@@ -20,7 +20,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
         trace!("get_blanket_impls({:?})", ty);
         let mut impls = Vec::new();
         for trait_def_id in cx.tcx.all_traits() {
-            if !cx.cache.effective_visibilities.is_directly_public(trait_def_id)
+            if !cx.cache.effective_visibilities.is_directly_public(cx.tcx, trait_def_id)
                 || cx.generated_synthetics.get(&(ty.0, trait_def_id)).is_some()
             {
                 continue;

src/librustdoc/clean/inline.rs

@@ -374,7 +374,7 @@ pub(crate) fn build_impl(
     if !did.is_local() {
         if let Some(traitref) = associated_trait {
             let did = traitref.def_id;
-            if !cx.cache.effective_visibilities.is_directly_public(did) {
+            if !cx.cache.effective_visibilities.is_directly_public(tcx, did) {
                 return;
             }
 
@@ -403,7 +403,7 @@ pub(crate) fn build_impl(
     // reachable in rustdoc generated documentation
     if !did.is_local() {
         if let Some(did) = for_.def_id(&cx.cache) {
-            if !cx.cache.effective_visibilities.is_directly_public(did) {
+            if !cx.cache.effective_visibilities.is_directly_public(tcx, did) {
                 return;
             }
 

src/librustdoc/clean/mod.rs

@@ -1431,7 +1431,7 @@ fn maybe_expand_private_type_alias<'tcx>(
     let Res::Def(DefKind::TyAlias, def_id) = path.res else { return None };
     // Substitute private type aliases
     let def_id = def_id.as_local()?;
-    let alias = if !cx.cache.effective_visibilities.is_exported(def_id.to_def_id()) {
+    let alias = if !cx.cache.effective_visibilities.is_exported(cx.tcx, def_id.to_def_id()) {
         &cx.tcx.hir().expect_item(def_id).kind
     } else {
         return None;

src/librustdoc/clean/utils.rs

@@ -8,7 +8,6 @@ use crate::clean::{
 };
 use crate::core::DocContext;
 use crate::formats::item_type::ItemType;
-use crate::visit_lib::LibEmbargoVisitor;
 
 use rustc_ast as ast;
 use rustc_ast::tokenstream::TokenTree;
@@ -32,7 +31,7 @@ pub(crate) fn krate(cx: &mut DocContext<'_>) -> Crate {
 
     for &cnum in cx.tcx.crates(()) {
         // Analyze doc-reachability for extern items
-        LibEmbargoVisitor::new(cx).visit_lib(cnum);
+        crate::visit_lib::lib_embargo_visit_item(cx, cnum.as_def_id());
     }
 
     // Clean the crate, translating the entire librustc_ast AST to one that is

src/librustdoc/core.rs

@@ -348,7 +348,6 @@ pub(crate) fn run_global_ctxt(
 
     let auto_traits =
         tcx.all_traits().filter(|&trait_def_id| tcx.trait_is_auto(trait_def_id)).collect();
-    let effective_visibilities = tcx.effective_visibilities(()).map_id(Into::into);
 
     let mut ctxt = DocContext {
         tcx,
@@ -361,7 +360,7 @@ pub(crate) fn run_global_ctxt(
         impl_trait_bounds: Default::default(),
         generated_synthetics: Default::default(),
         auto_traits,
-        cache: Cache::new(effective_visibilities, render_options.document_private),
+        cache: Cache::new(render_options.document_private),
         inlined: FxHashSet::default(),
         output_format,
         render_options,

src/librustdoc/formats/cache.rs

@@ -2,7 +2,6 @@ use std::mem;
 
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_hir::def_id::{CrateNum, DefId};
-use rustc_middle::middle::privacy::EffectiveVisibilities;
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_span::Symbol;
 
@@ -15,6 +14,7 @@ use crate::html::format::join_with_double_colon;
 use crate::html::markdown::short_markdown_summary;
 use crate::html::render::search_index::get_function_type_for_search;
 use crate::html::render::IndexItem;
+use crate::visit_lib::RustdocEffectiveVisibilities;
 
 /// This cache is used to store information about the [`clean::Crate`] being
 /// rendered in order to provide more useful documentation. This contains
@@ -78,7 +78,7 @@ pub(crate) struct Cache {
     // Note that external items for which `doc(hidden)` applies to are shown as
     // non-reachable while local items aren't. This is because we're reusing
     // the effective visibilities from the privacy check pass.
-    pub(crate) effective_visibilities: EffectiveVisibilities<DefId>,
+    pub(crate) effective_visibilities: RustdocEffectiveVisibilities,
 
     /// The version of the crate being documented, if given from the `--crate-version` flag.
     pub(crate) crate_version: Option<String>,
@@ -132,11 +132,8 @@ struct CacheBuilder<'a, 'tcx> {
 }
 
 impl Cache {
-    pub(crate) fn new(
-        effective_visibilities: EffectiveVisibilities<DefId>,
-        document_private: bool,
-    ) -> Self {
-        Cache { effective_visibilities, document_private, ..Cache::default() }
+    pub(crate) fn new(document_private: bool) -> Self {
+        Cache { document_private, ..Cache::default() }
     }
 
     /// Populates the `Cache` with more data. The returned `Crate` will be missing some data that was
@@ -387,7 +384,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
                         || self
                             .cache
                             .effective_visibilities
-                            .is_directly_public(item.item_id.expect_def_id())
+                            .is_directly_public(self.tcx, item.item_id.expect_def_id())
                     {
                         self.cache.paths.insert(
                             item.item_id.expect_def_id(),

src/librustdoc/html/format.rs

@@ -659,7 +659,7 @@ pub(crate) fn href_with_root_path(
     }
 
     if !did.is_local()
-        && !cache.effective_visibilities.is_directly_public(did)
+        && !cache.effective_visibilities.is_directly_public(tcx, did)
         && !cache.document_private
         && !cache.primitive_locations.values().any(|&id| id == did)
     {

src/librustdoc/passes/check_doc_test_visibility.rs

@@ -56,7 +56,7 @@ impl crate::doctest::Tester for Tests {
 }
 
 pub(crate) fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) -> bool {
-    if !cx.cache.effective_visibilities.is_directly_public(item.item_id.expect_def_id())
+    if !cx.cache.effective_visibilities.is_directly_public(cx.tcx, item.item_id.expect_def_id())
         || matches!(
             *item.kind,
             clean::StructFieldItem(_)
@@ -130,7 +130,7 @@ pub(crate) fn look_for_tests<'tcx>(cx: &DocContext<'tcx>, dox: &str, item: &Item
             );
         }
     } else if tests.found_tests > 0
-        && !cx.cache.effective_visibilities.is_exported(item.item_id.expect_def_id())
+        && !cx.cache.effective_visibilities.is_exported(cx.tcx, item.item_id.expect_def_id())
     {
         cx.tcx.struct_span_lint_hir(
             crate::lint::PRIVATE_DOC_TESTS,

src/librustdoc/passes/strip_hidden.rs

@@ -27,6 +27,7 @@ pub(crate) fn strip_hidden(krate: clean::Crate, cx: &mut DocContext<'_>) -> clea
 
     // strip all impls referencing stripped items
     let mut stripper = ImplStripper {
+        tcx: cx.tcx,
         retained: &retained,
         cache: &cx.cache,
         is_json_output,

src/librustdoc/passes/strip_private.rs

@@ -22,6 +22,7 @@ pub(crate) fn strip_private(mut krate: clean::Crate, cx: &mut DocContext<'_>) ->
     // strip all private items
     {
         let mut stripper = Stripper {
+            tcx: cx.tcx,
             retained: &mut retained,
             effective_visibilities: &cx.cache.effective_visibilities,
             update_retained: true,
@@ -32,6 +33,7 @@ pub(crate) fn strip_private(mut krate: clean::Crate, cx: &mut DocContext<'_>) ->
 
     // strip all impls referencing private items
     let mut stripper = ImplStripper {
+        tcx: cx.tcx,
         retained: &retained,
         cache: &cx.cache,
         is_json_output,

src/librustdoc/passes/stripper.rs

@@ -1,6 +1,6 @@
 //! A collection of utility functions for the `strip_*` passes.
 use rustc_hir::def_id::DefId;
-use rustc_middle::middle::privacy::EffectiveVisibilities;
+use rustc_middle::ty::TyCtxt;
 use rustc_span::symbol::sym;
 
 use std::mem;
@@ -8,10 +8,12 @@ use std::mem;
 use crate::clean::{self, Item, ItemId, ItemIdSet, NestedAttributesExt};
 use crate::fold::{strip_item, DocFolder};
 use crate::formats::cache::Cache;
+use crate::visit_lib::RustdocEffectiveVisibilities;
 
-pub(crate) struct Stripper<'a> {
+pub(crate) struct Stripper<'a, 'tcx> {
+    pub(crate) tcx: TyCtxt<'tcx>,
     pub(crate) retained: &'a mut ItemIdSet,
-    pub(crate) effective_visibilities: &'a EffectiveVisibilities<DefId>,
+    pub(crate) effective_visibilities: &'a RustdocEffectiveVisibilities,
     pub(crate) update_retained: bool,
     pub(crate) is_json_output: bool,
 }
@@ -21,18 +23,19 @@ pub(crate) struct Stripper<'a> {
 // are in the public API, which is not enough.
 #[inline]
 fn is_item_reachable(
+    tcx: TyCtxt<'_>,
     is_json_output: bool,
-    effective_visibilities: &EffectiveVisibilities<DefId>,
+    effective_visibilities: &RustdocEffectiveVisibilities,
     item_id: ItemId,
 ) -> bool {
     if is_json_output {
-        effective_visibilities.is_reachable(item_id.expect_def_id())
+        effective_visibilities.is_reachable(tcx, item_id.expect_def_id())
     } else {
-        effective_visibilities.is_exported(item_id.expect_def_id())
+        effective_visibilities.is_exported(tcx, item_id.expect_def_id())
     }
 }
 
-impl<'a> DocFolder for Stripper<'a> {
+impl<'a> DocFolder for Stripper<'a, '_> {
     fn fold_item(&mut self, i: Item) -> Option<Item> {
         match *i.kind {
             clean::StrippedItem(..) => {
@@ -66,7 +69,12 @@ impl<'a> DocFolder for Stripper<'a> {
             | clean::ForeignTypeItem => {
                 let item_id = i.item_id;
                 if item_id.is_local()
-                    && !is_item_reachable(self.is_json_output, self.effective_visibilities, item_id)
+                    && !is_item_reachable(
+                        self.tcx,
+                        self.is_json_output,
+                        self.effective_visibilities,
+                        item_id,
+                    )
                 {
                     debug!("Stripper: stripping {:?} {:?}", i.type_(), i.name);
                     return None;
@@ -146,14 +154,15 @@ impl<'a> DocFolder for Stripper<'a> {
 }
 
 /// This stripper discards all impls which reference stripped items
-pub(crate) struct ImplStripper<'a> {
+pub(crate) struct ImplStripper<'a, 'tcx> {
+    pub(crate) tcx: TyCtxt<'tcx>,
     pub(crate) retained: &'a ItemIdSet,
     pub(crate) cache: &'a Cache,
     pub(crate) is_json_output: bool,
     pub(crate) document_private: bool,
 }
 
-impl<'a> ImplStripper<'a> {
+impl<'a> ImplStripper<'a, '_> {
     #[inline]
     fn should_keep_impl(&self, item: &Item, for_def_id: DefId) -> bool {
         if !for_def_id.is_local() || self.retained.contains(&for_def_id.into()) {
@@ -161,7 +170,7 @@ impl<'a> ImplStripper<'a> {
         } else if self.is_json_output {
             // If the "for" item is exported and the impl block isn't `#[doc(hidden)]`, then we
             // need to keep it.
-            self.cache.effective_visibilities.is_exported(for_def_id)
+            self.cache.effective_visibilities.is_exported(self.tcx, for_def_id)
                 && !item.attrs.lists(sym::doc).has_word(sym::hidden)
         } else {
             false
@@ -169,7 +178,7 @@ impl<'a> ImplStripper<'a> {
     }
 }
 
-impl<'a> DocFolder for ImplStripper<'a> {
+impl<'a> DocFolder for ImplStripper<'a, '_> {
     fn fold_item(&mut self, i: Item) -> Option<Item> {
         if let clean::ImplItem(ref imp) = *i.kind {
             // Impl blocks can be skipped if they are: empty; not a trait impl; and have no
@@ -185,6 +194,7 @@ impl<'a> DocFolder for ImplStripper<'a> {
                         let item_id = i.item_id;
                         item_id.is_local()
                             && !is_item_reachable(
+                                self.tcx,
                                 self.is_json_output,
                                 &self.cache.effective_visibilities,
                                 item_id,

src/librustdoc/visit_ast.rs

@@ -7,15 +7,14 @@ use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::DefId;
 use rustc_hir::Node;
 use rustc_hir::CRATE_HIR_ID;
-use rustc_middle::middle::privacy::Level;
-use rustc_middle::ty::{TyCtxt, Visibility};
+use rustc_middle::ty::TyCtxt;
 use rustc_span::def_id::{CRATE_DEF_ID, LOCAL_CRATE};
 use rustc_span::symbol::{kw, sym, Symbol};
 use rustc_span::Span;
 
 use std::mem;
 
-use crate::clean::{self, cfg::Cfg, AttributesExt, NestedAttributesExt};
+use crate::clean::{cfg::Cfg, AttributesExt, NestedAttributesExt};
 use crate::core;
 
 /// This module is used to store stuff from Rust's AST in a more convenient
@@ -221,23 +220,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
         // made reachable by cross-crate inlining which we're checking here.
         // (this is done here because we need to know this upfront).
         if !res_did.is_local() && !is_no_inline {
-            let attrs = clean::inline::load_attrs(self.cx, res_did);
-            let self_is_hidden = attrs.lists(sym::doc).has_word(sym::hidden);
-            if !self_is_hidden {
-                if let Res::Def(kind, did) = res {
-                    if kind == DefKind::Mod {
-                        crate::visit_lib::LibEmbargoVisitor::new(self.cx).visit_mod(did)
-                    } else {
-                        // All items need to be handled here in case someone wishes to link
-                        // to them with intra-doc links
-                        self.cx.cache.effective_visibilities.set_public_at_level(
-                            did,
-                            || Visibility::Restricted(CRATE_DEF_ID),
-                            Level::Direct,
-                        );
-                    }
-                }
-            }
+            crate::visit_lib::lib_embargo_visit_item(self.cx, res_did);
             return false;
         }
 
@@ -246,7 +229,8 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
             None => return false,
         };
 
-        let is_private = !self.cx.cache.effective_visibilities.is_directly_public(res_did);
+        let is_private =
+            !self.cx.cache.effective_visibilities.is_directly_public(self.cx.tcx, res_did);
         let is_hidden = inherits_doc_hidden(self.cx.tcx, res_hir_id);
 
         // Only inline if requested or if the item would otherwise be stripped.

src/librustdoc/visit_lib.rs

@@ -1,86 +1,74 @@
+use crate::core::DocContext;
 use rustc_data_structures::fx::FxHashSet;
-use rustc_hir::def::{DefKind, Res};
-use rustc_hir::def_id::{CrateNum, DefId, CRATE_DEF_ID};
-use rustc_middle::middle::privacy::{EffectiveVisibilities, Level};
-use rustc_middle::ty::{TyCtxt, Visibility};
+use rustc_hir::def::DefKind;
+use rustc_hir::def_id::DefId;
+use rustc_middle::ty::TyCtxt;
 
 // FIXME: this may not be exhaustive, but is sufficient for rustdocs current uses
 
+#[derive(Default)]
+pub(crate) struct RustdocEffectiveVisibilities {
+    extern_public: FxHashSet<DefId>,
+}
+
+macro_rules! define_method {
+    ($method:ident) => {
+        pub(crate) fn $method(&self, tcx: TyCtxt<'_>, def_id: DefId) -> bool {
+            match def_id.as_local() {
+                Some(def_id) => tcx.effective_visibilities(()).$method(def_id),
+                None => self.extern_public.contains(&def_id),
+            }
+        }
+    };
+}
+
+impl RustdocEffectiveVisibilities {
+    define_method!(is_directly_public);
+    define_method!(is_exported);
+    define_method!(is_reachable);
+}
+
+pub(crate) fn lib_embargo_visit_item(cx: &mut DocContext<'_>, def_id: DefId) {
+    assert!(!def_id.is_local());
+    LibEmbargoVisitor {
+        tcx: cx.tcx,
+        extern_public: &mut cx.cache.effective_visibilities.extern_public,
+        visited_mods: Default::default(),
+    }
+    .visit_item(def_id)
+}
+
 /// Similar to `librustc_privacy::EmbargoVisitor`, but also takes
 /// specific rustdoc annotations into account (i.e., `doc(hidden)`)
-pub(crate) struct LibEmbargoVisitor<'a, 'tcx> {
+struct LibEmbargoVisitor<'a, 'tcx> {
     tcx: TyCtxt<'tcx>,
     // Effective visibilities for reachable nodes
-    effective_visibilities: &'a mut EffectiveVisibilities<DefId>,
-    // Previous level, None means unreachable
-    prev_level: Option<Level>,
+    extern_public: &'a mut FxHashSet<DefId>,
     // Keeps track of already visited modules, in case a module re-exports its parent
     visited_mods: FxHashSet<DefId>,
 }
 
-impl<'a, 'tcx> LibEmbargoVisitor<'a, 'tcx> {
-    pub(crate) fn new(cx: &'a mut crate::core::DocContext<'tcx>) -> LibEmbargoVisitor<'a, 'tcx> {
-        LibEmbargoVisitor {
-            tcx: cx.tcx,
-            effective_visibilities: &mut cx.cache.effective_visibilities,
-            prev_level: Some(Level::Direct),
-            visited_mods: FxHashSet::default(),
-        }
-    }
-
-    pub(crate) fn visit_lib(&mut self, cnum: CrateNum) {
-        let did = cnum.as_def_id();
-        self.update(did, Some(Level::Direct));
-        self.visit_mod(did);
-    }
-
-    // Updates node level and returns the updated level
-    fn update(&mut self, did: DefId, level: Option<Level>) -> Option<Level> {
-        let is_hidden = self.tcx.is_doc_hidden(did);
-
-        let old_level = self.effective_visibilities.public_at_level(did);
-        // Visibility levels can only grow
-        if level > old_level && !is_hidden {
-            self.effective_visibilities.set_public_at_level(
-                did,
-                || Visibility::Restricted(CRATE_DEF_ID),
-                level.unwrap(),
-            );
-            level
-        } else {
-            old_level
-        }
-    }
-
-    pub(crate) fn visit_mod(&mut self, def_id: DefId) {
+impl LibEmbargoVisitor<'_, '_> {
+    fn visit_mod(&mut self, def_id: DefId) {
         if !self.visited_mods.insert(def_id) {
             return;
         }
 
         for item in self.tcx.module_children(def_id).iter() {
             if let Some(def_id) = item.res.opt_def_id() {
-                if self.tcx.def_key(def_id).parent.map_or(false, |d| d == def_id.index)
-                    || item.vis.is_public()
-                {
-                    self.visit_item(item.res);
+                if item.vis.is_public() {
+                    self.visit_item(def_id);
                 }
             }
         }
     }
 
-    fn visit_item(&mut self, res: Res<!>) {
-        let def_id = res.def_id();
-        let vis = self.tcx.visibility(def_id);
-        let inherited_item_level = if vis.is_public() { self.prev_level } else { None };
-
-        let item_level = self.update(def_id, inherited_item_level);
-
-        if let Res::Def(DefKind::Mod, _) = res {
-            let orig_level = self.prev_level;
-
-            self.prev_level = item_level;
-            self.visit_mod(def_id);
-            self.prev_level = orig_level;
+    fn visit_item(&mut self, def_id: DefId) {
+        if !self.tcx.is_doc_hidden(def_id) {
+            self.extern_public.insert(def_id);
+            if self.tcx.def_kind(def_id) == DefKind::Mod {
+                self.visit_mod(def_id);
+            }
         }
     }
 }