From 7be2d2f008595114e19aad0aa0608ddedfe3edf7 Mon Sep 17 00:00:00 2001
From: Aleksey Kladov <aleksey.kladov@gmail.com>
Date: Tue, 22 Jun 2021 21:22:36 +0300
Subject: internal: remove one more accidentally quadratic code-path

Definition::visibility was implemented in a rather roundabout way -- by
asking the parent module about the effective visibility.

This is problematic for a couple of reasons:

* first, it doesn't work for local items
* second, asking module about visibility of a child is a linear
  operation (that's a problem in itself, tracked in #9378)

Instead, lets ask the declared visibility directly, we have all the code
for it, and need only to actually us it.
---
 crates/hir/src/lib.rs     | 30 ++++++++++--------------------
 crates/ide_db/src/defs.rs | 28 ++++++++++++++++++++++------
 2 files changed, 32 insertions(+), 26 deletions(-)

(limited to 'crates')

diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 9c56fe1a6..3f0000889 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -303,26 +303,6 @@ impl ModuleDef {
         Some(segments.join("::"))
     }
 
-    pub fn definition_visibility(&self, db: &dyn HirDatabase) -> Option<Visibility> {
-        let module = match self {
-            ModuleDef::Module(it) => it.parent(db)?,
-            ModuleDef::Function(it) => return Some(it.visibility(db)),
-            ModuleDef::Adt(it) => it.module(db),
-            ModuleDef::Variant(it) => {
-                let parent = it.parent_enum(db);
-                let module = it.module(db);
-                return module.visibility_of(db, &ModuleDef::Adt(Adt::Enum(parent)));
-            }
-            ModuleDef::Const(it) => return Some(it.visibility(db)),
-            ModuleDef::Static(it) => it.module(db),
-            ModuleDef::Trait(it) => it.module(db),
-            ModuleDef::TypeAlias(it) => return Some(it.visibility(db)),
-            ModuleDef::BuiltinType(_) => return None,
-        };
-
-        module.visibility_of(db, self)
-    }
-
     pub fn name(self, db: &dyn HirDatabase) -> Option<Name> {
         match self {
             ModuleDef::Adt(it) => Some(it.name(db)),
@@ -893,6 +873,16 @@ impl Adt {
     }
 }
 
+impl HasVisibility for Adt {
+    fn visibility(&self, db: &dyn HirDatabase) -> Visibility {
+        match self {
+            Adt::Struct(it) => it.visibility(db),
+            Adt::Union(it) => it.visibility(db),
+            Adt::Enum(it) => it.visibility(db),
+        }
+    }
+}
+
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
 pub enum VariantDef {
     Struct(Struct),
diff --git a/crates/ide_db/src/defs.rs b/crates/ide_db/src/defs.rs
index a54f2c323..fddd1fc2d 100644
--- a/crates/ide_db/src/defs.rs
+++ b/crates/ide_db/src/defs.rs
@@ -43,13 +43,29 @@ impl Definition {
 
     pub fn visibility(&self, db: &RootDatabase) -> Option<Visibility> {
         match self {
-            Definition::Macro(_) => None,
             Definition::Field(sf) => Some(sf.visibility(db)),
-            Definition::ModuleDef(def) => def.definition_visibility(db),
-            Definition::SelfType(_) => None,
-            Definition::Local(_) => None,
-            Definition::GenericParam(_) => None,
-            Definition::Label(_) => None,
+            Definition::ModuleDef(def) => match def {
+                ModuleDef::Module(it) => {
+                    // FIXME: should work like other cases here.
+                    let parent = it.parent(db)?;
+                    parent.visibility_of(db, def)
+                }
+                ModuleDef::Function(it) => Some(it.visibility(db)),
+                ModuleDef::Adt(it) => Some(it.visibility(db)),
+                ModuleDef::Const(it) => Some(it.visibility(db)),
+                ModuleDef::Static(it) => Some(it.visibility(db)),
+                ModuleDef::Trait(it) => Some(it.visibility(db)),
+                ModuleDef::TypeAlias(it) => Some(it.visibility(db)),
+                // NB: Variants don't have their own visibility, and just inherit
+                // one from the parent. Not sure if that's the right thing to do.
+                ModuleDef::Variant(it) => Some(it.parent_enum(db).visibility(db)),
+                ModuleDef::BuiltinType(_) => None,
+            },
+            Definition::Macro(_)
+            | Definition::SelfType(_)
+            | Definition::Local(_)
+            | Definition::GenericParam(_)
+            | Definition::Label(_) => None,
         }
     }
 
-- 
cgit v1.2.3