Unnamed repository; edit this file 'description' to name the repository.
Optimize `pub(crate)` visibility resolution
Lukas Wirth 10 months ago
parent 4b38ea5 · commit e129cdc
-rw-r--r--crates/hir-def/src/expr_store/pretty.rs1
-rw-r--r--crates/hir-def/src/expr_store/tests/body/block.rs1
-rw-r--r--crates/hir-def/src/find_path.rs1
-rw-r--r--crates/hir-def/src/item_scope.rs22
-rw-r--r--crates/hir-def/src/item_tree.rs11
-rw-r--r--crates/hir-def/src/item_tree/pretty.rs1
-rw-r--r--crates/hir-def/src/nameres/collector.rs3
-rw-r--r--crates/hir-def/src/nameres/path_resolution.rs1
-rw-r--r--crates/hir-def/src/resolver.rs1
-rw-r--r--crates/hir-def/src/visibility.rs47
-rw-r--r--crates/hir-ty/src/display.rs1
-rw-r--r--crates/hir/src/symbols.rs1
12 files changed, 55 insertions, 36 deletions
diff --git a/crates/hir-def/src/expr_store/pretty.rs b/crates/hir-def/src/expr_store/pretty.rs
index 7b452721df..ae8a959164 100644
--- a/crates/hir-def/src/expr_store/pretty.rs
+++ b/crates/hir-def/src/expr_store/pretty.rs
@@ -144,6 +144,7 @@ pub fn print_variant_body_hir(db: &dyn DefDatabase, owner: VariantId, edition: E
w!(p, "{}", interned.display(db, p.edition))
}
crate::item_tree::RawVisibility::Public => w!(p, "pub "),
+ crate::item_tree::RawVisibility::PubCrate => w!(p, "pub(crate) "),
}
if *is_unsafe {
w!(p, "unsafe ");
diff --git a/crates/hir-def/src/expr_store/tests/body/block.rs b/crates/hir-def/src/expr_store/tests/body/block.rs
index 5fe7277817..5a694c2092 100644
--- a/crates/hir-def/src/expr_store/tests/body/block.rs
+++ b/crates/hir-def/src/expr_store/tests/body/block.rs
@@ -457,7 +457,6 @@ fn foo() {
#[test]
fn is_visible_from_same_def_map() {
// Regression test for https://github.com/rust-lang/rust-analyzer/issues/9481
- cov_mark::check!(is_visible_from_same_block_def_map);
check_at(
r#"
fn outer() {
diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs
index 6ebc0d8dfd..2820729d60 100644
--- a/crates/hir-def/src/find_path.rs
+++ b/crates/hir-def/src/find_path.rs
@@ -615,6 +615,7 @@ fn find_local_import_locations(
cov_mark::hit!(discount_private_imports);
false
}
+ Visibility::PubCrate(_) => true,
Visibility::Public => true,
};
diff --git a/crates/hir-def/src/item_scope.rs b/crates/hir-def/src/item_scope.rs
index 8591874eb9..efa4399468 100644
--- a/crates/hir-def/src/item_scope.rs
+++ b/crates/hir-def/src/item_scope.rs
@@ -20,7 +20,7 @@ use crate::{
LocalModuleId, Lookup, MacroId, ModuleDefId, ModuleId, TraitId, UseId,
db::DefDatabase,
per_ns::{Item, MacrosItem, PerNs, TypesItem, ValuesItem},
- visibility::{Visibility, VisibilityExplicitness},
+ visibility::Visibility,
};
#[derive(Debug, Default)]
@@ -720,33 +720,19 @@ impl ItemScope {
}
/// Marks everything that is not a procedural macro as private to `this_module`.
- pub(crate) fn censor_non_proc_macros(&mut self, this_module: ModuleId) {
+ pub(crate) fn censor_non_proc_macros(&mut self, krate: Crate) {
self.types
.values_mut()
.map(|def| &mut def.vis)
.chain(self.values.values_mut().map(|def| &mut def.vis))
.chain(self.unnamed_trait_imports.iter_mut().map(|(_, def)| &mut def.vis))
- .for_each(|vis| match vis {
- &mut Visibility::Module(_, visibility_explicitness) => {
- *vis = Visibility::Module(this_module, visibility_explicitness)
- }
- Visibility::Public => {
- *vis = Visibility::Module(this_module, VisibilityExplicitness::Implicit)
- }
- });
+ .for_each(|vis| *vis = Visibility::PubCrate(krate));
for mac in self.macros.values_mut() {
if matches!(mac.def, MacroId::ProcMacroId(_) if mac.import.is_none()) {
continue;
}
- match mac.vis {
- Visibility::Module(_, visibility_explicitness) => {
- mac.vis = Visibility::Module(this_module, visibility_explicitness)
- }
- Visibility::Public => {
- mac.vis = Visibility::Module(this_module, VisibilityExplicitness::Implicit)
- }
- }
+ mac.vis = Visibility::PubCrate(krate)
}
}
diff --git a/crates/hir-def/src/item_tree.rs b/crates/hir-def/src/item_tree.rs
index f61b403b84..bf14c0ecee 100644
--- a/crates/hir-def/src/item_tree.rs
+++ b/crates/hir-def/src/item_tree.rs
@@ -416,7 +416,7 @@ impl Index<RawVisibilityId> for ItemTree {
static VIS_PUB: RawVisibility = RawVisibility::Public;
static VIS_PRIV_IMPLICIT: OnceLock<RawVisibility> = OnceLock::new();
static VIS_PRIV_EXPLICIT: OnceLock<RawVisibility> = OnceLock::new();
- static VIS_PUB_CRATE: OnceLock<RawVisibility> = OnceLock::new();
+ static VIS_PUB_CRATE: RawVisibility = RawVisibility::PubCrate;
match index {
RawVisibilityId::PRIV_IMPLICIT => VIS_PRIV_IMPLICIT.get_or_init(|| {
@@ -432,12 +432,7 @@ impl Index<RawVisibilityId> for ItemTree {
)
}),
RawVisibilityId::PUB => &VIS_PUB,
- RawVisibilityId::PUB_CRATE => VIS_PUB_CRATE.get_or_init(|| {
- RawVisibility::Module(
- Interned::new(ModPath::from_kind(PathKind::Crate)),
- VisibilityExplicitness::Explicit,
- )
- }),
+ RawVisibilityId::PUB_CRATE => &VIS_PUB_CRATE,
_ => &self.vis.arena[index.0 as usize],
}
}
@@ -555,6 +550,8 @@ pub enum RawVisibility {
/// `pub(in module)`, `pub(crate)` or `pub(super)`. Also private, which is
/// equivalent to `pub(self)`.
Module(Interned<ModPath>, VisibilityExplicitness),
+ /// `pub(crate)`.
+ PubCrate,
/// `pub`.
Public,
}
diff --git a/crates/hir-def/src/item_tree/pretty.rs b/crates/hir-def/src/item_tree/pretty.rs
index eb97081128..a8e816bd3a 100644
--- a/crates/hir-def/src/item_tree/pretty.rs
+++ b/crates/hir-def/src/item_tree/pretty.rs
@@ -111,6 +111,7 @@ impl Printer<'_> {
w!(self, "pub({}) ", path.display(self.db, self.edition))
}
RawVisibility::Public => w!(self, "pub "),
+ RawVisibility::PubCrate => w!(self, "pub(crate) "),
};
}
diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs
index 508fcc2fa5..78fdc27560 100644
--- a/crates/hir-def/src/nameres/collector.rs
+++ b/crates/hir-def/src/nameres/collector.rs
@@ -437,9 +437,8 @@ impl<'db> DefCollector<'db> {
// Additionally, while the proc macro entry points must be `pub`, they are not publicly
// exported in type/value namespace. This function reduces the visibility of all items
// in the crate root that aren't proc macros.
- let module_id = self.def_map.module_id(DefMap::ROOT);
let root = &mut self.def_map.modules[DefMap::ROOT];
- root.scope.censor_non_proc_macros(module_id);
+ root.scope.censor_non_proc_macros(self.def_map.krate);
}
}
diff --git a/crates/hir-def/src/nameres/path_resolution.rs b/crates/hir-def/src/nameres/path_resolution.rs
index ca39173d47..307acaf1fe 100644
--- a/crates/hir-def/src/nameres/path_resolution.rs
+++ b/crates/hir-def/src/nameres/path_resolution.rs
@@ -129,6 +129,7 @@ impl DefMap {
}
}
RawVisibility::Public => Visibility::Public,
+ RawVisibility::PubCrate => Visibility::PubCrate(self.krate),
};
// In block expressions, `self` normally refers to the containing non-block module, and
diff --git a/crates/hir-def/src/resolver.rs b/crates/hir-def/src/resolver.rs
index 491b6204bc..7b24f6cc74 100644
--- a/crates/hir-def/src/resolver.rs
+++ b/crates/hir-def/src/resolver.rs
@@ -305,6 +305,7 @@ impl<'db> Resolver<'db> {
}),
)
}
+ RawVisibility::PubCrate => Some(Visibility::PubCrate(self.krate())),
RawVisibility::Public => Some(Visibility::Public),
}
}
diff --git a/crates/hir-def/src/visibility.rs b/crates/hir-def/src/visibility.rs
index aea2ac550e..11efeed17b 100644
--- a/crates/hir-def/src/visibility.rs
+++ b/crates/hir-def/src/visibility.rs
@@ -2,6 +2,7 @@
use std::iter;
+use base_db::Crate;
use hir_expand::{InFile, Lookup};
use la_arena::ArenaMap;
use syntax::ast::{self, HasVisibility};
@@ -19,6 +20,8 @@ pub use crate::item_tree::{RawVisibility, VisibilityExplicitness};
pub enum Visibility {
/// Visibility is restricted to a certain module.
Module(ModuleId, VisibilityExplicitness),
+ /// Visibility is restricted to the crate.
+ PubCrate(Crate),
/// Visibility is unrestricted.
Public,
}
@@ -41,6 +44,7 @@ impl Visibility {
pub fn is_visible_from(self, db: &dyn DefDatabase, from_module: ModuleId) -> bool {
let to_module = match self {
Visibility::Module(m, _) => m,
+ Visibility::PubCrate(krate) => return from_module.krate == krate,
Visibility::Public => return true,
};
// if they're not in the same crate, it can't be visible
@@ -59,6 +63,7 @@ impl Visibility {
) -> bool {
let to_module = match self {
Visibility::Module(m, _) => m,
+ Visibility::PubCrate(krate) => return def_map.krate() == krate,
Visibility::Public => return true,
};
// if they're not in the same crate, it can't be visible
@@ -132,26 +137,41 @@ impl Visibility {
pub(crate) fn max(self, other: Visibility, def_map: &DefMap) -> Option<Visibility> {
match (self, other) {
(_, Visibility::Public) | (Visibility::Public, _) => Some(Visibility::Public),
+ (Visibility::PubCrate(krate), Visibility::PubCrate(krateb)) => {
+ if krate == krateb {
+ Some(Visibility::PubCrate(krate))
+ } else {
+ None
+ }
+ }
+ (Visibility::Module(mod_, _), Visibility::PubCrate(krate))
+ | (Visibility::PubCrate(krate), Visibility::Module(mod_, _)) => {
+ if mod_.krate == krate {
+ Some(Visibility::PubCrate(krate))
+ } else {
+ None
+ }
+ }
(Visibility::Module(mod_a, expl_a), Visibility::Module(mod_b, expl_b)) => {
- if mod_a.krate != mod_b.krate {
+ if mod_a.krate != def_map.krate() || mod_b.krate != def_map.krate() {
return None;
}
let def_block = def_map.block_id();
- if (mod_a.containing_block(), mod_b.containing_block()) != (def_block, def_block) {
+ if mod_a.containing_block() != def_block || mod_b.containing_block() != def_block {
return None;
}
let mut a_ancestors =
iter::successors(Some(mod_a.local_id), |&m| def_map[m].parent);
- let mut b_ancestors =
- iter::successors(Some(mod_b.local_id), |&m| def_map[m].parent);
if a_ancestors.any(|m| m == mod_b.local_id) {
// B is above A
return Some(Visibility::Module(mod_b, expl_b));
}
+ let mut b_ancestors =
+ iter::successors(Some(mod_b.local_id), |&m| def_map[m].parent);
if b_ancestors.any(|m| m == mod_a.local_id) {
// A is above B
return Some(Visibility::Module(mod_a, expl_a));
@@ -169,26 +189,37 @@ impl Visibility {
pub(crate) fn min(self, other: Visibility, def_map: &DefMap) -> Option<Visibility> {
match (self, other) {
(vis, Visibility::Public) | (Visibility::Public, vis) => Some(vis),
+ (Visibility::PubCrate(krate), Visibility::PubCrate(krateb)) => {
+ if krate == krateb {
+ Some(Visibility::PubCrate(krate))
+ } else {
+ None
+ }
+ }
+ (Visibility::Module(mod_, exp), Visibility::PubCrate(krate))
+ | (Visibility::PubCrate(krate), Visibility::Module(mod_, exp)) => {
+ if mod_.krate == krate { Some(Visibility::Module(mod_, exp)) } else { None }
+ }
(Visibility::Module(mod_a, expl_a), Visibility::Module(mod_b, expl_b)) => {
- if mod_a.krate != mod_b.krate {
+ if mod_a.krate != def_map.krate() || mod_b.krate != def_map.krate() {
return None;
}
let def_block = def_map.block_id();
- if (mod_a.containing_block(), mod_b.containing_block()) != (def_block, def_block) {
+ if mod_a.containing_block() != def_block || mod_b.containing_block() != def_block {
return None;
}
let mut a_ancestors =
iter::successors(Some(mod_a.local_id), |&m| def_map[m].parent);
- let mut b_ancestors =
- iter::successors(Some(mod_b.local_id), |&m| def_map[m].parent);
if a_ancestors.any(|m| m == mod_b.local_id) {
// B is above A
return Some(Visibility::Module(mod_a, expl_a));
}
+ let mut b_ancestors =
+ iter::successors(Some(mod_b.local_id), |&m| def_map[m].parent);
if b_ancestors.any(|m| m == mod_a.local_id) {
// A is above B
return Some(Visibility::Module(mod_b, expl_b));
diff --git a/crates/hir-ty/src/display.rs b/crates/hir-ty/src/display.rs
index da21f08a1e..1aa7e0fcf8 100644
--- a/crates/hir-ty/src/display.rs
+++ b/crates/hir-ty/src/display.rs
@@ -2082,6 +2082,7 @@ pub fn write_visibility(
) -> Result<(), HirDisplayError> {
match vis {
Visibility::Public => write!(f, "pub "),
+ Visibility::PubCrate(_) => write!(f, "pub(crate) "),
Visibility::Module(vis_id, _) => {
let def_map = module_id.def_map(f.db);
let root_module_id = def_map.module_id(DefMap::ROOT);
diff --git a/crates/hir/src/symbols.rs b/crates/hir/src/symbols.rs
index 05f4a830da..d80b704f53 100644
--- a/crates/hir/src/symbols.rs
+++ b/crates/hir/src/symbols.rs
@@ -164,6 +164,7 @@ impl<'a> SymbolCollector<'a> {
let is_explicit_import = |vis| match vis {
Visibility::Public => true,
+ Visibility::PubCrate(_) => true,
Visibility::Module(_, VisibilityExplicitness::Explicit) => true,
Visibility::Module(_, VisibilityExplicitness::Implicit) => false,
};