Rollup merge of #102109 - petrochenkov:addids, r=oli-obk

resolve: Set effective visibilities for imports more precisely

Instead of setting them for all primary and additional IDs of the import, only set them for the binding's true ID.

Author: matthiaskrgr

compiler/rustc_resolve/src/access_levels.rs

@@ -1,4 +1,3 @@
-use crate::imports::ImportKind;
 use crate::NameBinding;
 use crate::NameBindingKind;
 use crate::Resolver;
@@ -54,15 +53,11 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
         // sets the rest of the `use` chain to `AccessLevel::Exported` until
         // we hit the actual exported item.
         let set_import_binding_access_level =
-            |this: &mut Self, mut binding: &NameBinding<'a>, mut access_level| {
+            |this: &mut Self, mut binding: &NameBinding<'a>, mut access_level, ns| {
                 while let NameBindingKind::Import { binding: nested_binding, import, .. } =
                     binding.kind
                 {
-                    this.set_access_level(import.id, access_level);
-                    if let ImportKind::Single { additional_ids, .. } = import.kind {
-                        this.set_access_level(additional_ids.0, access_level);
-                        this.set_access_level(additional_ids.1, access_level);
-                    }
+                    this.set_access_level(this.r.import_id_for_ns(import, ns), access_level);
 
                     access_level = Some(AccessLevel::Exported);
                     binding = nested_binding;
@@ -72,11 +67,11 @@ 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 (.., name_resolution) in resolutions.borrow().iter() {
+        for (key, name_resolution) in resolutions.borrow().iter() {
             if let Some(binding) = name_resolution.borrow().binding() && binding.vis.is_public() && !binding.is_ambiguity() {
                 let access_level = match binding.is_import() {
                     true => {
-                        set_import_binding_access_level(self, binding, module_level);
+                        set_import_binding_access_level(self, binding, module_level, key.ns);
                         Some(AccessLevel::Exported)
                     },
                     false => module_level,

compiler/rustc_resolve/src/imports.rs

@@ -2,7 +2,7 @@
 
 use crate::diagnostics::Suggestion;
 use crate::Determinacy::{self, *};
-use crate::Namespace::{MacroNS, TypeNS};
+use crate::Namespace::{self, *};
 use crate::{module_to_string, names_to_string};
 use crate::{AmbiguityKind, BindingKey, ModuleKind, ResolutionError, Resolver, Segment};
 use crate::{Finalize, Module, ModuleOrUniformRoot, ParentScope, PerNS, ScopeSet};
@@ -371,6 +371,31 @@ 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

@@ -55,8 +55,21 @@ mod outer { //~ ERROR Public: pub(self), Exported: pub(self), Reachable: pub(sel
     }
 }
 
-pub use outer::inner1;
+#[rustc_effective_visibility]
+pub use outer::inner1; //~ ERROR Public: pub, Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
 
 pub fn foo() -> outer::ReachableStruct { outer::ReachableStruct {a: 0} }
 
+mod half_public_import {
+    #[rustc_effective_visibility]
+    pub type HalfPublicImport = u8; //~ ERROR Public: pub(self), Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
+    #[rustc_effective_visibility]
+    #[allow(non_upper_case_globals)]
+    pub(crate) const HalfPublicImport: u8 = 0; //~ ERROR Public: pub(self), Exported: pub(self), Reachable: pub(self), ReachableFromImplTrait: pub(self)
+}
+
+#[rustc_effective_visibility]
+pub use half_public_import::HalfPublicImport; //~ ERROR Public: pub, Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
+                                              //~^ ERROR Public: pub(self), Exported: pub(self), Reachable: pub(self), ReachableFromImplTrait: pub(self)
+
 fn main() {}

src/test/ui/privacy/access_levels.stderr

@@ -88,6 +88,36 @@ error: Public: pub(self), Exported: pub(self), Reachable: pub, ReachableFromImpl
 LL |         pub a: u8,
    |         ^^^^^^^^^
 
+error: Public: pub, Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
+  --> $DIR/access_levels.rs:59:9
+   |
+LL | pub use outer::inner1;
+   |         ^^^^^^^^^^^^^
+
+error: Public: pub(self), Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
+  --> $DIR/access_levels.rs:65:5
+   |
+LL |     pub type HalfPublicImport = u8;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: Public: pub(self), Exported: pub(self), Reachable: pub(self), ReachableFromImplTrait: pub(self)
+  --> $DIR/access_levels.rs:68:5
+   |
+LL |     pub(crate) const HalfPublicImport: u8 = 0;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+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(self), Exported: pub(self), Reachable: pub(self), ReachableFromImplTrait: pub(self)
+  --> $DIR/access_levels.rs:72:9
+   |
+LL | pub use half_public_import::HalfPublicImport;
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
 error: Public: pub(self), Exported: pub, Reachable: pub, ReachableFromImplTrait: pub
   --> $DIR/access_levels.rs:14:13
    |
@@ -100,5 +130,5 @@ error: Public: pub(self), Exported: pub, Reachable: pub, ReachableFromImplTrait:
 LL |             type B;
    |             ^^^^^^
 
-error: aborting due to 17 previous errors
+error: aborting due to 22 previous errors