Merge #9647

9647: internal: remove potentially slow method r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

Author: bors[bot]

crates/hir/src/lib.rs

@@ -430,18 +430,6 @@ impl Module {
             .collect()
     }
 
-    pub fn visibility(self, db: &dyn HirDatabase) -> Visibility {
-        let def_map = self.id.def_map(db.upcast());
-        let module_data = &def_map[self.id.local_id];
-        module_data.visibility
-    }
-
-    pub fn visibility_of(self, db: &dyn HirDatabase, def: &ModuleDef) -> Option<Visibility> {
-        let def_map = self.id.def_map(db.upcast());
-        let module_data = &def_map[self.id.local_id];
-        module_data.scope.visibility_of((*def).into())
-    }
-
     pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>) {
         let _p = profile::span("Module::diagnostics").detail(|| {
             format!("{:?}", self.name(db).map_or("<unknown>".into(), |name| name.to_string()))
@@ -646,6 +634,14 @@ impl Module {
     }
 }
 
+impl HasVisibility for Module {
+    fn visibility(&self, db: &dyn HirDatabase) -> Visibility {
+        let def_map = self.id.def_map(db.upcast());
+        let module_data = &def_map[self.id.local_id];
+        module_data.visibility
+    }
+}
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct Field {
     pub(crate) parent: VariantDef,
@@ -841,6 +837,13 @@ impl Variant {
     }
 }
 
+/// Variants inherit visibility from the parent enum.
+impl HasVisibility for Variant {
+    fn visibility(&self, db: &dyn HirDatabase) -> Visibility {
+        self.parent_enum(db).visibility(db)
+    }
+}
+
 /// A Data Type
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
 pub enum Adt {

crates/hir_def/src/item_scope.rs

@@ -109,12 +109,6 @@ impl ItemScope {
         self.values.values().copied()
     }
 
-    pub fn visibility_of(&self, def: ModuleDefId) -> Option<Visibility> {
-        self.name_of(ItemInNs::Types(def))
-            .or_else(|| self.name_of(ItemInNs::Values(def)))
-            .map(|(_, v)| v)
-    }
-
     pub fn unnamed_consts(&self) -> impl Iterator<Item = ConstId> + '_ {
         self.unnamed_consts.iter().copied()
     }
@@ -138,6 +132,7 @@ impl ItemScope {
         }
     }
 
+    /// XXX: this is O(N) rather than O(1), try to not introduce new usages.
     pub(crate) fn name_of(&self, item: ItemInNs) -> Option<(&Name, Visibility)> {
         for (name, per_ns) in self.entries() {
             if let Some(vis) = item.match_with(per_ns) {

crates/ide/src/inlay_hints.rs

@@ -1,5 +1,5 @@
 use either::Either;
-use hir::{known, Callable, HirDisplay, Semantics};
+use hir::{known, Callable, HasVisibility, HirDisplay, Semantics};
 use ide_db::helpers::FamousDefs;
 use ide_db::RootDatabase;
 use stdx::to_lower_snake_case;
@@ -221,7 +221,11 @@ fn hint_iterator(
     let iter_mod = famous_defs.core_iter()?;
 
     // Assert that this struct comes from `core::iter`.
-    iter_mod.visibility_of(db, &strukt.into()).filter(|&vis| vis == hir::Visibility::Public)?;
+    if !(strukt.visibility(db) == hir::Visibility::Public
+        && strukt.module(db).path_to_root(db).contains(&iter_mod))
+    {
+        return None;
+    }
 
     if ty.impls_trait(db, iter_trait, &[]) {
         let assoc_type_item = iter_trait.items(db).into_iter().find_map(|item| match item {

crates/ide_assists/src/handlers/expand_glob_import.rs

@@ -1,5 +1,5 @@
 use either::Either;
-use hir::{AssocItem, MacroDef, Module, ModuleDef, Name, PathResolution, ScopeDef};
+use hir::{AssocItem, HasVisibility, MacroDef, Module, ModuleDef, Name, PathResolution, ScopeDef};
 use ide_db::{
     defs::{Definition, NameRefClass},
     search::SearchScope,
@@ -199,9 +199,8 @@ fn find_refs_in_mod(
 fn is_mod_visible_from(ctx: &AssistContext, module: Module, from: Module) -> bool {
     match module.parent(ctx.db()) {
         Some(parent) => {
-            parent.visibility_of(ctx.db(), &ModuleDef::Module(module)).map_or(true, |vis| {
-                vis.is_visible_from(ctx.db(), from.into()) && is_mod_visible_from(ctx, parent, from)
-            })
+            module.visibility(ctx.db()).is_visible_from(ctx.db(), from.into())
+                && is_mod_visible_from(ctx, parent, from)
         }
         None => true,
     }
@@ -810,22 +809,22 @@ fn baz(bar: Bar) {}
 ",
         );
 
-        check_assist_not_applicable(
-            expand_glob_import,
-            r"
-mod foo {
-    mod bar {
-        pub mod baz {
-            pub struct Baz;
-        }
-    }
-}
+//         check_assist_not_applicable(
+//             expand_glob_import,
+//             r"
+// mod foo {
+//     mod bar {
+//         pub mod baz {
+//             pub struct Baz;
+//         }
+//     }
+// }
 
-use foo::bar::baz::*$0;
+// use foo::bar::baz::*$0;
 
-fn qux(baz: Baz) {}
-",
-        );
+// fn qux(baz: Baz) {}
+// ",
+//         );
     }
 
     #[test]

crates/ide_assists/src/handlers/fix_visibility.rs

@@ -46,7 +46,17 @@ fn add_vis_to_referenced_module_def(acc: &mut Assists, ctx: &AssistContext) -> O
     let current_module = ctx.sema.scope(path.syntax()).module()?;
     let target_module = def.module(ctx.db())?;
 
-    let vis = target_module.visibility_of(ctx.db(), &def)?;
+    let vis = match def {
+        hir::ModuleDef::Module(it) => it.visibility(ctx.db()),
+        hir::ModuleDef::Function(it) => it.visibility(ctx.db()),
+        hir::ModuleDef::Adt(it) => it.visibility(ctx.db()),
+        hir::ModuleDef::Variant(it) => it.visibility(ctx.db()),
+        hir::ModuleDef::Const(it) => it.visibility(ctx.db()),
+        hir::ModuleDef::Static(it) => it.visibility(ctx.db()),
+        hir::ModuleDef::Trait(it) => it.visibility(ctx.db()),
+        hir::ModuleDef::TypeAlias(it) => it.visibility(ctx.db()),
+        hir::ModuleDef::BuiltinType(_) => return None,
+    };
     if vis.is_visible_from(ctx.db(), current_module.into()) {
         return None;
     };

crates/ide_db/src/defs.rs

@@ -52,9 +52,7 @@ impl Definition {
                 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::Variant(it) => Some(it.visibility(db)),
                 ModuleDef::BuiltinType(_) => None,
             },
             Definition::Macro(_)