Skip to content

Commit e129cdc

Browse files
VeykrilVeykril
committed
Optimize pub(crate) visibility resolution
1 parent 4b38ea5 commit e129cdc

File tree

12 files changed

+55
-36
lines changed

12 files changed

+55
-36
lines changed

‎crates/hir-def/src/expr_store/pretty.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ pub fn print_variant_body_hir(db: &dyn DefDatabase, owner: VariantId, edition: E
144144
w!(p, "{}", interned.display(db, p.edition))
145145
}
146146
crate::item_tree::RawVisibility::Public => w!(p, "pub "),
147+
crate::item_tree::RawVisibility::PubCrate => w!(p, "pub(crate) "),
147148
}
148149
if *is_unsafe {
149150
w!(p, "unsafe ");

‎crates/hir-def/src/expr_store/tests/body/block.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,6 @@ fn foo() {
457457
#[test]
458458
fn is_visible_from_same_def_map() {
459459
// Regression test for https://github.com/rust-lang/rust-analyzer/issues/9481
460-
cov_mark::check!(is_visible_from_same_block_def_map);
461460
check_at(
462461
r#"
463462
fn outer() {

‎crates/hir-def/src/find_path.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,7 @@ fn find_local_import_locations(
615615
cov_mark::hit!(discount_private_imports);
616616
false
617617
}
618+
Visibility::PubCrate(_) => true,
618619
Visibility::Public => true,
619620
};
620621

‎crates/hir-def/src/item_scope.rs‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use crate::{
2020
LocalModuleId, Lookup, MacroId, ModuleDefId, ModuleId, TraitId, UseId,
2121
db::DefDatabase,
2222
per_ns::{Item, MacrosItem, PerNs, TypesItem, ValuesItem},
23-
visibility::{Visibility, VisibilityExplicitness},
23+
visibility::Visibility,
2424
};
2525

2626
#[derive(Debug, Default)]
@@ -720,33 +720,19 @@ impl ItemScope {
720720
}
721721

722722
/// Marks everything that is not a procedural macro as private to `this_module`.
723-
pub(crate) fn censor_non_proc_macros(&mut self, this_module: ModuleId) {
723+
pub(crate) fn censor_non_proc_macros(&mut self, krate: Crate) {
724724
self.types
725725
.values_mut()
726726
.map(|def| &mut def.vis)
727727
.chain(self.values.values_mut().map(|def| &mut def.vis))
728728
.chain(self.unnamed_trait_imports.iter_mut().map(|(_, def)| &mut def.vis))
729-
.for_each(|vis| match vis {
730-
&mut Visibility::Module(_, visibility_explicitness) => {
731-
*vis = Visibility::Module(this_module, visibility_explicitness)
732-
}
733-
Visibility::Public => {
734-
*vis = Visibility::Module(this_module, VisibilityExplicitness::Implicit)
735-
}
736-
});
729+
.for_each(|vis| *vis = Visibility::PubCrate(krate));
737730

738731
for mac in self.macros.values_mut() {
739732
if matches!(mac.def, MacroId::ProcMacroId(_) if mac.import.is_none()) {
740733
continue;
741734
}
742-
match mac.vis {
743-
Visibility::Module(_, visibility_explicitness) => {
744-
mac.vis = Visibility::Module(this_module, visibility_explicitness)
745-
}
746-
Visibility::Public => {
747-
mac.vis = Visibility::Module(this_module, VisibilityExplicitness::Implicit)
748-
}
749-
}
735+
mac.vis = Visibility::PubCrate(krate)
750736
}
751737
}
752738

‎crates/hir-def/src/item_tree.rs‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ impl Index<RawVisibilityId> for ItemTree {
416416
static VIS_PUB: RawVisibility = RawVisibility::Public;
417417
static VIS_PRIV_IMPLICIT: OnceLock<RawVisibility> = OnceLock::new();
418418
static VIS_PRIV_EXPLICIT: OnceLock<RawVisibility> = OnceLock::new();
419-
static VIS_PUB_CRATE: OnceLock<RawVisibility> = OnceLock::new();
419+
static VIS_PUB_CRATE: RawVisibility = RawVisibility::PubCrate;
420420

421421
match index {
422422
RawVisibilityId::PRIV_IMPLICIT => VIS_PRIV_IMPLICIT.get_or_init(|| {
@@ -432,12 +432,7 @@ impl Index<RawVisibilityId> for ItemTree {
432432
)
433433
}),
434434
RawVisibilityId::PUB => &VIS_PUB,
435-
RawVisibilityId::PUB_CRATE => VIS_PUB_CRATE.get_or_init(|| {
436-
RawVisibility::Module(
437-
Interned::new(ModPath::from_kind(PathKind::Crate)),
438-
VisibilityExplicitness::Explicit,
439-
)
440-
}),
435+
RawVisibilityId::PUB_CRATE => &VIS_PUB_CRATE,
441436
_ => &self.vis.arena[index.0 as usize],
442437
}
443438
}
@@ -555,6 +550,8 @@ pub enum RawVisibility {
555550
/// `pub(in module)`, `pub(crate)` or `pub(super)`. Also private, which is
556551
/// equivalent to `pub(self)`.
557552
Module(Interned<ModPath>, VisibilityExplicitness),
553+
/// `pub(crate)`.
554+
PubCrate,
558555
/// `pub`.
559556
Public,
560557
}

‎crates/hir-def/src/item_tree/pretty.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ impl Printer<'_> {
111111
w!(self, "pub({}) ", path.display(self.db, self.edition))
112112
}
113113
RawVisibility::Public => w!(self, "pub "),
114+
RawVisibility::PubCrate => w!(self, "pub(crate) "),
114115
};
115116
}
116117

‎crates/hir-def/src/nameres/collector.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -437,9 +437,8 @@ impl<'db> DefCollector<'db> {
437437
// Additionally, while the proc macro entry points must be `pub`, they are not publicly
438438
// exported in type/value namespace. This function reduces the visibility of all items
439439
// in the crate root that aren't proc macros.
440-
let module_id = self.def_map.module_id(DefMap::ROOT);
441440
let root = &mut self.def_map.modules[DefMap::ROOT];
442-
root.scope.censor_non_proc_macros(module_id);
441+
root.scope.censor_non_proc_macros(self.def_map.krate);
443442
}
444443
}
445444

‎crates/hir-def/src/nameres/path_resolution.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ impl DefMap {
129129
}
130130
}
131131
RawVisibility::Public => Visibility::Public,
132+
RawVisibility::PubCrate => Visibility::PubCrate(self.krate),
132133
};
133134

