resolve: Revert "Set effective visibilities for imports more precisely"

Author: petrochenkov

compiler/rustc_resolve/src/access_levels.rs

@@ -1,5 +1,4 @@
-use crate::NameBindingKind;
-use crate::Resolver;
+use crate::{ImportKind, NameBindingKind, Resolver};
 use rustc_ast::ast;
 use rustc_ast::visit;
 use rustc_ast::visit::Visitor;
@@ -45,31 +44,40 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
         let module = self.r.get_module(module_id.to_def_id()).unwrap();
         let resolutions = self.r.resolutions(module);
 
-        for (key, name_resolution) in resolutions.borrow().iter() {
+        for (_, name_resolution) in resolutions.borrow().iter() {
             if let Some(mut binding) = name_resolution.borrow().binding() && !binding.is_ambiguity() {
                 // 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.
 
-                // FIXME: tag and is_public() condition must be deleted,
-                // but assertion fail occurs in import_id_for_ns
+                // FIXME: tag and is_public() condition should be removed, but assertions occur.
                 let tag = if binding.is_import() { AccessLevel::Exported } else { AccessLevel::Public };
                 if binding.vis.is_public() {
                     let mut prev_parent_id = module_id;
                     let mut level = AccessLevel::Public;
                     while let NameBindingKind::Import { binding: nested_binding, import, .. } =
                         binding.kind
                     {
-                        let id = self.r.local_def_id(self.r.import_id_for_ns(import, key.ns));
-                        self.update(
-                            id,
+                        let mut update = |node_id| self.update(
+                            self.r.local_def_id(node_id),
                             binding.vis.expect_local(),
                             prev_parent_id,
                             level,
                         );
+                        // In theory all the import IDs have individual visibilities and effective
+                        // visibilities, but in practice these IDs go straigth to HIR where all
+                        // their few uses assume that their (effective) visibility applies to the
+                        // whole syntactic `use` item. So we update them all to the maximum value
+                        // among the potential individual effective visibilities. Maybe HIR for
+                        // imports shouldn't use three IDs at all.
+                        update(import.id);
+                        if let ImportKind::Single { additional_ids, .. } = import.kind {
+                            update(additional_ids.0);
+                            update(additional_ids.1);
+                        }
 
                         level = AccessLevel::Exported;
-                        prev_parent_id = id;
+                        prev_parent_id = self.r.local_def_id(import.id);
                         binding = nested_binding;
                     }
                 }

compiler/rustc_resolve/src/imports.rs

@@ -2,7 +2,7 @@
 
 use crate::diagnostics::{import_candidates, Suggestion};
 use crate::Determinacy::{self, *};
-use crate::Namespace::{self, *};
+use crate::Namespace::*;
 use crate::{module_to_string, names_to_string, ImportSuggestion};
 use crate::{AmbiguityKind, BindingKey, ModuleKind, ResolutionError, Resolver, Segment};
 use crate::{Finalize, Module, ModuleOrUniformRoot, ParentScope, PerNS, ScopeSet};
@@ -371,31 +371,6 @@ impl<'a> Resolver<'a> {
             self.used_imports.insert(import.id);
         }
     }
-
-    /// Take primary and additional node IDs from an import and select one that corresponds to the
-    /// given namespace. The logic must match the corresponding logic from `fn lower_use_tree` that
-    /// assigns resolutons to IDs.
-    pub(crate) fn import_id_for_ns(&self, import: &Import<'_>, ns: Namespace) -> NodeId {
-        if let ImportKind::Single { additional_ids: (id1, id2), .. } = import.kind {
-            if let Some(resolutions) = self.import_res_map.get(&import.id) {
-                assert!(resolutions[ns].is_some(), "incorrectly finalized import");
-                return match ns {
-                    TypeNS => import.id,
-                    ValueNS => match resolutions.type_ns {
-                        Some(_) => id1,
-                        None => import.id,
-                    },
-                    MacroNS => match (resolutions.type_ns, resolutions.value_ns) {
-                        (Some(_), Some(_)) => id2,
-                        (Some(_), None) | (None, Some(_)) => id1,
-                        (None, None) => import.id,
-                    },
-                };
-            }
-        }
-
-        import.id
-    }
 }
 
 /// An error that may be transformed into a diagnostic later. Used to combine multiple unresolved

src/test/ui/privacy/access_levels.rs

@@ -70,5 +70,6 @@ mod half_public_import {
 
 #[rustc_effective_visibility]
 pub use half_public_import::HalfPublicImport; //~ ERROR Public: pub, Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
+                                              //~^ ERROR Public: pub, Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
 
 fn main() {}

src/test/ui/privacy/access_levels.stderr

@@ -112,6 +112,12 @@ error: Public: pub, Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
 LL | pub use half_public_import::HalfPublicImport;
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+error: Public: pub, Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
+  --> $DIR/access_levels.rs:72:9
+   |
+LL | pub use half_public_import::HalfPublicImport;
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
 error: Public: pub(crate), Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
   --> $DIR/access_levels.rs:14:13
    |
@@ -124,5 +130,5 @@ error: Public: pub(crate), Exported: pub, Reachable: pub, ReachableFromImplTrait
 LL |             type B;
    |             ^^^^^^
 
-error: aborting due to 21 previous errors
+error: aborting due to 22 previous errors