about summary refs log tree commit diff
path: root/src/tools
diff options
context:
space:
mode:
authorLukas Wirth <me@lukaswirth.dev>2025-06-15 08:36:27 +0000
committerGitHub <noreply@github.com>2025-06-15 08:36:27 +0000
commite557333b035eeb407ab0791927d137a5c2140c22 (patch)
tree7b56c63e89cc87e3f5008bd330ba55fcea6bd3ad /src/tools
parentbad1c639a5b471241a819386d2d5072175e4d524 (diff)
parent6ff82fbba606b58a58a24daa82ee6f6c320c9170 (diff)
downloadrust-e557333b035eeb407ab0791927d137a5c2140c22.tar.gz
rust-e557333b035eeb407ab0791927d137a5c2140c22.zip
Merge pull request #20009 from Veykril/push-rvnnorzvpnqv
Optimize `pub(crate)` and `pub(self)` visibility resolution
Diffstat (limited to 'src/tools')
-rw-r--r--src/tools/rust-analyzer/crates/hir-def/src/expr_store/pretty.rs4
-rw-r--r--src/tools/rust-analyzer/crates/hir-def/src/expr_store/tests/body/block.rs2
-rw-r--r--src/tools/rust-analyzer/crates/hir-def/src/find_path.rs2
-rw-r--r--src/tools/rust-analyzer/crates/hir-def/src/item_scope.rs22
-rw-r--r--src/tools/rust-analyzer/crates/hir-def/src/item_tree.rs33
-rw-r--r--src/tools/rust-analyzer/crates/hir-def/src/item_tree/pretty.rs4
-rw-r--r--src/tools/rust-analyzer/crates/hir-def/src/item_tree/tests.rs2
-rw-r--r--src/tools/rust-analyzer/crates/hir-def/src/lib.rs4
-rw-r--r--src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs3
-rw-r--r--src/tools/rust-analyzer/crates/hir-def/src/nameres/path_resolution.rs37
-rw-r--r--src/tools/rust-analyzer/crates/hir-def/src/resolver.rs4
-rw-r--r--src/tools/rust-analyzer/crates/hir-def/src/visibility.rs90
-rw-r--r--src/tools/rust-analyzer/crates/hir-ty/src/display.rs1
-rw-r--r--src/tools/rust-analyzer/crates/hir/src/symbols.rs1
14 files changed, 136 insertions, 73 deletions
diff --git a/src/tools/rust-analyzer/crates/hir-def/src/expr_store/pretty.rs b/src/tools/rust-analyzer/crates/hir-def/src/expr_store/pretty.rs
index 7b452721dfe..56c7655f9ea 100644
--- a/src/tools/rust-analyzer/crates/hir-def/src/expr_store/pretty.rs
+++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/hir-def/src/expr_store/tests/body/block.rs b/src/tools/rust-analyzer/crates/hir-def/src/expr_store/tests/body/block.rs
index 5fe72778172..bb0b70bc5bf 100644
--- a/src/tools/rust-analyzer/crates/hir-def/src/expr_store/tests/body/block.rs
+++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/hir-def/src/find_path.rs b/src/tools/rust-analyzer/crates/hir-def/src/find_path.rs
index 6ebc0d8dfd9..dccfff002f2 100644
--- a/src/tools/rust-analyzer/crates/hir-def/src/find_path.rs
+++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/hir-def/src/item_scope.rs b/src/tools/rust-analyzer/crates/hir-def/src/item_scope.rs
index 8591874eb94..efa43994685 100644
--- a/src/tools/rust-analyzer/crates/hir-def/src/item_scope.rs
+++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/hir-def/src/item_tree.rs b/src/tools/rust-analyzer/crates/hir-def/src/item_tree.rs
index f61b403b84e..c6333398574 100644
--- a/src/tools/rust-analyzer/crates/hir-def/src/item_tree.rs
+++ b/src/tools/rust-analyzer/crates/hir-def/src/item_tree.rs
@@ -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,
 }
diff --git a/src/tools/rust-analyzer/crates/hir-def/src/item_tree/pretty.rs b/src/tools/rust-analyzer/crates/hir-def/src/item_tree/pretty.rs
index eb97081128e..696174cb072 100644
--- a/src/tools/rust-analyzer/crates/hir-def/src/item_tree/pretty.rs
+++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/hir-def/src/item_tree/tests.rs b/src/tools/rust-analyzer/crates/hir-def/src/item_tree/tests.rs
index e39efd31c6c..5923b3ea491 100644
--- a/src/tools/rust-analyzer/crates/hir-def/src/item_tree/tests.rs
+++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/hir-def/src/lib.rs b/src/tools/rust-analyzer/crates/hir-def/src/lib.rs
index 0cd7e894327..a542214d303 100644
--- a/src/tools/rust-analyzer/crates/hir-def/src/lib.rs
+++ b/src/tools/rust-analyzer/crates/hir-def/src/lib.rs
@@ -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,
 }
diff --git a/src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs b/src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs
index 508fcc2fa51..78fdc275606 100644
--- a/src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs
+++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/hir-def/src/nameres/path_resolution.rs b/src/tools/rust-analyzer/crates/hir-def/src/nameres/path_resolution.rs
index ca39173d477..e8235b1c96f 100644
--- a/src/tools/rust-analyzer/crates/hir-def/src/nameres/path_resolution.rs
+++ b/src/tools/rust-analyzer/crates/hir-def/src/nameres/path_resolution.rs
@@ -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)
     }
 
diff --git a/src/tools/rust-analyzer/crates/hir-def/src/resolver.rs b/src/tools/rust-analyzer/crates/hir-def/src/resolver.rs
index 491b6204bce..6f321980af4 100644
--- a/src/tools/rust-analyzer/crates/hir-def/src/resolver.rs
+++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/hir-def/src/visibility.rs b/src/tools/rust-analyzer/crates/hir-def/src/visibility.rs
index aea2ac550e3..2514e88864f 100644
--- a/src/tools/rust-analyzer/crates/hir-def/src/visibility.rs
+++ b/src/tools/rust-analyzer/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<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));
diff --git a/src/tools/rust-analyzer/crates/hir-ty/src/display.rs b/src/tools/rust-analyzer/crates/hir-ty/src/display.rs
index da21f08a1e1..1aa7e0fcf88 100644
--- a/src/tools/rust-analyzer/crates/hir-ty/src/display.rs
+++ b/src/tools/rust-analyzer/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/src/tools/rust-analyzer/crates/hir/src/symbols.rs b/src/tools/rust-analyzer/crates/hir/src/symbols.rs
index cb9901ab11b..64f2a910bd1 100644
--- a/src/tools/rust-analyzer/crates/hir/src/symbols.rs
+++ b/src/tools/rust-analyzer/crates/hir/src/symbols.rs
@@ -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,
         };