From ba4834c092ed524e7839d21ea40a644db6e6555f Mon Sep 17 00:00:00 2001
From: Vadim Petrochenkov <vadim.petrochenkov@gmail.com>
Date: Wed, 19 Oct 2022 17:56:41 +0400
Subject: [PATCH] resolve: Revert "Set effective visibilities for imports more
precisely"
---
compiler/rustc_resolve/src/access_levels.rs | 26 +++++++++++++-------
compiler/rustc_resolve/src/imports.rs | 27 +--------------------
src/test/ui/privacy/access_levels.rs | 1 +
src/test/ui/privacy/access_levels.stderr | 8 +++++-
4 files changed, 26 insertions(+), 36 deletions(-)
diff --git a/compiler/rustc_resolve/src/access_levels.rs b/compiler/rustc_resolve/src/access_levels.rs
index 1cef60949d812..257784341e3f8 100644
--- a/compiler/rustc_resolve/src/access_levels.rs
+++ b/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,14 +44,13 @@ 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;
@@ -60,16 +58,26 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
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;
}
}
diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs
index 0a86374d76d56..f2cc50c199fc8 100644
--- a/compiler/rustc_resolve/src/imports.rs
+++ b/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
diff --git a/src/test/ui/privacy/access_levels.rs b/src/test/ui/privacy/access_levels.rs
index cc074a4f95865..42c9975bedb70 100644
--- a/src/test/ui/privacy/access_levels.rs
+++ b/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() {}
diff --git a/src/test/ui/privacy/access_levels.stderr b/src/test/ui/privacy/access_levels.stderr
index 19199a3eb87a6..111e02bc329cc 100644
--- a/src/test/ui/privacy/access_levels.stderr
+++ b/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