Merge #1238

1238: Macro queries r=edwin0cheng a=matklad

In https://github.com/rust-analyzer/rust-analyzer/pull/1231, I've added aggressive clean up of `ast_id_to_node` query. 

The result of this query is a `SyntaxTree`, and we don't want to retain syntax trees in memory unless absolutely necessary.

Moreover, `SyntaxTree` has identity equality semantics, meaning that we'll get a diffferent syntax tree for a file after every reparse. That means that `ast_id_to_node` query should not genereally be used in HIR, unless it is behind some kind of salsa firewall, like the `raw` module of name resoulution.

However, that PR resulted in the abysmal performance: turns out we were using  ast_id_to_node quite heavily in hir when expanding macros! 

So this PR  installs the more incremental-friendly query structure:

* converting source to token tree is now a query; changing source without affecting token-trees will now preserve macro expansions
* expand macro (tt -> tt) is now a query as well, so we cache macro expansions *before* parsing them into item lists or expressions, which is nice: we can cache expansion without knowing the calling context!

r? @edwin0cheng 

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

4 files changed, 63 insertions, 68 deletions

diff --git a/crates/ra_hir/src/db.rs b/crates/ra_hir/src/db.rs
index 3ac3c79a3..398e00c42 100644
--- a/crates/ra_hir/src/db.rs
+++ b/crates/ra_hir/src/db.rs
@@ -42,7 +42,13 @@ pub trait DefDatabase: SourceDatabase {
     #[salsa::invoke(crate::ids::macro_def_query)]
     fn macro_def(&self, macro_id: MacroDefId) -> Option<Arc<mbe::MacroRules>>;
 
-    #[salsa::invoke(HirFileId::hir_parse_query)]
+    #[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>;
+
+    #[salsa::invoke(crate::ids::HirFileId::hir_parse_query)]
     fn hir_parse(&self, file_id: HirFileId) -> TreeArc<SourceFile>;
 
     #[salsa::invoke(crate::adt::StructData::struct_data_query)]
@@ -60,7 +66,7 @@ pub trait DefDatabase: SourceDatabase {
     #[salsa::invoke(crate::source_id::AstIdMap::ast_id_map_query)]
     fn ast_id_map(&self, file_id: HirFileId) -> Arc<AstIdMap>;
 
-    #[salsa::invoke(crate::source_id::AstIdMap::ast_id_to_node_query)]
+    #[salsa::invoke(crate::source_id::AstIdMap::file_item_query)]
     fn ast_id_to_node(&self, file_id: HirFileId, ast_id: ErasedFileAstId) -> TreeArc<SyntaxNode>;
 
     #[salsa::invoke(RawItems::raw_items_query)]
diff --git a/crates/ra_hir/src/expr.rs b/crates/ra_hir/src/expr.rs
index cdadf3ba6..692da2895 100644
--- a/crates/ra_hir/src/expr.rs
+++ b/crates/ra_hir/src/expr.rs
@@ -5,14 +5,13 @@ use rustc_hash::FxHashMap;
 
 use ra_arena::{Arena, RawId, impl_arena_id, map::ArenaMap};
 use ra_syntax::{
-    SyntaxNodePtr, AstPtr, AstNode,TreeArc,
+    SyntaxNodePtr, AstPtr, AstNode,
     ast::{self, LoopBodyOwner, ArgListOwner, NameOwner, LiteralKind,ArrayExprKind, TypeAscriptionOwner}
 };
 
 use crate::{
     Path, Name, HirDatabase, Resolver,DefWithBody, Either, HirFileId,
     name::AsName,
-    ids::{MacroCallId},
     type_ref::{Mutability, TypeRef},
 };
 use crate::{path::GenericArgs, ty::primitive::{IntTy, UncertainIntTy, FloatTy, UncertainFloatTy}};
@@ -826,21 +825,20 @@ where
                     .with_file_id(self.current_file_id);
 
                 if let Some(call_id) = self.resolver.resolve_macro_call(self.db, path, ast_id) {
-                    if let Some(expr) = expand_macro_to_expr(self.db, call_id, e.token_tree()) {
-                        log::debug!("macro expansion {}", expr.syntax().debug_dump());
-                        let old_file_id =
-                            std::mem::replace(&mut self.current_file_id, call_id.into());
-                        let id = self.collect_expr(&expr);
-                        self.current_file_id = old_file_id;
-                        id
-                    } else {
-                        // FIXME: Instead of just dropping the error from expansion
-                        // report it
-                        self.alloc_expr(Expr::Missing, syntax_ptr)
+                    if let Some(tt) = self.db.macro_expand(call_id).ok() {
+                        if let Some(expr) = mbe::token_tree_to_expr(&tt).ok() {
+                            log::debug!("macro expansion {}", expr.syntax().debug_dump());
+                            let old_file_id =
+                                std::mem::replace(&mut self.current_file_id, call_id.into());
+                            let id = self.collect_expr(&expr);
+                            self.current_file_id = old_file_id;
+                            return id;
+                        }
                     }
-                } else {
-                    self.alloc_expr(Expr::Missing, syntax_ptr)
                 }
+                // FIXME: Instead of just dropping the error from expansion
+                // report it
+                self.alloc_expr(Expr::Missing, syntax_ptr)
             }
         }
     }