134135
// In block expressions, `self` normally refers to the containing non-block module, and

‎crates/hir-def/src/resolver.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,7 @@ impl<'db> Resolver<'db> {
305305
}),
306306
)
307307
}
308+
RawVisibility::PubCrate => Some(Visibility::PubCrate(self.krate())),
308309
RawVisibility::Public => Some(Visibility::Public),
309310
}
310311
}

‎crates/hir-def/src/visibility.rs‎

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
33
use std::iter;
44

5+
use base_db::Crate;
56
use hir_expand::{InFile, Lookup};
67
use la_arena::ArenaMap;
78
use syntax::ast::{self, HasVisibility};
@@ -19,6 +20,8 @@ pub use crate::item_tree::{RawVisibility, VisibilityExplicitness};
1920
pub enum Visibility {
2021
/// Visibility is restricted to a certain module.
2122
Module(ModuleId, VisibilityExplicitness),
23+
/// Visibility is restricted to the crate.
24+
PubCrate(Crate),
2225
/// Visibility is unrestricted.
2326
Public,
2427
}
@@ -41,6 +44,7 @@ impl Visibility {
4144
pub fn is_visible_from(self, db: &dyn DefDatabase, from_module: ModuleId) -> bool {
4245
let to_module = match self {
4346
Visibility::Module(m, _) => m,
47+
Visibility::PubCrate(krate) => return from_module.krate == krate,
4448
Visibility::Public => return true,
4549
};
4650
// if they're not in the same crate, it can't be visible
@@ -59,6 +63,7 @@ impl Visibility {
5963
) -> bool {
6064
let to_module = match self {
6165
Visibility::Module(m, _) => m,
66+
Visibility::PubCrate(krate) => return def_map.krate() == krate,
6267
Visibility::Public => return true,
6368
};
6469
// if they're not in the same crate, it can't be visible
@@ -132,26 +137,41 @@ impl Visibility {
132137
pub(crate) fn max(self, other: Visibility, def_map: &DefMap) -> Option<Visibility> {
133138
match (self, other) {
134139
(_, Visibility::Public) | (Visibility::Public, _) => Some(Visibility::Public),
140+
(Visibility::PubCrate(krate), Visibility::PubCrate(krateb)) => {
141+
if krate == krateb {
142+
Some(Visibility::PubCrate(krate))
143+
} else {
144+
None
145+
}
146+
}
147+
(Visibility::Module(mod_, _), Visibility::PubCrate(krate))
148+
| (Visibility::PubCrate(krate), Visibility::Module(mod_, _)) => {
149+
if mod_.krate == krate {
150+
Some(Visibility::PubCrate(krate))
151+
} else {
152+
None
153+
}
154+
}
135155
(Visibility::Module(mod_a, expl_a), Visibility::Module(mod_b, expl_b)) => {
136-
if mod_a.krate != mod_b.krate {
156+
if mod_a.krate != def_map.krate() || mod_b.krate != def_map.krate() {
137157
return None;
138158
}
139159

140160
let def_block = def_map.block_id();
141-
if (mod_a.containing_block(), mod_b.containing_block()) != (def_block, def_block) {
161+
if mod_a.containing_block() != def_block || mod_b.containing_block() != def_block {
142162
return None;
143163
}
144164

145165
let mut a_ancestors =
146166
iter::successors(Some(mod_a.local_id), |&m| def_map[m].parent);
147-
let mut b_ancestors =
148-
iter::successors(Some(mod_b.local_id), |&m| def_map[m].parent);
149167

150168
if a_ancestors.any(|m| m == mod_b.local_id) {
151169
// B is above A
152170
return Some(Visibility::Module(mod_b, expl_b));
153171
}
154172

173+
let mut b_ancestors =
174+
iter::successors(Some(mod_b.local_id), |&m| def_map[m].parent);
155175
if b_ancestors.any(|m| m == mod_a.local_id) {
156176
// A is above B
157177
return Some(Visibility::Module(mod_a, expl_a));
@@ -169,26 +189,37 @@ impl Visibility {
169189
pub(crate) fn min(self, other: Visibility, def_map: &DefMap) -> Option<Visibility> {
170190
match (self, other) {
171191
(vis, Visibility::Public) | (Visibility::Public, vis) => Some(vis),
192+
(Visibility::PubCrate(krate), Visibility::PubCrate(krateb)) => {
193+
if krate == krateb {
194+
Some(Visibility::PubCrate(krate))
195+
} else {
196+
None
197+
}
198+
}
199+
(Visibility::Module(mod_, exp), Visibility::PubCrate(krate))
200+
| (Visibility::PubCrate(krate), Visibility::Module(mod_, exp)) => {
201+
if mod_.krate == krate { Some(Visibility::Module(mod_, exp)) } else { None }
202+
}
172203
(Visibility::Module(mod_a, expl_a), Visibility::Module(mod_b, expl_b)) => {
173-
if mod_a.krate != mod_b.krate {
204+
if mod_a.krate != def_map.krate() || mod_b.krate != def_map.krate() {
174205
return None;
175206
}
176207

177208
let def_block = def_map.block_id();
178-
if (mod_a.containing_block(), mod_b.containing_block()) != (def_block, def_block) {
209+
if mod_a.containing_block() != def_block || mod_b.containing_block() != def_block {
179210
return None;
180211
}
181212

182213
let mut a_ancestors =
183214
iter::successors(Some(mod_a.local_id), |&m| def_map[m].parent);
184-
let mut b_ancestors =
185-
iter::successors(Some(mod_b.local_id), |&m| def_map[m].parent);
186215

187216
if a_ancestors.any(|m| m == mod_b.local_id) {
188217
// B is above A
189218
return Some(Visibility::Module(mod_a, expl_a));
190219
}
191220

221+
let mut b_ancestors =
222+
iter::successors(Some(mod_b.local_id), |&m| def_map[m].parent);
192223
if b_ancestors.any(|m| m == mod_a.local_id) {
193224
// A is above B
194225
return Some(Visibility::Module(mod_b, expl_b));

0 commit comments

Comments
 (0)