Merge #8776

8776: fix: fix unnecessary recomputations due to macros r=jonas-schievink a=jonas-schievink

This computes a macro's fragment kind eagerly (when the calling file is still available in parsed form) and stores it in the `MacroCallLoc`. This means that during expansion we no longer have to reparse the file containing the macro call, avoiding the unnecessary salsa dependencies (https://github.com/rust-analyzer/rust-analyzer/pull/8746#issuecomment-834776349).

Marking as draft until I manage to find a test for this problem, since for some reason `typing_inside_a_function_should_not_invalidate_expansions` does not catch this (which might indicate that I misunderstand the problem).

I've manually confirmed that this fixes the issue described in https://github.com/rust-analyzer/rust-analyzer/pull/8746#issuecomment-834776349:

```
    7ms - parse_query @ FileId(179)
   12ms - SourceBinder::to_module_def
       12ms - crate_def_map:wait
            5ms - item_tree_query (1 calls)
            7ms - ???
```

Co-authored-by: Jonas Schievink <[email protected]>

6 files changed, 35 insertions, 11 deletions

diff --git a/crates/hir_def/src/item_tree.rs b/crates/hir_def/src/item_tree.rs
index 8d13c7e04..cad8a7479 100644
--- a/crates/hir_def/src/item_tree.rs
+++ b/crates/hir_def/src/item_tree.rs
@@ -18,7 +18,7 @@ use hir_expand::{
     ast_id_map::FileAstId,
     hygiene::Hygiene,
     name::{name, AsName, Name},
-    HirFileId, InFile,
+    FragmentKind, HirFileId, InFile,
 };
 use la_arena::{Arena, Idx, RawIdx};
 use profile::Count;
@@ -656,6 +656,7 @@ pub struct MacroCall {
     /// Path to the called macro.
     pub path: Interned<ModPath>,
     pub ast_id: FileAstId<ast::MacroCall>,
+    pub fragment: FragmentKind,
 }
 
 #[derive(Debug, Clone, Eq, PartialEq)]