@@ -999,20 +997,6 @@ where
     }
 }
 
-fn expand_macro_to_expr(
-    db: &impl HirDatabase,
-    macro_call: MacroCallId,
-    args: Option<&ast::TokenTree>,
-) -> Option<TreeArc<ast::Expr>> {
-    let rules = db.macro_def(macro_call.loc(db).def)?;
-
-    let args = mbe::ast_to_token_tree(args?)?.0;
-
-    let expanded = rules.expand(&args).ok()?;
-
-    mbe::token_tree_to_expr(&expanded).ok()
-}
-
 pub(crate) fn body_with_source_map_query(
     db: &impl HirDatabase,
     def: DefWithBody,
diff --git a/crates/ra_hir/src/ids.rs b/crates/ra_hir/src/ids.rs
index b0e9b1f9a..4102951c9 100644
--- a/crates/ra_hir/src/ids.rs
+++ b/crates/ra_hir/src/ids.rs
@@ -63,47 +63,27 @@ impl HirFileId {
         match file_id.0 {
             HirFileIdRepr::File(file_id) => db.parse(file_id),
             HirFileIdRepr::Macro(macro_call_id) => {
-                parse_macro(db, macro_call_id).unwrap_or_else(|err| {
-                    // Note:
-                    // The final goal we would like to make all parse_macro success,
-                    // such that the following log will not call anyway.
-                    log::warn!(
-                        "fail on macro_parse: (reason: {}) {}",
-                        err,
-                        macro_call_id.debug_dump(db)
-                    );
-
-                    // returning an empty string looks fishy...
-                    SourceFile::parse("")
-                })
+                match db.macro_expand(macro_call_id) {
+                    Ok(tt) => mbe::token_tree_to_ast_item_list(&tt),
+                    Err(err) => {
+                        // Note:
+                        // The final goal we would like to make all parse_macro success,
+                        // such that the following log will not call anyway.
+                        log::warn!(
+                            "fail on macro_parse: (reason: {}) {}",
+                            err,
+                            macro_call_id.debug_dump(db)
+                        );
+
+                        // returning an empty string looks fishy...
+                        SourceFile::parse("")
+                    }
+                }
             }
         }
     }
 }
 
-fn parse_macro(
-    db: &impl DefDatabase,
-    macro_call_id: MacroCallId,
-) -> Result<TreeArc<SourceFile>, String> {
-    let loc = macro_call_id.loc(db);
-    let macro_call = loc.ast_id.to_node(db);
-    let (macro_arg, _) = macro_call
-        .token_tree()
-        .and_then(mbe::ast_to_token_tree)
-        .ok_or("Fail to args in to tt::TokenTree")?;
-
-    let macro_rules = db.macro_def(loc.def).ok_or("Fail to find macro definition")?;
-    let tt = macro_rules.expand(&macro_arg).map_err(|err| format!("{:?}", err))?;
-
-    // Set a hard limit for the expanded tt
-    let count = tt.count();
-    if count > 65536 {
-        return Err(format!("Total tokens count exceed limit : count = {}", count));
-    }
-
-    Ok(mbe::token_tree_to_ast_item_list(&tt))
-}
-
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 enum HirFileIdRepr {
     File(FileId),
@@ -139,6 +119,31 @@ pub(crate) fn macro_def_query(db: &impl DefDatabase, id: MacroDefId) -> Option<A
     Some(Arc::new(rules))
 }
 
+pub(crate) fn macro_arg_query(db: &impl DefDatabase, id: MacroCallId) -> Option<Arc<tt::Subtree>> {
+    let loc = id.loc(db);
+    let macro_call = loc.ast_id.to_node(db);
+    let arg = macro_call.token_tree()?;
+    let (tt, _) = mbe::ast_to_token_tree(arg)?;
+    Some(Arc::new(tt))
+}
+
+pub(crate) fn macro_expand_query(
+    db: &impl DefDatabase,
+    id: MacroCallId,
+) -> Result<Arc<tt::Subtree>, String> {
+    let loc = id.loc(db);
+    let macro_arg = db.macro_arg(id).ok_or("Fail to args in to tt::TokenTree")?;
+
+    let macro_rules = db.macro_def(loc.def).ok_or("Fail to find macro definition")?;
+    let tt = macro_rules.expand(&macro_arg).map_err(|err| format!("{:?}", err))?;
+    // Set a hard limit for the expanded tt
+    let count = tt.count();
+    if count > 65536 {
+        return Err(format!("Total tokens count exceed limit : count = {}", count));
+    }
+    Ok(Arc::new(tt))
+}
+
 macro_rules! impl_intern_key {
     ($name:ident) => {
         impl salsa::InternKey for $name {
diff --git a/crates/ra_hir/src/source_id.rs b/crates/ra_hir/src/source_id.rs
index a2bc9a799..0a8fb6d32 100644
--- a/crates/ra_hir/src/source_id.rs
+++ b/crates/ra_hir/src/source_id.rs
@@ -92,7 +92,7 @@ impl AstIdMap {
         Arc::new(AstIdMap::from_source_file(&source_file))
     }
 
-    pub(crate) fn ast_id_to_node_query(
+    pub(crate) fn file_item_query(
         db: &impl DefDatabase,
         file_id: HirFileId,
         ast_id: ErasedFileAstId,