moving public/export res to resolve

lambinoo · lambinoo · commit b888dc9f762c · 2021-08-04T17:48:40.000+02:00
diff --git a/compiler/rustc_middle/src/middle/privacy.rs b/compiler/rustc_middle/src/middle/privacy.rs
@@ -26,7 +26,7 @@ pub enum AccessLevel {
 }
 
 /// Holds a map of accessibility levels for reachable HIR nodes.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct AccessLevels<Id = LocalDefId> {
     pub map: FxHashMap<Id, AccessLevel>,
 }
diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs
@@ -8,7 +8,7 @@ use crate::infer::canonical::{Canonical, CanonicalVarInfo, CanonicalVarInfos};
 use crate::lint::{struct_lint_level, LintDiagnosticBuilder, LintLevelSource};
 use crate::middle;
 use crate::middle::cstore::EncodedMetadata;
-use crate::middle::privacy::AccessLevel;
+use crate::middle::privacy::AccessLevels;
 use crate::middle::resolve_lifetime::{self, LifetimeScopeForPath, ObjectLifetimeDefault};
 use crate::middle::stability;
 use crate::mir::interpret::{self, AllocId, Allocation, ConstValue, Scalar};
@@ -1161,8 +1161,8 @@ impl<'tcx> TyCtxt<'tcx> {
         )
     }
 
