Merge #1447

1447: make sure that CrateDefMap is independent from syntax r=matklad a=matklad



Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

crates/ra_hir/src/db.rs

@@ -20,38 +20,15 @@ use crate::{
     lang_item::{LangItems, LangItemTarget}, type_alias::TypeAliasData,
 };
 
-// This database has access to source code, so queries here are not really
-// incremental.
-#[salsa::query_group(AstDatabaseStorage)]
-pub trait AstDatabase: SourceDatabase {
+/// We store all interned things in the single QueryGroup.
+///
+/// This is done mainly to allow both "volatile" `AstDatabase` and "stable"
+/// `DefDatabase` to access macros, without adding hard dependencies between the
+/// two.
+#[salsa::query_group(InternDatabaseStorage)]
+pub trait InternDatabase: SourceDatabase {
     #[salsa::interned]
     fn intern_macro(&self, macro_call: MacroCallLoc) -> ids::MacroCallId;
-
-    #[salsa::invoke(crate::source_id::AstIdMap::ast_id_map_query)]
-    fn ast_id_map(&self, file_id: HirFileId) -> Arc<AstIdMap>;
-    #[salsa::transparent]
-    #[salsa::invoke(crate::source_id::AstIdMap::file_item_query)]
-    fn ast_id_to_node(&self, file_id: HirFileId, ast_id: ErasedFileAstId) -> TreeArc<SyntaxNode>;
-    #[salsa::transparent]
-    #[salsa::invoke(crate::ids::HirFileId::parse_or_expand_query)]
-    fn parse_or_expand(&self, file_id: HirFileId) -> Option<TreeArc<SyntaxNode>>;
-    #[salsa::invoke(crate::ids::HirFileId::parse_macro_query)]
-    fn parse_macro(&self, macro_file: ids::MacroFile) -> Option<TreeArc<SyntaxNode>>;
-
-    #[salsa::invoke(crate::ids::macro_def_query)]
-    fn macro_def(&self, macro_id: MacroDefId) -> Option<Arc<mbe::MacroRules>>;
-
-    #[salsa::invoke(crate::ids::macro_arg_query)]
-    fn macro_arg(&self, macro_call: ids::MacroCallId) -> Option<Arc<tt::Subtree>>;
-
-    #[salsa::invoke(crate::ids::macro_expand_query)]
-    fn macro_expand(&self, macro_call: ids::MacroCallId) -> Result<Arc<tt::Subtree>, String>;
-}
-
-// This database uses `AstDatabase` internally,
-#[salsa::query_group(DefDatabaseStorage)]
-#[salsa::requires(AstDatabase)]
-pub trait DefDatabase: SourceDatabase {
     #[salsa::interned]
     fn intern_function(&self, loc: ids::ItemLoc<ast::FnDef>) -> ids::FunctionId;
     #[salsa::interned]
@@ -72,7 +49,36 @@ pub trait DefDatabase: SourceDatabase {
     fn intern_type_ctor(&self, type_ctor: TypeCtor) -> ids::TypeCtorId;
     #[salsa::interned]
     fn intern_impl_block(&self, impl_block: ImplBlock) -> ids::GlobalImplId;
+}
 
+/// This database has access to source code, so queries here are not really
+/// incremental.
+#[salsa::query_group(AstDatabaseStorage)]
+pub trait AstDatabase: InternDatabase {
+    #[salsa::invoke(crate::source_id::AstIdMap::ast_id_map_query)]
+    fn ast_id_map(&self, file_id: HirFileId) -> Arc<AstIdMap>;
+    #[salsa::transparent]
+    #[salsa::invoke(crate::source_id::AstIdMap::file_item_query)]
+    fn ast_id_to_node(&self, file_id: HirFileId, ast_id: ErasedFileAstId) -> TreeArc<SyntaxNode>;
+    #[salsa::transparent]
+    #[salsa::invoke(crate::ids::HirFileId::parse_or_expand_query)]
+    fn parse_or_expand(&self, file_id: HirFileId) -> Option<TreeArc<SyntaxNode>>;
+    #[salsa::invoke(crate::ids::HirFileId::parse_macro_query)]
+    fn parse_macro(&self, macro_file: ids::MacroFile) -> Option<TreeArc<SyntaxNode>>;
+
+    #[salsa::invoke(crate::ids::macro_def_query)]
+    fn macro_def(&self, macro_id: MacroDefId) -> Option<Arc<mbe::MacroRules>>;
+    #[salsa::invoke(crate::ids::macro_arg_query)]
+    fn macro_arg(&self, macro_call: ids::MacroCallId) -> Option<Arc<tt::Subtree>>;
+
+    #[salsa::invoke(crate::ids::macro_expand_query)]
+    fn macro_expand(&self, macro_call: ids::MacroCallId) -> Result<Arc<tt::Subtree>, String>;
+}
+
+// This database uses `AstDatabase` internally,
+#[salsa::query_group(DefDatabaseStorage)]
+#[salsa::requires(AstDatabase)]
+pub trait DefDatabase: InternDatabase {
     #[salsa::invoke(crate::adt::StructData::struct_data_query)]
     fn struct_data(&self, s: Struct) -> Arc<StructData>;
 

crates/ra_hir/src/ids.rs

@@ -9,7 +9,7 @@ use ra_prof::profile;
 use mbe::MacroRules;
 
 use crate::{
-    Module, DefDatabase, AstId, FileAstId, AstDatabase, Source,
+    Module, DefDatabase, AstId, FileAstId, AstDatabase, Source, InternDatabase,
 };
 
 /// hir makes heavy use of ids: integer (u32) handlers to various things. You
@@ -37,7 +37,7 @@ pub struct HirFileId(HirFileIdRepr);
 impl HirFileId {
     /// For macro-expansion files, returns the file original source file the
     /// expansion originated from.
-    pub fn original_file(self, db: &impl AstDatabase) -> FileId {
+    pub fn original_file(self, db: &impl InternDatabase) -> FileId {
         match self.0 {
             HirFileIdRepr::File(file_id) => file_id,
             HirFileIdRepr::Macro(macro_file) => {
@@ -187,7 +187,7 @@ pub struct MacroCallLoc {
 }
 
 impl MacroCallId {
-    pub(crate) fn loc(self, db: &impl AstDatabase) -> MacroCallLoc {
+    pub(crate) fn loc(self, db: &impl InternDatabase) -> MacroCallLoc {
         db.lookup_intern_macro(self)
     }
 
@@ -198,7 +198,7 @@ impl MacroCallId {
 }
 
 impl MacroCallLoc {
-    pub(crate) fn id(self, db: &impl AstDatabase) -> MacroCallId {
+    pub(crate) fn id(self, db: &impl InternDatabase) -> MacroCallId {
         db.intern_macro(self)
     }
 }
@@ -235,10 +235,13 @@ pub(crate) struct LocationCtx<DB> {
     file_id: HirFileId,
 }
 
-impl<'a, DB: DefDatabase + AstDatabase> LocationCtx<&'a DB> {
+impl<'a, DB: DefDatabase> LocationCtx<&'a DB> {
     pub(crate) fn new(db: &'a DB, module: Module, file_id: HirFileId) -> LocationCtx<&'a DB> {
         LocationCtx { db, module, file_id }
     }
+}
+
+impl<'a, DB: DefDatabase + AstDatabase> LocationCtx<&'a DB> {
     pub(crate) fn to_def<N, DEF>(self, ast: &N) -> DEF
     where
         N: AstNode,
@@ -257,10 +260,7 @@ pub(crate) trait AstItemDef<N: AstNode>: salsa::InternKey + Clone {
         let item_id = items.ast_id(ast);
         Self::from_ast_id(ctx, item_id)
     }
-    fn from_ast_id(
-        ctx: LocationCtx<&(impl AstDatabase + DefDatabase)>,
-        ast_id: FileAstId<N>,
-    ) -> Self {
+    fn from_ast_id(ctx: LocationCtx<&impl DefDatabase>, ast_id: FileAstId<N>) -> Self {
         let loc = ItemLoc { module: ctx.module, ast_id: ast_id.with_file_id(ctx.file_id) };
         Self::intern(ctx.db, loc)
     }

crates/ra_hir/src/lib.rs

@@ -47,7 +47,7 @@ mod code_model;
 mod marks;
 
 use crate::{
-    db::{AstDatabase, DefDatabase, HirDatabase},
+    db::{InternDatabase, AstDatabase, DefDatabase, HirDatabase},
     name::{AsName, KnownName},
     source_id::{FileAstId, AstId},
     resolve::Resolver,

crates/ra_hir/src/mock.rs

@@ -15,6 +15,7 @@ pub const WORKSPACE: SourceRootId = SourceRootId(0);
 
 #[salsa::database(
     ra_db::SourceDatabaseStorage,
+    db::InternDatabaseStorage,
     db::AstDatabaseStorage,
     db::DefDatabaseStorage,
     db::HirDatabaseStorage

crates/ra_hir/src/nameres.rs

@@ -231,7 +231,9 @@ fn or(left: ItemOrMacro, right: ItemOrMacro) -> ItemOrMacro {
 
 impl CrateDefMap {
     pub(crate) fn crate_def_map_query(
-        db: &(impl DefDatabase + AstDatabase),
+        // Note that this doesn't have `+ AstDatabase`!
+        // This gurantess that `CrateDefMap` is stable across reparses.
+        db: &impl DefDatabase,
         krate: Crate,
     ) -> Arc<CrateDefMap> {
         let _p = profile("crate_def_map_query");

crates/ra_hir/src/nameres/collector.rs

@@ -7,7 +7,7 @@ use ra_syntax::ast;
 
 use crate::{
     Function, Module, Struct, Union, Enum, Const, Static, Trait, TypeAlias, MacroDef,
-    DefDatabase, HirFileId, Name, Path, AstDatabase,
+    DefDatabase, HirFileId, Name, Path,
     KnownName, AstId,
     nameres::{
         Resolution, PerNs, ModuleDef, ReachedFixedPoint, ResolveMode,
@@ -19,10 +19,7 @@ use crate::{
     either::Either,
 };
 
-pub(super) fn collect_defs(
-    db: &(impl DefDatabase + AstDatabase),
-    mut def_map: CrateDefMap,
-) -> CrateDefMap {
+pub(super) fn collect_defs(db: &impl DefDatabase, mut def_map: CrateDefMap) -> CrateDefMap {
     // populate external prelude
     for dep in def_map.krate.dependencies(db) {
         log::debug!("crate dep {:?} -> {:?}", dep.name, dep.krate);
@@ -95,7 +92,7 @@ struct DefCollector<DB> {
 
 impl<'a, DB> DefCollector<&'a DB>
 where
-    DB: DefDatabase + AstDatabase,
+    DB: DefDatabase,
 {
     fn collect(&mut self) {
         let crate_graph = self.db.crate_graph();
@@ -465,7 +462,7 @@ where
             ModCollector { def_collector: &mut *self, file_id, module_id, raw_items: &raw_items }
                 .collect(raw_items.items());
         } else {
-            log::error!("Too deep macro expansion: {}", macro_call_id.debug_dump(self.db));
+            log::error!("Too deep macro expansion: {:?}", macro_call_id);
             self.def_map.poison_macros.insert(macro_def_id);
         }
 
@@ -487,7 +484,7 @@ struct ModCollector<'a, D> {
 
 impl<DB> ModCollector<'_, &'_ mut DefCollector<&'_ DB>>
 where
-    DB: DefDatabase + AstDatabase,
+    DB: DefDatabase,
 {
     fn collect(&mut self, items: &[raw::RawItem]) {
         for item in items {
@@ -632,7 +629,7 @@ fn is_macro_rules(path: &Path) -> bool {
 }
 
 fn resolve_submodule(
-    db: &(impl DefDatabase + AstDatabase),
+    db: &impl DefDatabase,
     file_id: HirFileId,
     name: &Name,
     is_root: bool,
@@ -675,7 +672,7 @@ mod tests {
     use rustc_hash::FxHashSet;
 
     fn do_collect_defs(
-        db: &(impl DefDatabase + AstDatabase),
+        db: &impl DefDatabase,
         def_map: CrateDefMap,
         monitor: MacroStackMonitor,
     ) -> CrateDefMap {

crates/ra_ide_api/src/db.rs

@@ -14,6 +14,7 @@ use crate::{LineIndex, symbol_index::{self, SymbolsDatabase}};
     ra_db::SourceDatabaseStorage,
     LineIndexDatabaseStorage,
     symbol_index::SymbolsDatabaseStorage,
+    hir::db::InternDatabaseStorage,
     hir::db::AstDatabaseStorage,
     hir::db::DefDatabaseStorage,
     hir::db::HirDatabaseStorage