access_levels.rs refactor

compiler/rustc_resolve/src/access_levels.rs

@@ -1,25 +1,21 @@
+use crate::imports::ImportKind;
+use crate::NameBinding;
+use crate::NameBindingKind;
+use crate::Resolver;
 use rustc_ast::ast;
 use rustc_ast::visit;
 use rustc_ast::visit::Visitor;
 use rustc_ast::Crate;
 use rustc_ast::EnumDef;
-use rustc_ast::ForeignMod;
 use rustc_ast::NodeId;
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::def_id::CRATE_DEF_ID;
 use rustc_middle::middle::privacy::AccessLevel;
-use rustc_middle::ty::Visibility;
+use rustc_middle::ty::DefIdTree;
 use rustc_span::sym;
 
-use crate::imports::ImportKind;
-use crate::BindingKey;
-use crate::NameBinding;
-use crate::NameBindingKind;
-use crate::Resolver;
-
 pub struct AccessLevelsVisitor<'r, 'a> {
     r: &'r mut Resolver<'a>,
-    prev_level: Option<AccessLevel>,
     changed: bool,
 }
 
@@ -28,11 +24,10 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
     /// For now, this doesn't resolve macros (FIXME) and cannot resolve Impl, as we
     /// need access to a TyCtxt for that.
     pub fn compute_access_levels<'c>(r: &'r mut Resolver<'a>, krate: &'c Crate) {
-        let mut visitor =
-            AccessLevelsVisitor { r, changed: false, prev_level: Some(AccessLevel::Public) };
+        let mut visitor = AccessLevelsVisitor { r, changed: false };
 
         visitor.set_access_level_def_id(CRATE_DEF_ID, Some(AccessLevel::Public));
-        visitor.set_exports_access_level(CRATE_DEF_ID);
+        visitor.set_bindings_access_level(CRATE_DEF_ID);
 
         while visitor.changed {
             visitor.reset();
@@ -44,15 +39,17 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
 
     fn reset(&mut self) {
         self.changed = false;
-        self.prev_level = Some(AccessLevel::Public);
     }
 
-    /// Update the access level of the exports of the given module accordingly. The module access
+    /// Update the access level of the bindings in the given module accordingly. The module access
     /// level has to be Exported or Public.
     /// This will also follow `use` chains (see PrivacyVisitor::set_import_binding_access_level).
-    fn set_exports_access_level(&mut self, module_id: LocalDefId) {
+    fn set_bindings_access_level(&mut self, module_id: LocalDefId) {
         assert!(self.r.module_map.contains_key(&&module_id.to_def_id()));
-
+        let module_level = self.r.access_levels.map.get(&module_id).copied();
+        if !module_level.is_some() {
+            return;
+        }
         // Set the given binding access level to `AccessLevel::Public` and
         // sets the rest of the `use` chain to `AccessLevel::Exported` until
         // we hit the actual exported item.
@@ -72,28 +69,20 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
                 }
             };
 
-        let module_level = self.r.access_levels.map.get(&module_id).copied();
-        assert!(module_level >= Some(AccessLevel::Exported));
-
-        if let Some(exports) = self.r.reexport_map.get(&module_id) {
-            let pub_exports = exports
-                .iter()
-                .filter(|ex| ex.vis == Visibility::Public)
-                .cloned()
-                .collect::<Vec<_>>();
-
-            let module = self.r.get_module(module_id.to_def_id()).unwrap();
-            for export in pub_exports.into_iter() {
-                if let Some(export_def_id) = export.res.opt_def_id().and_then(|id| id.as_local()) {
-                    self.set_access_level_def_id(export_def_id, Some(AccessLevel::Exported));
-                }
-
-                if let Some(ns) = export.res.ns() {
-                    let key = BindingKey { ident: export.ident, ns, disambiguator: 0 };
-                    let name_res = self.r.resolution(module, key);
-                    if let Some(binding) = name_res.borrow().binding() {
-                        set_import_binding_access_level(self, binding, module_level)
-                    }
+        let module = self.r.get_module(module_id.to_def_id()).unwrap();
+        let resolutions = self.r.resolutions(module);
+
+        for (.., name_resolution) in resolutions.borrow().iter() {
+            if let Some(binding) = name_resolution.borrow().binding() && binding.vis.is_public() {
+                let access_level = match self.r.is_reexport(binding) {
+                    Some(_) => {
+                        set_import_binding_access_level(self, binding, module_level);
+                        Some(AccessLevel::Exported)
+                    },
+                    None => module_level,
+                };
+                if let Some(def_id) = binding.res().opt_def_id().and_then(|id| id.as_local()) {
+                    self.set_access_level_def_id(def_id, access_level);
                 }
             }
         }
@@ -127,97 +116,59 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
 
 impl<'r, 'ast> Visitor<'ast> for AccessLevelsVisitor<'ast, 'r> {
     fn visit_item(&mut self, item: &'ast ast::Item) {
-        let inherited_item_level = match item.kind {
+        let def_id = self.r.local_def_id(item.id);
+        // Set access level of nested items.
+        // If it's a mod, also make the visitor walk all of its items
+        match item.kind {
             // Resolved in rustc_privacy when types are available
             ast::ItemKind::Impl(..) => return,
 
-            // Only exported `macro_rules!` items are public, but they always are
-            ast::ItemKind::MacroDef(ref macro_def) if macro_def.macro_rules => {
-                let is_macro_export =
-                    item.attrs.iter().any(|attr| attr.has_name(sym::macro_export));
-                if is_macro_export { Some(AccessLevel::Public) } else { None }
-            }
-
-            // Foreign modules inherit level from parents.
-            ast::ItemKind::ForeignMod(..) => self.prev_level,
-
-            // Other `pub` items inherit levels from parents.
-            ast::ItemKind::ExternCrate(..)
-            | ast::ItemKind::Use(..)
-            | ast::ItemKind::Static(..)
-            | ast::ItemKind::Const(..)
-            | ast::ItemKind::Fn(..)
-            | ast::ItemKind::Mod(..)
-            | ast::ItemKind::GlobalAsm(..)
-            | ast::ItemKind::TyAlias(..)
-            | ast::ItemKind::Enum(..)
-            | ast::ItemKind::Struct(..)
-            | ast::ItemKind::Union(..)
-            | ast::ItemKind::Trait(..)
-            | ast::ItemKind::TraitAlias(..)
-            | ast::ItemKind::MacroDef(..) => {
-                if item.vis.kind.is_pub() {
-                    self.prev_level
-                } else {
-                    None
-                }
-            }
-
             // Should be unreachable at this stage
             ast::ItemKind::MacCall(..) => panic!(
                 "ast::ItemKind::MacCall encountered, this should not anymore appear at this stage"
             ),
-        };
 
-        let access_level = self.set_access_level(item.id, inherited_item_level);
+            // Foreign modules inherit level from parents.
+            ast::ItemKind::ForeignMod(..) => {
+                let parent_level =
+                    self.r.access_levels.map.get(&self.r.local_parent(def_id)).copied();
+                self.set_access_level(item.id, parent_level);
+            }
 
-        // Set access level of nested items.
-        // If it's a mod, also make the visitor walk all of its items
-        match item.kind {
-            ast::ItemKind::Mod(..) => {
-                if access_level.is_some() {
-                    self.set_exports_access_level(self.r.local_def_id(item.id));
+            // Only exported `macro_rules!` items are public, but they always are
+            ast::ItemKind::MacroDef(ref macro_def) if macro_def.macro_rules => {
+                if item.attrs.iter().any(|attr| attr.has_name(sym::macro_export)) {
+                    self.set_access_level(item.id, Some(AccessLevel::Public));
                 }
+            }
 
-                let orig_level = std::mem::replace(&mut self.prev_level, access_level);
+            ast::ItemKind::Mod(..) => {
+                self.set_bindings_access_level(def_id);
                 visit::walk_item(self, item);
-                self.prev_level = orig_level;
             }
 
-            ast::ItemKind::ForeignMod(ForeignMod { ref items, .. }) => {
-                for nested in items {
-                    if nested.vis.kind.is_pub() {
-                        self.set_access_level(nested.id, access_level);
-                    }
-                }
-            }
             ast::ItemKind::Enum(EnumDef { ref variants }, _) => {
+                self.set_bindings_access_level(def_id);
                 for variant in variants {
-                    let variant_level = self.set_access_level(variant.id, access_level);
-                    if let Some(ctor_id) = variant.data.ctor_id() {
-                        self.set_access_level(ctor_id, access_level);
-                    }
-
+                    let variant_def_id = self.r.local_def_id(variant.id);
+                    let variant_level = self.r.access_levels.map.get(&variant_def_id).copied();
                     for field in variant.data.fields() {
                         self.set_access_level(field.id, variant_level);
                     }
                 }
             }
-            ast::ItemKind::Struct(ref def, _) | ast::ItemKind::Union(ref def, _) => {
-                if let Some(ctor_id) = def.ctor_id() {
-                    self.set_access_level(ctor_id, access_level);
-                }
 
+            ast::ItemKind::Struct(ref def, _) | ast::ItemKind::Union(ref def, _) => {
+                let inherited_level = self.r.access_levels.map.get(&def_id).copied();
                 for field in def.fields() {
                     if field.vis.kind.is_pub() {
-                        self.set_access_level(field.id, access_level);
+                        self.set_access_level(field.id, inherited_level);
                     }
                 }
             }
-            ast::ItemKind::Trait(ref trait_kind) => {
-                for nested in trait_kind.items.iter() {
-                    self.set_access_level(nested.id, access_level);
-                }
+
+            ast::ItemKind::Trait(..) => {
+                self.set_bindings_access_level(def_id);
             }
 
             ast::ItemKind::ExternCrate(..)
@@ -229,9 +180,6 @@ impl<'r, 'ast> Visitor<'ast> for AccessLevelsVisitor<'ast, 'r> {
             | ast::ItemKind::TraitAlias(..)
             | ast::ItemKind::MacroDef(..)
             | ast::ItemKind::Fn(..) => return,
-
-            // Unreachable kinds
-            ast::ItemKind::Impl(..) | ast::ItemKind::MacCall(..) => unreachable!(),
         }
     }
 }

compiler/rustc_resolve/src/imports.rs

@@ -1091,24 +1091,15 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
         if let Some(def_id) = module.opt_def_id() {
             let mut reexports = Vec::new();
 
-            module.for_each_child(self.r, |_, ident, _, binding| {
-                // FIXME: Consider changing the binding inserted by `#[macro_export] macro_rules`
-                // into the crate root to actual `NameBindingKind::Import`.
-                if binding.is_import()
-                    || matches!(binding.kind, NameBindingKind::Res(_, _is_macro_export @ true))
-                {
-                    let res = binding.res().expect_non_local();
-                    // Ambiguous imports are treated as errors at this point and are
-                    // not exposed to other crates (see #36837 for more details).
-                    if res != def::Res::Err && !binding.is_ambiguity() {
-                        reexports.push(ModChild {
-                            ident,
-                            res,
-                            vis: binding.vis,
-                            span: binding.span,
-                            macro_rules: false,
-                        });
-                    }
+            module.for_each_child(self.r, |this, ident, _, binding| {
+                if let Some(res) = this.is_reexport(binding) {
+                    reexports.push(ModChild {
+                        ident,
+                        res,
+                        vis: binding.vis,
+                        span: binding.span,
+                        macro_rules: false,
+                    });
                 }
             });
 

compiler/rustc_resolve/src/lib.rs

@@ -2014,6 +2014,24 @@ impl<'a> Resolver<'a> {
         }
         self.main_def = Some(MainDefinition { res, is_import, span });
     }
+
+    // Items that go to reexport table encoded to metadata and visible through it to other crates.
+    fn is_reexport(&self, binding: &NameBinding<'a>) -> Option<def::Res<!>> {
+        // FIXME: Consider changing the binding inserted by `#[macro_export] macro_rules`
+        // into the crate root to actual `NameBindingKind::Import`.
+        if binding.is_import()
+            || matches!(binding.kind, NameBindingKind::Res(_, _is_macro_export @ true))
+        {
+            let res = binding.res().expect_non_local();
+            // Ambiguous imports are treated as errors at this point and are
+            // not exposed to other crates (see #36837 for more details).
+            if res != def::Res::Err && !binding.is_ambiguity() {
+                return Some(res);
+            }
+        }
+
+        return None;
+    }
 }
 
 fn names_to_string(names: &[Symbol]) -> String {