-    pub fn get_resolver_access_level(self, local_def_id: LocalDefId) -> Option<AccessLevel> {
-        self.gcx.untracked_resolutions.access_levels.get(&local_def_id).copied()
+    pub fn get_resolver_access_levels(self) -> AccessLevels {
+        self.gcx.untracked_resolutions.access_levels.clone()
     }
 
     pub fn lift<T: Lift<'tcx>>(self, value: T) -> Option<T::Lifted> {
diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs
@@ -23,7 +23,7 @@ pub use vtable::*;
 use crate::hir::exports::ExportMap;
 use crate::ich::StableHashingContext;
 use crate::middle::cstore::CrateStoreDyn;
-use crate::middle::privacy::AccessLevel;
+use crate::middle::privacy::AccessLevels;
 use crate::mir::{Body, GeneratorLayout};
 use crate::traits::{self, Reveal};
 use crate::ty;
@@ -119,7 +119,7 @@ pub struct ResolverOutputs {
     pub definitions: rustc_hir::definitions::Definitions,
     pub cstore: Box<CrateStoreDyn>,
     pub visibilities: FxHashMap<LocalDefId, Visibility>,
-    pub access_levels: FxHashMap<LocalDefId, AccessLevel>,
+    pub access_levels: AccessLevels,
     pub extern_crate_map: FxHashMap<LocalDefId, CrateNum>,
     pub maybe_unused_trait_imports: FxHashSet<LocalDefId>,
     pub maybe_unused_extern_crates: Vec<(LocalDefId, Span)>,
diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs
@@ -11,7 +11,7 @@ use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::struct_span_err;
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
-use rustc_hir::def_id::{DefId, LocalDefId};
+use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID};
 use rustc_hir::intravisit::{self, DeepVisitor, NestedVisitorMap, Visitor};
 use rustc_hir::{AssocItemKind, HirIdSet, Node, PatKind};
 use rustc_middle::bug;
@@ -586,107 +586,26 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> {
     }
 
     fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
-        let inherited_item_level = match item.kind {
-            hir::ItemKind::Impl { .. } => {
-                Option::<AccessLevel>::of_impl(item.hir_id(), self.tcx, &self.access_levels)
-            }
-            // Foreign modules inherit level from parents.
-            hir::ItemKind::ForeignMod { .. }
-            | hir::ItemKind::Const(..)
-            | hir::ItemKind::Enum(..)
-            | hir::ItemKind::ExternCrate(..)
-            | hir::ItemKind::GlobalAsm(..)
-            | hir::ItemKind::Fn(..)
-            | hir::ItemKind::Mod(..)
-            | hir::ItemKind::Static(..)
-            | hir::ItemKind::Struct(..)
-            | hir::ItemKind::Trait(..)
-            | hir::ItemKind::TraitAlias(..)
-            | hir::ItemKind::OpaqueTy(..)
-            | hir::ItemKind::TyAlias(..)
-            | hir::ItemKind::Union(..)
-            | hir::ItemKind::Use(..) => {
-                if item.vis.node.is_pub() {
-                    self.prev_level
-                } else {
-                    None
-                }
-            }
-        };
-
-        // Update level of the item itself.
-        let item_level = self.update(item.def_id, inherited_item_level);
-
-        // Update levels of nested things.
-        match item.kind {
-            hir::ItemKind::Enum(ref def, _) => {
-                for variant in def.variants {
-                    let variant_level =
-                        self.update(self.tcx.hir().local_def_id(variant.id), item_level);
-                    if let Some(ctor_hir_id) = variant.data.ctor_hir_id() {
-                        self.update(self.tcx.hir().local_def_id(ctor_hir_id), item_level);
-                    }
-                    for field in variant.data.fields() {
-                        self.update(self.tcx.hir().local_def_id(field.hir_id), variant_level);
-                    }
-                }
-            }
-            hir::ItemKind::Impl(ref impl_) => {
-                for impl_item_ref in impl_.items {
-                    if impl_.of_trait.is_some() || impl_item_ref.vis.node.is_pub() {
-                        self.update(impl_item_ref.id.def_id, item_level);
-                    }
-                }
-            }
-            hir::ItemKind::Trait(.., trait_item_refs) => {
-                for trait_item_ref in trait_item_refs {
-                    self.update(trait_item_ref.id.def_id, item_level);
+        if let hir::ItemKind::Impl(ref impl_data) = item.kind {
+            let impl_level =
+                Option::<AccessLevel>::of_impl(item.hir_id(), self.tcx, &self.access_levels);
+            self.update(item.def_id, impl_level);
+            for nested in impl_data.items {
+                if impl_data.of_trait.is_some() || nested.vis.node.is_pub() {
+                    self.update(nested.id.def_id, impl_level);
                 }
             }
-            hir::ItemKind::Struct(ref def, _) | hir::ItemKind::Union(ref def, _) => {
-                if let Some(ctor_hir_id) = def.ctor_hir_id() {
-                    self.update(self.tcx.hir().local_def_id(ctor_hir_id), item_level);
-                }
-                for field in def.fields() {
-                    if field.vis.node.is_pub() {
-                        self.update(self.tcx.hir().local_def_id(field.hir_id), item_level);
-                    }
-                }
-            }
-            hir::ItemKind::ForeignMod { items, .. } => {
-                for foreign_item in items {
-                    if foreign_item.vis.node.is_pub() {
-                        self.update(foreign_item.id.def_id, item_level);
-                    }
-                }
-            }
-            hir::ItemKind::OpaqueTy(..)
-            | hir::ItemKind::Use(..)
-            | hir::ItemKind::Static(..)
-            | hir::ItemKind::Const(..)
-            | hir::ItemKind::GlobalAsm(..)
-            | hir::ItemKind::TyAlias(..)
-            | hir::ItemKind::Mod(..)
-            | hir::ItemKind::TraitAlias(..)
-            | hir::ItemKind::Fn(..)
-            | hir::ItemKind::ExternCrate(..) => {}
         }
 
+        let item_level = self.get(item.def_id);
+
         // Mark all items in interfaces of reachable items as reachable.
         match item.kind {
             // The interface is empty.
             hir::ItemKind::ExternCrate(..) => {}
             // All nested items are checked by `visit_item`.
             hir::ItemKind::Mod(..) => {}
-            // Re-exports are handled in `visit_mod`. However, in order to avoid looping over
-            // all of the items of a mod in `visit_mod` looking for use statements, we handle
-            // making sure that intermediate use statements have their visibilities updated here.
-            hir::ItemKind::Use(..) => {
-                if item.vis.node.is_pub() {
-                    let access_level = self.tcx.get_resolver_access_level(item.def_id);
-                    self.update(item.hir_id(), access_level);
-                }
-            }
+            hir::ItemKind::Use(..) => {}
             // The interface is empty.
             hir::ItemKind::GlobalAsm(..) => {}
             hir::ItemKind::OpaqueTy(..) => {
@@ -810,23 +729,6 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> {
     }
 
     fn visit_mod(&mut self, m: &'tcx hir::Mod<'tcx>, _sp: Span, id: hir::HirId) {
-        // This code is here instead of in visit_item so that the
-        // crate module gets processed as well.
-        if self.prev_level.is_some() {
-            let def_id = self.tcx.hir().local_def_id(id);
-            if let Some(exports) = self.tcx.module_exports(def_id) {
-                for export in exports.iter() {
-                    if export.vis == ty::Visibility::Public {
-                        if let Some(def_id) = export.res.opt_def_id() {
-                            if let Some(def_id) = def_id.as_local() {
-                                self.update(def_id, Some(AccessLevel::Exported));
-                            }
-                        }
-                    }
-                }
-            }
-        }
-
         intravisit::walk_mod(self, m, id);
     }
 
@@ -2086,11 +1988,12 @@ fn privacy_access_levels(tcx: TyCtxt<'_>, (): ()) -> &AccessLevels {
     // items which are reachable from external crates based on visibility.
     let mut visitor = EmbargoVisitor {
         tcx,
-        access_levels: Default::default(),
+        access_levels: tcx.get_resolver_access_levels(),
         macro_reachable: Default::default(),
         prev_level: Some(AccessLevel::Public),
         changed: false,
     };
+
     loop {
         intravisit::walk_crate(&mut visitor, tcx.hir().krate());
         if visitor.changed {
@@ -2099,7 +2002,6 @@ fn privacy_access_levels(tcx: TyCtxt<'_>, (): ()) -> &AccessLevels {
             break;
         }
     }
-    visitor.update(CRATE_DEF_ID, Some(AccessLevel::Public));
 
     tcx.arena.alloc(visitor.access_levels)
 }
diff --git a/compiler/rustc_resolve/src/access_levels.rs b/compiler/rustc_resolve/src/access_levels.rs
@@ -3,6 +3,7 @@ use rustc_ast::visit;
 use rustc_ast::visit::Visitor;
 use rustc_ast::EnumDef;
 use rustc_ast::ForeignMod;
+use rustc_ast_lowering::ResolverAstLowering;
 use rustc_middle::middle::privacy::AccessLevel;
 
 use crate::Resolver;
@@ -18,13 +19,15 @@ impl<'r, 'a> PrivacyVisitor<'r, 'a> {
     }
 }
 
-impl<'r, 'a> Visitor<'a> for PrivacyVisitor<'a, 'r> {
-    fn visit_item(&mut self, item: &'a ast::Item) {
+impl<'r, 'ast> Visitor<'ast> for PrivacyVisitor<'ast, 'r> {
+    fn visit_item(&mut self, item: &'ast ast::Item) {
         let inherited_item_level = match item.kind {
             // TODO Is this the correct behavior for those macros?
             ast::ItemKind::MacCall(..) | ast::ItemKind::MacroDef(..) => None,
 
+            // Resolved in privacy when hir is available
             ast::ItemKind::Impl(..) => None,
+
             ast::ItemKind::ForeignMod(..) => self.prev_level,
 
             ast::ItemKind::ExternCrate(..)
@@ -51,18 +54,6 @@ impl<'r, 'a> Visitor<'a> for PrivacyVisitor<'a, 'r> {
         let access_level = self.r.set_access_level(item.id, inherited_item_level);
 
         match item.kind {
-            ast::ItemKind::ExternCrate(..)
-            | ast::ItemKind::Use(..)
-            | ast::ItemKind::Static(..)
-            | ast::ItemKind::Const(..)
-            | ast::ItemKind::GlobalAsm(..)
-            | ast::ItemKind::TyAlias(..)
-            | ast::ItemKind::Mod(..)
-            | ast::ItemKind::TraitAlias(..)
-            | ast::ItemKind::MacroDef(..)
-            | ast::ItemKind::MacCall(..)
-            | ast::ItemKind::Fn(..) => {}
-
             ast::ItemKind::ForeignMod(ForeignMod { ref items, .. }) => {
                 for nested in items {
                     if nested.vis.kind.is_pub() {
@@ -99,17 +90,37 @@ impl<'r, 'a> Visitor<'a> for PrivacyVisitor<'a, 'r> {
             }
             ast::ItemKind::Impl(ref impl_kind) => {
                 for nested in impl_kind.items.iter() {
+                    tracing::info!(
+                        "ast::ItemKind::Impl: nested={:?}",
+                        self.r.opt_local_def_id(nested.id)
+                    );
                     if impl_kind.of_trait.is_some() || nested.vis.kind.is_pub() {
                         self.r.set_access_level(nested.id, access_level);
                     }
                 }
             }
+
+            ast::ItemKind::ExternCrate(..)
+            | ast::ItemKind::Use(..)
+            | ast::ItemKind::Static(..)
+            | ast::ItemKind::Const(..)
+            | ast::ItemKind::GlobalAsm(..)
+            | ast::ItemKind::TyAlias(..)
+            | ast::ItemKind::Mod(..)
+            | ast::ItemKind::TraitAlias(..)
+            | ast::ItemKind::MacroDef(..)
+            | ast::ItemKind::MacCall(..)
+            | ast::ItemKind::Fn(..) => {}
         }
 
         let orig_level = std::mem::replace(&mut self.prev_level, access_level);
         visit::walk_item(self, item);
         self.prev_level = orig_level;
     }
 
-    fn visit_block(&mut self, _block: &'a ast::Block) {}
+    fn visit_block(&mut self, block: &'ast ast::Block) {
+        let orig_level = std::mem::take(&mut self.prev_level);
+        visit::walk_block(self, block);
+        self.prev_level = orig_level;
+    }
 }
diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
@@ -46,7 +46,7 @@ use rustc_index::vec::IndexVec;
 use rustc_metadata::creader::{CStore, CrateLoader};
 use rustc_middle::hir::exports::ExportMap;
 use rustc_middle::middle::cstore::{CrateStore, MetadataLoaderDyn};
-use rustc_middle::middle::privacy::AccessLevel;
+use rustc_middle::middle::privacy::{AccessLevel, AccessLevels};
 use rustc_middle::span_bug;
 use rustc_middle::ty::query::Providers;
 use rustc_middle::ty::{self, DefIdTree, MainDefinition, ResolverOutputs, Visibility};
@@ -1035,7 +1035,7 @@ pub struct Resolver<'a> {
 
     main_def: Option<MainDefinition>,
 
-    nodes_access_level: FxHashMap<LocalDefId, AccessLevel>,
+    nodes_access_level: AccessLevels,
 }
 
 /// Nothing really interesting here; it just provides memory for the rest of the crate.
@@ -1533,10 +1533,18 @@ impl<'a> Resolver<'a> {
 
     /// Compute access levels for exports and intermediate use statements
     fn resolve_access_level(&mut self, krate: &Crate) {
-        let mut privacy_levels =
+        self.set_access_level(CRATE_NODE_ID, Some(AccessLevel::Public));
+
+        let mut privacy_visitor =
             access_levels::PrivacyVisitor::new(self, Some(AccessLevel::Public));
-        visit::walk_crate(&mut privacy_levels, krate);
+        visit::walk_crate(&mut privacy_visitor, krate);
+
+        self.set_exports_access_level();
+
+        tracing::info!("nodes_access_level: {:#?}", self.nodes_access_level);
+    }
 
+    fn set_exports_access_level(&mut self) {
         let root = self.graph_root();
         let exports = root.def_id().and_then(|id| self.export_map.get(&id.expect_local()));
 
@@ -1545,6 +1553,10 @@ impl<'a> Resolver<'a> {
                 exports.iter().filter(|ex| ex.vis == Visibility::Public).collect::<Vec<_>>();
 
             for export in public_exports {
+                if let Some(export_def_id) = export.res.def_id().as_local() {
+                    self.set_access_level_def_id(export_def_id, Some(AccessLevel::Public));
+                }
+
                 if let Some(ns) = export.res.ns() {
                     let key = self.new_key(export.ident, ns);
                     let name_res = self.resolution(root, key);
@@ -1554,8 +1566,6 @@ impl<'a> Resolver<'a> {
                 }
             }
         }
-
-        tracing::debug!("nodes_access_level: {:?}", self.nodes_access_level);
     }
 
     /// Set the given binding access level to `AccessLevel::Public` and
@@ -1584,12 +1594,21 @@ impl<'a> Resolver<'a> {
         node_id: NodeId,
         access_level: Option<AccessLevel>,
     ) -> Option<AccessLevel> {
-        if let Some(access_level) = access_level {
-            let def_id = self.local_def_id(node_id);
-            self.nodes_access_level.insert(def_id, access_level);
-        }
+        self.set_access_level_def_id(self.local_def_id(node_id), access_level)
+    }
 
-        access_level
+    fn set_access_level_def_id(
+        &mut self,
+        def_id: LocalDefId,
+        access_level: Option<AccessLevel>,
+    ) -> Option<AccessLevel> {
+        let old_level = self.nodes_access_level.map.get(&def_id).copied();
+        if old_level < access_level {
+            self.nodes_access_level.map.insert(def_id, access_level.unwrap());
+            access_level
+        } else {
+            old_level
+        }
     }
 
     pub fn traits_in_scope(

Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ pub enum AccessLevel {`
`26`	`26`	`}`
`27`	`27`
`28`	`28`	`/// Holds a map of accessibility levels for reachable HIR nodes.`
`29`		`-#[derive(Debug)]`
	`29`	`+#[derive(Debug, Clone)]`
`30`	`30`	`pub struct AccessLevels<Id = LocalDefId> {`
`31`	`31`	`pub map: FxHashMap<Id, AccessLevel>,`
`32`	`32`	`}`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ use crate::infer::canonical::{Canonical, CanonicalVarInfo, CanonicalVarInfos};`
`8`	`8`	`use crate::lint::{struct_lint_level, LintDiagnosticBuilder, LintLevelSource};`
`9`	`9`	`use crate::middle;`
`10`	`10`	`use crate::middle::cstore::EncodedMetadata;`
`11`		`-use crate::middle::privacy::AccessLevel;`
	`11`	`+use crate::middle::privacy::AccessLevels;`
`12`	`12`	`use crate::middle::resolve_lifetime::{self, LifetimeScopeForPath, ObjectLifetimeDefault};`
`13`	`13`	`use crate::middle::stability;`
`14`	`14`	`use crate::mir::interpret::{self, AllocId, Allocation, ConstValue, Scalar};`
`@@ -1161,8 +1161,8 @@ impl<'tcx> TyCtxt<'tcx> {`
`1161`	`1161`	`)`
`1162`	`1162`	`}`
`1163`	`1163`
`1164`		`- pub fn get_resolver_access_level(self, local_def_id: LocalDefId) -> Option<AccessLevel> {`
`1165`		`- self.gcx.untracked_resolutions.access_levels.get(&local_def_id).copied()`
	`1164`	`+ pub fn get_resolver_access_levels(self) -> AccessLevels {`
	`1165`	`+ self.gcx.untracked_resolutions.access_levels.clone()`
`1166`	`1166`	`}`
`1167`	`1167`
`1168`	`1168`	`pub fn lift<T: Lift<'tcx>>(self, value: T) -> Option<T::Lifted> {`