Merge #3234

3234: Don't store deriveable Module info in NameDefinition r=matklad a=matklad



bors r+
🤖

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

4 files changed, 84 insertions, 60 deletions

diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs
index 1bdcda069..efc3502d0 100644
--- a/crates/ra_hir/src/code_model.rs
+++ b/crates/ra_hir/src/code_model.rs
@@ -30,6 +30,7 @@ use ra_syntax::{
     ast::{self, AttrsOwner},
     AstNode,
 };
+use rustc_hash::FxHashSet;
 
 use crate::{
     db::{DefDatabase, HirDatabase},
@@ -123,10 +124,25 @@ impl_froms!(
     BuiltinType
 );
 
+impl ModuleDef {
+    pub fn module(self, db: &impl HirDatabase) -> Option<Module> {
+        match self {
+            ModuleDef::Module(it) => it.parent(db),
+            ModuleDef::Function(it) => Some(it.module(db)),
+            ModuleDef::Adt(it) => Some(it.module(db)),
+            ModuleDef::EnumVariant(it) => Some(it.module(db)),
+            ModuleDef::Const(it) => Some(it.module(db)),
+            ModuleDef::Static(it) => Some(it.module(db)),
+            ModuleDef::Trait(it) => Some(it.module(db)),
+            ModuleDef::TypeAlias(it) => Some(it.module(db)),
+            ModuleDef::BuiltinType(_) => None,
+        }
+    }
+}
+
 pub use hir_def::{
     attr::Attrs, item_scope::ItemInNs, visibility::Visibility, AssocItemId, AssocItemLoc,
 };
-use rustc_hash::FxHashSet;
 
 impl Module {
     pub(crate) fn new(krate: Crate, crate_module_id: LocalModuleId) -> Module {
@@ -649,6 +665,17 @@ pub struct MacroDef {
     pub(crate) id: MacroDefId,
 }
 
+impl MacroDef {
+    /// FIXME: right now, this just returns the root module of the crate that
+    /// defines this macro. The reasons for this is that macros are expanded
+    /// early, in `ra_hir_expand`, where modules simply do not exist yet.
+    pub fn module(self, db: &impl HirDatabase) -> Option<Module> {
+        let krate = self.id.krate?;
+        let module_id = db.crate_def_map(krate).root;
+        Some(Module::new(Crate { id: krate }, module_id))
+    }
+}
+
 /// Invariant: `inner.as_assoc_item(db).is_some()`
 /// We do not actively enforce this invariant.
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
diff --git a/crates/ra_ide/src/references/classify.rs b/crates/ra_ide/src/references/classify.rs
index d0f03d8a8..2394e68d1 100644
--- a/crates/ra_ide/src/references/classify.rs
+++ b/crates/ra_ide/src/references/classify.rs
@@ -22,7 +22,7 @@ pub(crate) fn classify_name_ref(
     if let Some(method_call) = ast::MethodCallExpr::cast(parent.clone()) {
         tested_by!(goto_def_for_methods);
         if let Some(func) = analyzer.resolve_method_call(&method_call) {
-            return Some(from_module_def(sb.db, func.into(), None));
+            return Some(from_module_def(sb.db, func.into()));
         }
     }
 
@@ -43,7 +43,6 @@ pub(crate) fn classify_name_ref(
 
     // FIXME: find correct container and visibility for each case
     let visibility = None;
-    let container = sb.to_module_def(name_ref.file_id.original_file(sb.db))?;
 
     if let Some(macro_call) = parent.ancestors().find_map(ast::MacroCall::cast) {
         tested_by!(goto_def_for_macros);
@@ -51,40 +50,37 @@ pub(crate) fn classify_name_ref(
             analyzer.resolve_macro_call(sb.db, name_ref.with_value(&macro_call))
         {
             let kind = NameKind::Macro(macro_def);
-            return Some(NameDefinition { kind, container, visibility });
+            return Some(NameDefinition { kind, visibility });
         }
     }
 
     let path = name_ref.value.syntax().ancestors().find_map(ast::Path::cast)?;
     let resolved = analyzer.resolve_path(sb.db, &path)?;
     let res = match resolved {
-        PathResolution::Def(def) => from_module_def(sb.db, def, Some(container)),
+        PathResolution::Def(def) => from_module_def(sb.db, def),
         PathResolution::AssocItem(item) => {
             let def = match item {
                 hir::AssocItem::Function(it) => it.into(),
                 hir::AssocItem::Const(it) => it.into(),
                 hir::AssocItem::TypeAlias(it) => it.into(),
             };
-            from_module_def(sb.db, def, Some(container))
+            from_module_def(sb.db, def)
         }
         PathResolution::Local(local) => {
             let kind = NameKind::Local(local);
-            let container = local.module(sb.db);
-            NameDefinition { kind, container, visibility: None }
+            NameDefinition { kind, visibility: None }
         }
         PathResolution::TypeParam(par) => {
             let kind = NameKind::TypeParam(par);
-            let container = par.module(sb.db);
-            NameDefinition { kind, container, visibility }
+            NameDefinition { kind, visibility }
         }
         PathResolution::Macro(def) => {
             let kind = NameKind::Macro(def);
-            NameDefinition { kind, container, visibility }
+            NameDefinition { kind, visibility }
         }
         PathResolution::SelfType(impl_block) => {
             let kind = NameKind::SelfType(impl_block);
-            let container = impl_block.module(sb.db);
-            NameDefinition { kind, container, visibility }
+            NameDefinition { kind, visibility }
         }
     };
     Some(res)
diff --git a/crates/ra_ide/src/references/search_scope.rs b/crates/ra_ide/src/references/search_scope.rs
index 279f57be0..f9bbe64a4 100644
--- a/crates/ra_ide/src/references/search_scope.rs
+++ b/crates/ra_ide/src/references/search_scope.rs
@@ -19,10 +19,17 @@ pub struct SearchScope {
 }
 
 impl SearchScope {
+    fn empty() -> SearchScope {
+        SearchScope { entries: FxHashMap::default() }
+    }
+
     pub(crate) fn for_def(def: &NameDefinition, db: &RootDatabase) -> SearchScope {
         let _p = profile("search_scope");
-
-        let module_src = def.container.definition_source(db);
+        let module = match def.module(db) {
+            Some(it) => it,
+            None => return SearchScope::empty(),
+        };
+        let module_src = module.definition_source(db);
         let file_id = module_src.file_id.original_file(db);
 
         if let NameKind::Local(var) = def.kind {
@@ -39,7 +46,7 @@ impl SearchScope {
         let vis = def.visibility.as_ref().map(|v| v.syntax().to_string()).unwrap_or_default();
 
         if vis.as_str() == "pub(super)" {
-            if let Some(parent_module) = def.container.parent(db) {
+            if let Some(parent_module) = module.parent(db) {
                 let mut res = FxHashMap::default();
                 let parent_src = parent_module.definition_source(db);
                 let file_id = parent_src.file_id.original_file(db);
@@ -72,7 +79,7 @@ impl SearchScope {
                 return SearchScope::new(res);
             }
             if vis.as_str() == "pub" {
-                let krate = def.container.krate();
+                let krate = module.krate();
                 for rev_dep in krate.reverse_dependencies(db) {
                     let root_file = rev_dep.root_file(db);
                     let source_root_id = db.file_source_root(root_file);
diff --git a/crates/ra_ide_db/src/defs.rs b/crates/ra_ide_db/src/defs.rs
index 030f44f86..cc772de51 100644
--- a/crates/ra_ide_db/src/defs.rs
+++ b/crates/ra_ide_db/src/defs.rs
@@ -32,10 +32,22 @@ pub struct NameDefinition {
     pub visibility: Option<ast::Visibility>,
     /// FIXME: this doesn't really make sense. For example, builtin types don't
     /// really have a module.
-    pub container: Module,
     pub kind: NameKind,
 }
 
+impl NameDefinition {
+    pub fn module(&self, db: &RootDatabase) -> Option<Module> {
+        match self.kind {
+            NameKind::Macro(it) => it.module(db),
+            NameKind::StructField(it) => Some(it.parent_def(db).module(db)),
+            NameKind::ModuleDef(it) => it.module(db),
+            NameKind::SelfType(it) => Some(it.module(db)),
+            NameKind::Local(it) => Some(it.module(db)),
+            NameKind::TypeParam(it) => Some(it.module(db)),
+        }
+    }
+}
+
 pub fn classify_name(
     sb: &mut SourceBinder<RootDatabase>,
     name: InFile<&ast::Name>,
@@ -50,7 +62,6 @@ pub fn classify_name(
                 let local = sb.to_def(src)?;
                 Some(NameDefinition {
                     visibility: None,
-                    container: local.module(sb.db),
                     kind: NameKind::Local(local),
                 })
             },
@@ -61,57 +72,54 @@ pub fn classify_name(
             },
             ast::Module(it) => {
                 let def = sb.to_def(name.with_value(it))?;
-                Some(from_module_def(sb.db, def.into(), None))
+                Some(from_module_def(sb.db, def.into()))
             },
             ast::StructDef(it) => {
                 let src = name.with_value(it);
                 let def: hir::Struct = sb.to_def(src)?;
-                Some(from_module_def(sb.db, def.into(), None))
+                Some(from_module_def(sb.db, def.into()))
             },
             ast::EnumDef(it) => {
                 let src = name.with_value(it);
                 let def: hir::Enum = sb.to_def(src)?;
-                Some(from_module_def(sb.db, def.into(), None))
+                Some(from_module_def(sb.db, def.into()))
             },
             ast::TraitDef(it) => {
                 let src = name.with_value(it);
                 let def: hir::Trait = sb.to_def(src)?;
-                Some(from_module_def(sb.db, def.into(), None))
+                Some(from_module_def(sb.db, def.into()))
             },
             ast::StaticDef(it) => {
                 let src = name.with_value(it);
                 let def: hir::Static = sb.to_def(src)?;
-                Some(from_module_def(sb.db, def.into(), None))
+                Some(from_module_def(sb.db, def.into()))
             },
             ast::EnumVariant(it) => {
                 let src = name.with_value(it);
                 let def: hir::EnumVariant = sb.to_def(src)?;
-                Some(from_module_def(sb.db, def.into(), None))
+                Some(from_module_def(sb.db, def.into()))
             },
             ast::FnDef(it) => {
                 let src = name.with_value(it);
                 let def: hir::Function = sb.to_def(src)?;
-                Some(from_module_def(sb.db, def.into(), None))
+                Some(from_module_def(sb.db, def.into()))
             },
             ast::ConstDef(it) => {
                 let src = name.with_value(it);
                 let def: hir::Const = sb.to_def(src)?;
-                Some(from_module_def(sb.db, def.into(), None))
+                Some(from_module_def(sb.db, def.into()))
             },
             ast::TypeAliasDef(it) => {
                 let src = name.with_value(it);
                 let def: hir::TypeAlias = sb.to_def(src)?;
-                Some(from_module_def(sb.db, def.into(), None))
+                Some(from_module_def(sb.db, def.into()))
             },
             ast::MacroCall(it) => {
                 let src = name.with_value(it);
                 let def = sb.to_def(src.clone())?;
 
-                let module = sb.to_module_def(src.file_id.original_file(sb.db))?;
-
                 Some(NameDefinition {
                     visibility: None,
-                    container: module,
                     kind: NameKind::Macro(def),
                 })
             },
@@ -120,7 +128,6 @@ pub fn classify_name(
                 let def = sb.to_def(src)?;
                 Some(NameDefinition {
                     visibility: None,
-                    container: def.module(sb.db),
                     kind: NameKind::TypeParam(def),
                 })
             },
@@ -132,41 +139,28 @@ pub fn classify_name(
 pub fn from_struct_field(db: &RootDatabase, field: StructField) -> NameDefinition {
     let kind = NameKind::StructField(field);
     let parent = field.parent_def(db);
-    let container = parent.module(db);
     let visibility = match parent {
         VariantDef::Struct(s) => s.source(db).value.visibility(),
         VariantDef::Union(e) => e.source(db).value.visibility(),
         VariantDef::EnumVariant(e) => e.source(db).value.parent_enum().visibility(),
     };
-    NameDefinition { kind, container, visibility }
+    NameDefinition { kind, visibility }
 }
 
-pub fn from_module_def(
-    db: &RootDatabase,
-    def: ModuleDef,
-    module: Option<Module>,
-) -> NameDefinition {
+pub fn from_module_def(db: &RootDatabase, def: ModuleDef) -> NameDefinition {
     let kind = NameKind::ModuleDef(def);
-    let (container, visibility) = match def {
-        ModuleDef::Module(it) => {
-            let container = it.parent(db).or_else(|| Some(it)).unwrap();
-            let visibility = it.declaration_source(db).and_then(|s| s.value.visibility());
-            (container, visibility)
-        }
-        ModuleDef::EnumVariant(it) => {
-            let container = it.module(db);
-            let visibility = it.source(db).value.parent_enum().visibility();
-            (container, visibility)
-        }
-        ModuleDef::Function(it) => (it.module(db), it.source(db).value.visibility()),
-        ModuleDef::Const(it) => (it.module(db), it.source(db).value.visibility()),
-        ModuleDef::Static(it) => (it.module(db), it.source(db).value.visibility()),
-        ModuleDef::Trait(it) => (it.module(db), it.source(db).value.visibility()),
-        ModuleDef::TypeAlias(it) => (it.module(db), it.source(db).value.visibility()),
-        ModuleDef::Adt(Adt::Struct(it)) => (it.module(db), it.source(db).value.visibility()),
-        ModuleDef::Adt(Adt::Union(it)) => (it.module(db), it.source(db).value.visibility()),
-        ModuleDef::Adt(Adt::Enum(it)) => (it.module(db), it.source(db).value.visibility()),
-        ModuleDef::BuiltinType(..) => (module.unwrap(), None),
+    let visibility = match def {
+        ModuleDef::Module(it) => it.declaration_source(db).and_then(|s| s.value.visibility()),
+        ModuleDef::EnumVariant(it) => it.source(db).value.parent_enum().visibility(),
+        ModuleDef::Function(it) => it.source(db).value.visibility(),
+        ModuleDef::Const(it) => it.source(db).value.visibility(),
+        ModuleDef::Static(it) => it.source(db).value.visibility(),
+        ModuleDef::Trait(it) => it.source(db).value.visibility(),
+        ModuleDef::TypeAlias(it) => it.source(db).value.visibility(),
+        ModuleDef::Adt(Adt::Struct(it)) => it.source(db).value.visibility(),
+        ModuleDef::Adt(Adt::Union(it)) => it.source(db).value.visibility(),
+        ModuleDef::Adt(Adt::Enum(it)) => it.source(db).value.visibility(),
+        ModuleDef::BuiltinType(..) => None,
     };
-    NameDefinition { kind, container, visibility }
+    NameDefinition { kind, visibility }
 }