diff --git a/crates/hir_def/src/item_tree/lower.rs b/crates/hir_def/src/item_tree/lower.rs
index 5743b3386..fe348091d 100644
--- a/crates/hir_def/src/item_tree/lower.rs
+++ b/crates/hir_def/src/item_tree/lower.rs
@@ -624,7 +624,8 @@ impl<'a> Ctx<'a> {
     fn lower_macro_call(&mut self, m: &ast::MacroCall) -> Option<FileItemTreeId<MacroCall>> {
         let path = Interned::new(ModPath::from_src(self.db, m.path()?, &self.hygiene)?);
         let ast_id = self.source_ast_id_map.ast_id(m);
-        let res = MacroCall { path, ast_id };
+        let fragment = hir_expand::to_fragment_kind(m);
+        let res = MacroCall { path, ast_id, fragment };
         Some(id(self.data().macro_calls.alloc(res)))
     }
 
diff --git a/crates/hir_def/src/lib.rs b/crates/hir_def/src/lib.rs
index da46f16f7..e96ca953f 100644
--- a/crates/hir_def/src/lib.rs
+++ b/crates/hir_def/src/lib.rs
@@ -62,7 +62,8 @@ use hir_expand::{
     ast_id_map::FileAstId,
     eager::{expand_eager_macro, ErrorEmitted, ErrorSink},
     hygiene::Hygiene,
-    AstId, AttrId, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
+    AstId, AttrId, FragmentKind, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId,
+    MacroDefKind,
 };
 use la_arena::Idx;
 use nameres::DefMap;
@@ -652,6 +653,7 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
         resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
         mut error_sink: &mut dyn FnMut(mbe::ExpandError),
     ) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro> {
+        let fragment = hir_expand::to_fragment_kind(self.value);
         let ast_id = AstId::new(self.file_id, db.ast_id_map(self.file_id).ast_id(self.value));
         let h = Hygiene::new(db.upcast(), self.file_id);
         let path = self.value.path().and_then(|path| path::ModPath::from_src(db, path, &h));
@@ -667,6 +669,7 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
 
         macro_call_as_call_id(
             &AstIdWithPath::new(ast_id.file_id, ast_id.value, path),
+            fragment,
             db,
             krate,
             resolver,
@@ -695,6 +698,7 @@ pub struct UnresolvedMacro {
 
 fn macro_call_as_call_id(
     call: &AstIdWithPath<ast::MacroCall>,
+    fragment: FragmentKind,
     db: &dyn db::DefDatabase,
     krate: CrateId,
     resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
@@ -718,7 +722,11 @@ fn macro_call_as_call_id(
         .map(MacroCallId::from)
     } else {
         Ok(def
-            .as_lazy_macro(db.upcast(), krate, MacroCallKind::FnLike { ast_id: call.ast_id })
+            .as_lazy_macro(
+                db.upcast(),
+                krate,
+                MacroCallKind::FnLike { ast_id: call.ast_id, fragment },
+            )
             .into())
     };
     Ok(res)
diff --git a/crates/hir_def/src/nameres.rs b/crates/hir_def/src/nameres.rs
index 1bc72ec1f..249af6fc8 100644
--- a/crates/hir_def/src/nameres.rs
+++ b/crates/hir_def/src/nameres.rs
@@ -629,7 +629,7 @@ mod diagnostics {
                 DiagnosticKind::UnresolvedProcMacro { ast } => {
                     let mut precise_location = None;
                     let (file, ast, name) = match ast {
-                        MacroCallKind::FnLike { ast_id } => {
+                        MacroCallKind::FnLike { ast_id, .. } => {
                             let node = ast_id.to_node(db.upcast());
                             (ast_id.file_id, SyntaxNodePtr::from(AstPtr::new(&node)), None)
                         }
diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs
index 05ceb1efb..e89136ed1 100644
--- a/crates/hir_def/src/nameres/collector.rs
+++ b/crates/hir_def/src/nameres/collector.rs
@@ -13,7 +13,7 @@ use hir_expand::{
     builtin_macro::find_builtin_macro,
     name::{AsName, Name},
     proc_macro::ProcMacroExpander,
-    AttrId, HirFileId, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
+    AttrId, FragmentKind, HirFileId, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
 };
 use hir_expand::{InFile, MacroCallLoc};
 use rustc_hash::{FxHashMap, FxHashSet};
@@ -215,7 +215,7 @@ struct MacroDirective {
 
 #[derive(Clone, Debug, Eq, PartialEq)]
 enum MacroDirectiveKind {
-    FnLike { ast_id: AstIdWithPath<ast::MacroCall> },
+    FnLike { ast_id: AstIdWithPath<ast::MacroCall>, fragment: FragmentKind },
     Derive { ast_id: AstIdWithPath<ast::Item>, derive_attr: AttrId },
 }
 
@@ -807,9 +807,10 @@ impl DefCollector<'_> {
         let mut res = ReachedFixedPoint::Yes;
         macros.retain(|directive| {
             match &directive.kind {
-                MacroDirectiveKind::FnLike { ast_id } => {
+                MacroDirectiveKind::FnLike { ast_id, fragment } => {
                     match macro_call_as_call_id(
                         ast_id,
+                        *fragment,
                         self.db,
                         self.def_map.krate,
                         |path| {
@@ -926,8 +927,9 @@ impl DefCollector<'_> {
 
         for directive in &self.unexpanded_macros {
             match &directive.kind {
-                MacroDirectiveKind::FnLike { ast_id, .. } => match macro_call_as_call_id(
+                MacroDirectiveKind::FnLike { ast_id, fragment } => match macro_call_as_call_id(
                     ast_id,
+                    *fragment,
                     self.db,
                     self.def_map.krate,
                     |path| {
@@ -1496,6 +1498,7 @@ impl ModCollector<'_, '_> {
         let mut error = None;
         match macro_call_as_call_id(
             &ast_id,
+            mac.fragment,
             self.def_collector.db,
             self.def_collector.def_map.krate,
             |path| {
@@ -1524,9 +1527,14 @@ impl ModCollector<'_, '_> {
             }
             Ok(Err(_)) => {
                 // Built-in macro failed eager expansion.
+
+                // FIXME: don't parse the file here
+                let fragment = hir_expand::to_fragment_kind(
+                    &ast_id.ast_id.to_node(self.def_collector.db.upcast()),
+                );
                 self.def_collector.def_map.diagnostics.push(DefDiagnostic::macro_error(
                     self.module_id,
-                    MacroCallKind::FnLike { ast_id: ast_id.ast_id },
+                    MacroCallKind::FnLike { ast_id: ast_id.ast_id, fragment },
                     error.unwrap().to_string(),
                 ));
                 return;
@@ -1543,7 +1551,7 @@ impl ModCollector<'_, '_> {
         self.def_collector.unexpanded_macros.push(MacroDirective {
             module_id: self.module_id,
             depth: self.macro_depth + 1,
-            kind: MacroDirectiveKind::FnLike { ast_id },
+            kind: MacroDirectiveKind::FnLike { ast_id, fragment: mac.fragment },
         });
     }
 
diff --git a/crates/hir_def/src/nameres/tests/incremental.rs b/crates/hir_def/src/nameres/tests/incremental.rs
index 227ecd162..d884a6eb4 100644
--- a/crates/hir_def/src/nameres/tests/incremental.rs
+++ b/crates/hir_def/src/nameres/tests/incremental.rs
@@ -137,6 +137,9 @@ m!(Z);
         });
         let n_recalculated_item_trees = events.iter().filter(|it| it.contains("item_tree")).count();
         assert_eq!(n_recalculated_item_trees, 6);
+        let n_reparsed_macros =
+            events.iter().filter(|it| it.contains("parse_macro_expansion")).count();
+        assert_eq!(n_reparsed_macros, 3);
     }
 
     let new_text = r#"
@@ -155,5 +158,8 @@ m!(Z);
         });
         let n_recalculated_item_trees = events.iter().filter(|it| it.contains("item_tree")).count();
         assert_eq!(n_recalculated_item_trees, 1);
+        let n_reparsed_macros =
+            events.iter().filter(|it| it.contains("parse_macro_expansion")).count();
+        assert_eq!(n_reparsed_macros, 0);
     }
 }