diff --git a/crates/hir-def/src/expr_store/pretty.rs b/crates/hir-def/src/expr_store/pretty.rs index 7b452721dfe2..56c7655f9ea6 100644 --- a/crates/hir-def/src/expr_store/pretty.rs +++ b/crates/hir-def/src/expr_store/pretty.rs @@ -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 "); 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 5fe72778172f..bb0b70bc5bf2 100644 --- a/crates/hir-def/src/expr_store/tests/body/block.rs +++ b/crates/hir-def/src/expr_store/tests/body/block.rs @@ -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() { diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index 6ebc0d8dfd9c..dccfff002f23 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, }; @@ -1286,7 +1287,6 @@ $0 #[test] fn explicit_private_imports_crate() { - cov_mark::check!(explicit_private_imports); check_found_path( r#" //- /main.rs diff --git a/crates/hir-def/src/item_scope.rs b/crates/hir-def/src/item_scope.rs index 8591874eb943..efa439946850 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 f61b403b84ef..c63333985749 100644 --- a/crates/hir-def/src/item_tree.rs +++ b/crates/hir-def/src/item_tree.rs @@ -414,30 +414,17 @@ impl Index for ItemTree { type Output = RawVisibility; fn index(&self, index: RawVisibilityId) -> &Self::Output { static VIS_PUB: RawVisibility = RawVisibility::Public; - static VIS_PRIV_IMPLICIT: OnceLock = OnceLock::new(); - static VIS_PRIV_EXPLICIT: OnceLock = OnceLock::new(); - static VIS_PUB_CRATE: OnceLock = 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, VisibilityExplicitness), + /// `pub(self)`. + PubSelf(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 eb97081128ed..696174cb072b 100644 --- a/crates/hir-def/src/item_tree/pretty.rs +++ b/crates/hir-def/src/item_tree/pretty.rs @@ -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) "), }; } diff --git a/crates/hir-def/src/item_tree/tests.rs b/crates/hir-def/src/item_tree/tests.rs index e39efd31c6c1..5923b3ea4913 100644 --- a/crates/hir-def/src/item_tree/tests.rs +++ b/crates/hir-def/src/item_tree/tests.rs @@ -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 _}; diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs index 0cd7e894327c..a542214d3031 100644 --- a/crates/hir-def/src/lib.rs +++ b/crates/hir-def/src/lib.rs @@ -149,6 +149,7 @@ impl HasModule for ItemLoc { #[derive(Debug)] pub struct AssocItemLoc { + // FIXME: Store this as an erased `salsa::Id` to save space pub container: ItemContainerId, pub id: AstId, } @@ -577,6 +578,7 @@ pub type LocalModuleId = Idx; #[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 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, } diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 508fcc2fa513..78fdc275606e 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 ca39173d4771..e8235b1c96fe 100644 --- a/crates/hir-def/src/nameres/path_resolution.rs +++ b/crates/hir-def/src/nameres/path_resolution.rs @@ -106,7 +106,7 @@ impl DefMap { visibility: &RawVisibility, within_impl: bool, ) -> Option { - 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) } diff --git a/crates/hir-def/src/resolver.rs b/crates/hir-def/src/resolver.rs index 491b6204bce2..6f321980af43 100644 --- a/crates/hir-def/src/resolver.rs +++ b/crates/hir-def/src/resolver.rs @@ -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), } } diff --git a/crates/hir-def/src/visibility.rs b/crates/hir-def/src/visibility.rs index aea2ac550e3e..2514e88864f3 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,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 { 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 { 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)); diff --git a/crates/hir-ty/src/display.rs b/crates/hir-ty/src/display.rs index da21f08a1e1d..1aa7e0fcf88b 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 05f4a830da5e..d80b704f5306 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, };