user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #20009 from Veykril/push-rvnnorzvpnqv

Browse source 
Optimize `pub(crate)` and `pub(self)` visibility resolution
This commit is contained in:
Lukas Wirth 2025-06-15 08:36:27 +00:00 • committed by GitHub
commit e557333b03
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 136 additions and 73 deletions
Download patch file Download diff file Expand all files Collapse all files

4
src/tools/rust-analyzer/crates/hir-def/src/expr_store/pretty.rs
View file

@ -141,9 +141,11 @@ pub fn print_variant_body_hir(db: &dyn DefDatabase, owner: VariantId, edition: E
let FieldData { name, type_ref, visibility, is_unsafe } = data;
match visibility {
crate::item_tree::RawVisibility::Module(interned, _visibility_explicitness) => {
w!(p, "{}", interned.display(db, p.edition))
w!(p, "pub(in {})", interned.display(db, p.edition))
}
crate::item_tree::RawVisibility::Public => w!(p, "pub "),
crate::item_tree::RawVisibility::PubCrate => w!(p, "pub(crate) "),
crate::item_tree::RawVisibility::PubSelf(_) => w!(p, "pub(self) "),
}
if *is_unsafe {
w!(p, "unsafe ");

2
src/tools/rust-analyzer/crates/hir-def/src/expr_store/tests/body/block.rs
View file

@ -397,7 +397,6 @@ fn main() {
fn underscore_import() {
// This used to panic, because the default (private) visibility inside block expressions would
// point into the containing `DefMap`, which visibilities should never be able to do.
cov_mark::check!(adjust_vis_in_block_def_map);
check_at(
r#"
mod m {
@ -457,7 +456,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() {

2
src/tools/rust-analyzer/crates/hir-def/src/find_path.rs
View file

@ -615,6 +615,7 @@ fn find_local_import_locations(
cov_mark::hit!(discount_private_imports);
false
}
Visibility::PubCrate(_) => true,
Visibility::Public => true,
};
@ -1286,7 +1287,6 @@ $0
#[test]
fn explicit_private_imports_crate() {
cov_mark::check!(explicit_private_imports);
check_found_path(
r#"
//- /main.rs

22
src/tools/rust-analyzer/crates/hir-def/src/item_scope.rs
View file

@ -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)
}
}

33
src/tools/rust-analyzer/crates/hir-def/src/item_tree.rs
View file

@ -414,30 +414,17 @@ impl Index<RawVisibilityId> for ItemTree {
type Output = RawVisibility;
fn index(&self, index: RawVisibilityId) -> &Self::Output {
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_PRIV_IMPLICIT: RawVisibility =
RawVisibility::PubSelf(VisibilityExplicitness::Implicit);
static VIS_PRIV_EXPLICIT: RawVisibility =
RawVisibility::PubSelf(VisibilityExplicitness::Explicit);
static VIS_PUB_CRATE: RawVisibility = RawVisibility::PubCrate;
match index {
RawVisibilityId::PRIV_IMPLICIT => VIS_PRIV_IMPLICIT.get_or_init(|| {
RawVisibility::Module(
Interned::new(ModPath::from_kind(PathKind::SELF)),
VisibilityExplicitness::Implicit,
)
}),
RawVisibilityId::PRIV_EXPLICIT => VIS_PRIV_EXPLICIT.get_or_init(|| {
RawVisibility::Module(
Interned::new(ModPath::from_kind(PathKind::SELF)),
VisibilityExplicitness::Explicit,
)
}),
RawVisibilityId::PRIV_IMPLICIT => &VIS_PRIV_IMPLICIT,
RawVisibilityId::PRIV_EXPLICIT => &VIS_PRIV_EXPLICIT,
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 +542,10 @@ pub enum RawVisibility {
/// `pub(in module)`, `pub(crate)` or `pub(super)`. Also private, which is
/// equivalent to `pub(self)`.
Module(Interned<ModPath>, VisibilityExplicitness),
/// `pub(self)`.
PubSelf(VisibilityExplicitness),
/// `pub(crate)`.
PubCrate,
/// `pub`.
Public,
}

4
src/tools/rust-analyzer/crates/hir-def/src/item_tree/pretty.rs
View file

@ -108,9 +108,11 @@ impl Printer<'_> {
fn print_visibility(&mut self, vis: RawVisibilityId) {
match &self.tree[vis] {
RawVisibility::Module(path, _expl) => {
w!(self, "pub({}) ", path.display(self.db, self.edition))
w!(self, "pub(in {}) ", path.display(self.db, self.edition))
}
RawVisibility::Public => w!(self, "pub "),
RawVisibility::PubCrate => w!(self, "pub(crate) "),
RawVisibility::PubSelf(_) => w!(self, "pub(self) "),
};
}

2
src/tools/rust-analyzer/crates/hir-def/src/item_tree/tests.rs
View file

@ -39,7 +39,7 @@ use a::{c, d::{e}};
pub(self) extern crate self as renamed;
// AstId: ExternCrate[7E1C, 0]
pub(super) extern crate bli;
pub(in super) extern crate bli;
// AstId: Use[0000, 0]
pub use crate::path::{nested, items as renamed, Trait as _};

4
src/tools/rust-analyzer/crates/hir-def/src/lib.rs
View file

@ -149,6 +149,7 @@ impl<N: AstIdNode> HasModule for ItemLoc<N> {
#[derive(Debug)]
pub struct AssocItemLoc<N: AstIdNode> {
// FIXME: Store this as an erased `salsa::Id` to save space
pub container: ItemContainerId,
pub id: AstId<N>,
}
@ -577,6 +578,7 @@ pub type LocalModuleId = Idx<nameres::ModuleData>;
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub struct FieldId {
// FIXME: Store this as an erased `salsa::Id` to save space
pub parent: VariantId,
pub local_id: LocalFieldId,
}
@ -592,6 +594,7 @@ pub struct TupleFieldId {
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct TypeOrConstParamId {
// FIXME: Store this as an erased `salsa::Id` to save space
pub parent: GenericDefId,
pub local_id: LocalTypeOrConstParamId,
}
@ -650,6 +653,7 @@ impl From<ConstParamId> for TypeOrConstParamId {
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub struct LifetimeParamId {
// FIXME: Store this as an erased `salsa::Id` to save space
pub parent: GenericDefId,
pub local_id: LocalLifetimeParamId,
}

3
src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs
View file

@ -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);
}
}

37
src/tools/rust-analyzer/crates/hir-def/src/nameres/path_resolution.rs
View file

@ -106,7 +106,7 @@ impl DefMap {
visibility: &RawVisibility,
within_impl: bool,
) -> Option<Visibility> {
let mut vis = match visibility {
let vis = match visibility {
RawVisibility::Module(path, explicitness) => {
let (result, remaining) = self.resolve_path(
local_def_map,
@ -120,29 +120,36 @@ impl DefMap {
return None;
}
let types = result.take_types()?;
match types {
let mut vis = match types {
ModuleDefId::ModuleId(m) => Visibility::Module(m, *explicitness),
// error: visibility needs to refer to module
_ => {
return None;
}
};
// In block expressions, `self` normally refers to the containing non-block module, and
// `super` to its parent (etc.). However, visibilities must only refer to a module in the
// DefMap they're written in, so we restrict them when that happens.
if let Visibility::Module(m, mv) = vis {
// ...unless we're resolving visibility for an associated item in an impl.
if self.block_id() != m.block && !within_impl {
vis = Visibility::Module(self.module_id(Self::ROOT), mv);
tracing::debug!(
"visibility {:?} points outside DefMap, adjusting to {:?}",
m,
vis
);
}
}
vis
}
RawVisibility::PubSelf(explicitness) => {
Visibility::Module(self.module_id(original_module), *explicitness)
}
RawVisibility::Public => Visibility::Public,
RawVisibility::PubCrate => Visibility::PubCrate(self.krate),
};
// In block expressions, `self` normally refers to the containing non-block module, and
// `super` to its parent (etc.). However, visibilities must only refer to a module in the
// DefMap they're written in, so we restrict them when that happens.
if let Visibility::Module(m, mv) = vis {
// ...unless we're resolving visibility for an associated item in an impl.
if self.block_id() != m.block && !within_impl {
cov_mark::hit!(adjust_vis_in_block_def_map);
vis = Visibility::Module(self.module_id(Self::ROOT), mv);
tracing::debug!("visibility {:?} points outside DefMap, adjusting to {:?}", m, vis);
}
}
Some(vis)
}

4
src/tools/rust-analyzer/crates/hir-def/src/resolver.rs
View file

@ -305,6 +305,10 @@ impl<'db> Resolver<'db> {
}),
)
}
RawVisibility::PubSelf(explicitness) => {
Some(Visibility::Module(self.module(), *explicitness))
}
RawVisibility::PubCrate => Some(Visibility::PubCrate(self.krate())),
RawVisibility::Public => Some(Visibility::Public),
}
}

90
src/tools/rust-analyzer/crates/hir-def/src/visibility.rs
View file

@ -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,8 +44,13 @@ 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 from_module == to_module {
// if the modules are the same, visibility is trivially satisfied
return true;
}
// if they're not in the same crate, it can't be visible
if from_module.krate != to_module.krate {
return false;
@ -59,12 +67,18 @@ 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
if def_map.krate() != to_module.krate {
return false;
}
if from_module == to_module.local_id && def_map.block_id() == to_module.block {
// if the modules are the same, visibility is trivially satisfied
return true;
}
Self::is_visible_from_def_map_(db, def_map, to_module, from_module)
}
@ -88,9 +102,7 @@ impl Visibility {
// `to_module` is not a block, so there is no parent def map to use.
(None, _) => (),
// `to_module` is at `def_map`'s block, no need to move further.
(Some(a), Some(b)) if a == b => {
cov_mark::hit!(is_visible_from_same_block_def_map);
}
(Some(a), Some(b)) if a == b => {}
_ => {
if let Some(parent) = to_module.def_map(db).parent() {
to_module = parent;
@ -132,26 +144,56 @@ 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 == mod_b {
// Most module visibilities are `pub(self)`, and assuming no errors
// this will be the common and thus fast path.
return Some(Visibility::Module(
mod_a,
match (expl_a, expl_b) {
(VisibilityExplicitness::Explicit, _)
| (_, VisibilityExplicitness::Explicit) => {
VisibilityExplicitness::Explicit
}
_ => VisibilityExplicitness::Implicit,
},
));
}
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 +211,52 @@ 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 == mod_b {
// Most module visibilities are `pub(self)`, and assuming no errors
// this will be the common and thus fast path.
return Some(Visibility::Module(
mod_a,
match (expl_a, expl_b) {
(VisibilityExplicitness::Explicit, _)
| (_, VisibilityExplicitness::Explicit) => {
VisibilityExplicitness::Explicit
}
_ => VisibilityExplicitness::Implicit,
},
));
}
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));

1
src/tools/rust-analyzer/crates/hir-ty/src/display.rs
View file

@ -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);

1
src/tools/rust-analyzer/crates/hir/src/symbols.rs
View file

@ -165,6 +165,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,
};