Merge #623

623: WIP: module id is not def id r=matklad a=matklad

This achieves two things:

* makes module_tree & item_map per crate, not per source_root
* begins the refactoring to remove universal `DefId` in favor of having separate ids for each kind of `Def`. Currently, only modules get a differnt ID though. 

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

2 files changed, 88 insertions, 125 deletions

diff --git a/crates/ra_hir/src/nameres/lower.rs b/crates/ra_hir/src/nameres/lower.rs
index 4eea6ff1d..b4fe99ea7 100644
--- a/crates/ra_hir/src/nameres/lower.rs
+++ b/crates/ra_hir/src/nameres/lower.rs
@@ -1,17 +1,17 @@
 use std::sync::Arc;
 
 use ra_syntax::{
-    SyntaxKind, AstNode, SourceFile, TreeArc, AstPtr,
+    AstNode, SourceFile, TreeArc, AstPtr,
     ast::{self, ModuleItemOwner, NameOwner},
 };
-use ra_db::SourceRootId;
 use ra_arena::{Arena, RawId, impl_arena_id, map::ArenaMap};
 use rustc_hash::FxHashMap;
 
 use crate::{
-    SourceItemId, Path, ModuleSource, HirDatabase, Name, SourceFileItems,
-    HirFileId, MacroCallLoc, AsName, PerNs, DefId, DefKind, DefLoc,
-    module_tree::ModuleId
+    SourceItemId, Path, ModuleSource, HirDatabase, Name,
+    HirFileId, MacroCallLoc, AsName, PerNs, Function,
+    ModuleDef, Module, Struct, Enum, Const, Static, Trait, Type,
+    ids::LocationCtx,
 };
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -32,7 +32,7 @@ pub(super) struct ImportData {
 /// can avoid redoing name resolution.
 #[derive(Debug, Default, PartialEq, Eq)]
 pub struct LoweredModule {
-    pub(crate) declarations: FxHashMap<Name, PerNs<DefId>>,
+    pub(crate) declarations: FxHashMap<Name, PerNs<ModuleDef>>,
     pub(super) imports: Arena<ImportId, ImportData>,
 }
 
@@ -59,37 +59,31 @@ impl ImportSourceMap {
 impl LoweredModule {
     pub(crate) fn lower_module_module_query(
         db: &impl HirDatabase,
-        source_root_id: SourceRootId,
-        module_id: ModuleId,
+        module: Module,
     ) -> Arc<LoweredModule> {
-        db.lower_module(source_root_id, module_id).0
+        db.lower_module(module).0
     }
 
     pub(crate) fn lower_module_source_map_query(
         db: &impl HirDatabase,
-        source_root_id: SourceRootId,
-        module_id: ModuleId,
+        module: Module,
     ) -> Arc<ImportSourceMap> {
-        db.lower_module(source_root_id, module_id).1
+        db.lower_module(module).1
     }
 
     pub(crate) fn lower_module_query(
         db: &impl HirDatabase,
-        source_root_id: SourceRootId,
-        module_id: ModuleId,
+        module: Module,
     ) -> (Arc<LoweredModule>, Arc<ImportSourceMap>) {
-        let module_tree = db.module_tree(source_root_id);
-        let source = module_id.source(&module_tree);
-        let file_id = source.file_id;
-        let source = ModuleSource::from_source_item_id(db, source);
+        let (file_id, source) = module.definition_source(db);
+        let file_id: HirFileId = file_id.into();
         let mut source_map = ImportSourceMap::default();
         let mut res = LoweredModule::default();
         match source {
             ModuleSource::SourceFile(it) => res.fill(
                 &mut source_map,
                 db,
-                source_root_id,
-                module_id,
+                module,
                 file_id,
                 &mut it.items_with_macros(),
             ),
@@ -98,8 +92,7 @@ impl LoweredModule {
                     res.fill(
                         &mut source_map,
                         db,
-                        source_root_id,
-                        module_id,
+                        module,
                         file_id,
                         &mut item_list.items_with_macros(),
                     )
@@ -113,8 +106,7 @@ impl LoweredModule {
         &mut self,
         source_map: &mut ImportSourceMap,
         db: &impl HirDatabase,
-        source_root_id: SourceRootId,
-        module_id: ModuleId,
+        module: Module,
         file_id: HirFileId,
         items: &mut Iterator<Item = ast::ItemOrMacro>,
     ) {
@@ -123,21 +115,12 @@ impl LoweredModule {
         for item in items {
             match item {
                 ast::ItemOrMacro::Item(it) => {
-                    self.add_def_id(
-                        source_map,
-                        db,
-                        source_root_id,
-                        module_id,
-                        file_id,
-                        &file_items,
-                        it,
-                    );
+                    self.add_def_id(source_map, db, module, file_id, it);
                 }
                 ast::ItemOrMacro::Macro(macro_call) => {
                     let item_id = file_items.id_of_unchecked(macro_call.syntax());
                     let loc = MacroCallLoc {
-                        source_root_id,
-                        module_id,
+                        module,
                         source_item_id: SourceItemId {
                             file_id,
                             item_id: Some(item_id),
@@ -145,18 +128,9 @@ impl LoweredModule {
                     };
                     let id = loc.id(db);
                     let file_id = HirFileId::from(id);
-                    let file_items = db.file_items(file_id);
                     //FIXME: expand recursively
                     for item in db.hir_source_file(file_id).items() {
-                        self.add_def_id(
-                            source_map,
-                            db,
-                            source_root_id,
-                            module_id,
-                            file_id,
-                            &file_items,
-                            item,
-                        );
+                        self.add_def_id(source_map, db, module, file_id, item);
                     }
                 }
             }
@@ -167,41 +141,74 @@ impl LoweredModule {
         &mut self,
         source_map: &mut ImportSourceMap,
         db: &impl HirDatabase,
-        source_root_id: SourceRootId,
-        module_id: ModuleId,
+        module: Module,
         file_id: HirFileId,
-        file_items: &SourceFileItems,
         item: &ast::ModuleItem,
     ) {
-        let name = match item.kind() {
-            ast::ModuleItemKind::StructDef(it) => it.name(),
-            ast::ModuleItemKind::EnumDef(it) => it.name(),
-            ast::ModuleItemKind::FnDef(it) => it.name(),
-            ast::ModuleItemKind::TraitDef(it) => it.name(),
-            ast::ModuleItemKind::TypeDef(it) => it.name(),
+        let ctx = LocationCtx::new(db, module, file_id);
+        match item.kind() {
+            ast::ModuleItemKind::StructDef(it) => {
+                if let Some(name) = it.name() {
+                    let s = Struct { id: ctx.to_def(it) };
+                    let s: ModuleDef = s.into();
+                    self.declarations.insert(name.as_name(), PerNs::both(s, s));
+                }
+            }
+            ast::ModuleItemKind::EnumDef(it) => {
+                if let Some(name) = it.name() {
+                    let e = Enum { id: ctx.to_def(it) };
+                    let e: ModuleDef = e.into();
+                    self.declarations.insert(name.as_name(), PerNs::types(e));
+                }
+            }
+            ast::ModuleItemKind::FnDef(it) => {
+                if let Some(name) = it.name() {
+                    let func = Function { id: ctx.to_def(it) };
+                    self.declarations
+                        .insert(name.as_name(), PerNs::values(func.into()));
+                }
+            }
+            ast::ModuleItemKind::TraitDef(it) => {
+                if let Some(name) = it.name() {
+                    let t = Trait { id: ctx.to_def(it) };
+                    self.declarations
+                        .insert(name.as_name(), PerNs::types(t.into()));
+                }
+            }
+            ast::ModuleItemKind::TypeDef(it) => {
+                if let Some(name) = it.name() {
+                    let t = Type { id: ctx.to_def(it) };
+                    self.declarations
+                        .insert(name.as_name(), PerNs::types(t.into()));
+                }
+            }
             ast::ModuleItemKind::ImplBlock(_) => {
                 // impls don't define items
-                return;
             }
             ast::ModuleItemKind::UseItem(it) => {
                 self.add_use_item(source_map, it);
-                return;
             }
             ast::ModuleItemKind::ExternCrateItem(_) => {
                 // TODO
-                return;
             }
-            ast::ModuleItemKind::ConstDef(it) => it.name(),
-            ast::ModuleItemKind::StaticDef(it) => it.name(),
+            ast::ModuleItemKind::ConstDef(it) => {
+                if let Some(name) = it.name() {
+                    let c = Const { id: ctx.to_def(it) };
+                    self.declarations
+                        .insert(name.as_name(), PerNs::values(c.into()));
+                }
+            }
+            ast::ModuleItemKind::StaticDef(it) => {
+                if let Some(name) = it.name() {
+                    let s = Static { id: ctx.to_def(it) };
+                    self.declarations
+                        .insert(name.as_name(), PerNs::values(s.into()));
+                }
+            }
             ast::ModuleItemKind::Module(_) => {
                 // modules are handled separately direclty by nameres
-                return;
             }
         };
-        if let Some(name) = name {
-            let def_id = assign_def_id(db, source_root_id, module_id, file_id, file_items, item);
-            self.declarations.insert(name.as_name(), def_id);
-        }
     }
 
     fn add_use_item(&mut self, source_map: &mut ImportSourceMap, item: &ast::UseItem) {
@@ -216,46 +223,3 @@ impl LoweredModule {
         })
     }
 }
-
-fn assign_def_id(
-    db: &impl HirDatabase,
-    source_root_id: SourceRootId,
-    module_id: ModuleId,
-    file_id: HirFileId,
-    file_items: &SourceFileItems,
-    item: &ast::ModuleItem,
-) -> PerNs<DefId> {
-    // depending on the item kind, the location can define something in
-    // the values namespace, the types namespace, or both
-    let kind = DefKind::for_syntax_kind(item.syntax().kind());
-    let def_id = kind.map(|k| {
-        let item_id = file_items.id_of_unchecked(item.syntax());
-        let def_loc = DefLoc {
-            kind: k,
-            source_root_id,
-            module_id,
-            source_item_id: SourceItemId {
-                file_id,
-                item_id: Some(item_id),
-            },
-        };
-        def_loc.id(db)
-    });
-    def_id
-}
-
-impl DefKind {
-    fn for_syntax_kind(kind: SyntaxKind) -> PerNs<DefKind> {
-        match kind {
-            SyntaxKind::FN_DEF => PerNs::values(DefKind::Function),
-            SyntaxKind::MODULE => PerNs::types(DefKind::Module),
-            SyntaxKind::STRUCT_DEF => PerNs::both(DefKind::Struct, DefKind::StructCtor),
-            SyntaxKind::ENUM_DEF => PerNs::types(DefKind::Enum),
-            SyntaxKind::TRAIT_DEF => PerNs::types(DefKind::Trait),
-            SyntaxKind::TYPE_DEF => PerNs::types(DefKind::Type),
-            SyntaxKind::CONST_DEF => PerNs::values(DefKind::Const),
-            SyntaxKind::STATIC_DEF => PerNs::values(DefKind::Static),
-            _ => PerNs::none(),
-        }
-    }
-}
diff --git a/crates/ra_hir/src/nameres/tests.rs b/crates/ra_hir/src/nameres/tests.rs
index e92007453..9322bf08c 100644
--- a/crates/ra_hir/src/nameres/tests.rs
+++ b/crates/ra_hir/src/nameres/tests.rs
@@ -1,6 +1,6 @@
 use std::sync::Arc;
 
-use ra_db::{FilesDatabase, CrateGraph, SourceRootId, salsa::Database};
+use ra_db::{CrateGraph, SourceRootId, salsa::Database};
 use relative_path::RelativePath;
 use test_utils::{assert_eq_text, covers};
 
@@ -13,10 +13,10 @@ use crate::{
 
 fn item_map(fixture: &str) -> (Arc<ItemMap>, ModuleId) {
     let (db, pos) = MockDatabase::with_position(fixture);
-    let source_root = db.file_source_root(pos.file_id);
     let module = crate::source_binder::module_from_position(&db, pos).unwrap();
-    let module_id = module.def_id.loc(&db).module_id;
-    (db.item_map(source_root), module_id)
+    let krate = module.krate(&db).unwrap();
+    let module_id = module.module_id;
+    (db.item_map(krate.crate_id), module_id)
 }
 
 fn check_module_item_map(map: &ItemMap, module_id: ModuleId, expected: &str) {
@@ -238,14 +238,13 @@ fn item_map_across_crates() {
 
     db.set_crate_graph(crate_graph);
 
-    let source_root = db.file_source_root(main_id);
     let module = crate::source_binder::module_from_file_id(&db, main_id).unwrap();
-    let module_id = module.def_id.loc(&db).module_id;
-    let item_map = db.item_map(source_root);
+    let krate = module.krate(&db).unwrap();
+    let item_map = db.item_map(krate.crate_id);
 
     check_module_item_map(
         &item_map,
-        module_id,
+        module.module_id,
         "
             Baz: t v
             test_crate: t
@@ -292,12 +291,12 @@ fn import_across_source_roots() {
     db.set_crate_graph(crate_graph);
 
     let module = crate::source_binder::module_from_file_id(&db, main_id).unwrap();
-    let module_id = module.def_id.loc(&db).module_id;
-    let item_map = db.item_map(source_root);
+    let krate = module.krate(&db).unwrap();
+    let item_map = db.item_map(krate.crate_id);
 
     check_module_item_map(
         &item_map,
-        module_id,
+        module.module_id,
         "
             C: t v
             test_crate: t
@@ -333,14 +332,13 @@ fn reexport_across_crates() {
 
     db.set_crate_graph(crate_graph);
 
-    let source_root = db.file_source_root(main_id);
     let module = crate::source_binder::module_from_file_id(&db, main_id).unwrap();
-    let module_id = module.def_id.loc(&db).module_id;
-    let item_map = db.item_map(source_root);
+    let krate = module.krate(&db).unwrap();
+    let item_map = db.item_map(krate.crate_id);
 
     check_module_item_map(
         &item_map,
-        module_id,
+        module.module_id,
         "
             Baz: t v
             test_crate: t
@@ -350,10 +348,11 @@ fn reexport_across_crates() {
 
 fn check_item_map_is_not_recomputed(initial: &str, file_change: &str) {
     let (mut db, pos) = MockDatabase::with_position(initial);
-    let source_root = db.file_source_root(pos.file_id);
+    let module = crate::source_binder::module_from_file_id(&db, pos.file_id).unwrap();
+    let krate = module.krate(&db).unwrap();
     {
         let events = db.log_executed(|| {
-            db.item_map(source_root);
+            db.item_map(krate.crate_id);
         });
         assert!(format!("{:?}", events).contains("item_map"))
     }
@@ -362,7 +361,7 @@ fn check_item_map_is_not_recomputed(initial: &str, file_change: &str) {
 
     {
         let events = db.log_executed(|| {
-            db.item_map(source_root);
+            db.item_map(krate.crate_id);
         });
         assert!(
             !format!("{:?}", events).contains("item_map"),