Merge #1267

1267: Macro expand to r=edwin0cheng a=matklad

closes #1264 

The core problem this PR is trying to wrangle is that macros can expand to different stuffs, depending on context. 

That is, `foo!()` on the top-level expands to a list of items, but the same `foo!()` in expression position expands to expression.

Our current `hir_parse(HirFileId) -> TreeArc<SourceFile>` does not really support this. 

So, the plan is to change `hir_parse` to untyped inreface (`TreeArc<Syntaxnode>`), and add `expands_to` field to `MacroCallLoc`, such that the *target* of macro expansion is selected by the calling code and is part of macro id. 

This unfortunately looses some type-safety :( 


Moreover, this doesn't really fix #1264 by itself, because we die due to some other error inside macro expansion: expander fails to produce a tree with a single root, which trips assert inside rowan.

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

Author: bors[bot]

crates/ra_hir/src/db.rs

@@ -1,6 +1,6 @@
 use std::sync::{Arc, Mutex};
 
-use ra_syntax::{SyntaxNode, TreeArc, SourceFile, SmolStr, ast};
+use ra_syntax::{SyntaxNode, TreeArc, SmolStr, ast};
 use ra_db::{SourceDatabase, salsa};
 
 use crate::{
@@ -54,8 +54,8 @@ pub trait DefDatabase: SourceDatabase {
     #[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::ids::HirFileId::parse_or_expand_query)]
+    fn parse_or_expand(&self, file_id: HirFileId) -> Option<TreeArc<SyntaxNode>>;
 
     #[salsa::invoke(crate::adt::StructData::struct_data_query)]
     fn struct_data(&self, s: Struct) -> Arc<StructData>;

crates/ra_hir/src/diagnostics.rs

@@ -1,6 +1,6 @@
 use std::{fmt, any::Any};
 
-use ra_syntax::{SyntaxNodePtr, TreeArc, AstPtr, TextRange, ast, SyntaxNode, AstNode};
+use ra_syntax::{SyntaxNodePtr, TreeArc, AstPtr, TextRange, ast, SyntaxNode};
 use relative_path::RelativePathBuf;
 
 use crate::{HirFileId, HirDatabase, Name};
@@ -29,8 +29,8 @@ pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static {
 
 impl dyn Diagnostic {
     pub fn syntax_node(&self, db: &impl HirDatabase) -> TreeArc<SyntaxNode> {
-        let source_file = db.hir_parse(self.file());
-        self.syntax_node_ptr().to_node(source_file.syntax()).to_owned()
+        let node = db.parse_or_expand(self.file()).unwrap();
+        self.syntax_node_ptr().to_node(&*node).to_owned()
     }
     pub fn downcast_ref<D: Diagnostic>(&self) -> Option<&D> {
         self.as_any().downcast_ref()

crates/ra_hir/src/expr.rs

@@ -6,11 +6,11 @@ use rustc_hash::FxHashMap;
 use ra_arena::{Arena, RawId, impl_arena_id, map::ArenaMap};
 use ra_syntax::{
     SyntaxNodePtr, AstPtr, AstNode,
-    ast::{self, LoopBodyOwner, ArgListOwner, NameOwner, LiteralKind,ArrayExprKind, TypeAscriptionOwner}
+    ast::{self, LoopBodyOwner, ArgListOwner, NameOwner, LiteralKind,ArrayExprKind, TypeAscriptionOwner},
 };
 
 use crate::{
-    Path, Name, HirDatabase, Resolver,DefWithBody, Either, HirFileId, MacroCallLoc,
+    Path, Name, HirDatabase, Resolver,DefWithBody, Either, HirFileId, MacroCallLoc, MacroFileKind,
     name::AsName,
     type_ref::{Mutability, TypeRef},
 };
@@ -830,11 +830,11 @@ where
 
                 if let Some(def) = self.resolver.resolve_macro_call(path) {
                     let call_id = MacroCallLoc { def, ast_id }.id(self.db);
-                    if let Some(tt) = self.db.macro_expand(call_id).ok() {
-                        if let Some(expr) = mbe::token_tree_to_expr(&tt).ok() {
+                    let file_id = call_id.as_file(MacroFileKind::Expr);
+                    if let Some(node) = self.db.parse_or_expand(file_id) {
+                        if let Some(expr) = ast::Expr::cast(&*node) {
                             log::debug!("macro expansion {}", expr.syntax().debug_dump());
-                            let old_file_id =
-                                std::mem::replace(&mut self.current_file_id, call_id.into());
+                            let old_file_id = std::mem::replace(&mut self.current_file_id, file_id);
                             let id = self.collect_expr(&expr);
                             self.current_file_id = old_file_id;
                             return id;

crates/ra_hir/src/ids.rs

@@ -4,7 +4,7 @@ use std::{
 };
 
 use ra_db::{FileId, salsa};
-use ra_syntax::{TreeArc, SourceFile, AstNode, ast};
+use ra_syntax::{TreeArc, AstNode, ast, SyntaxNode};
 use mbe::MacroRules;
 
 use crate::{
@@ -39,8 +39,8 @@ impl HirFileId {
     pub fn original_file(self, db: &impl DefDatabase) -> FileId {
         match self.0 {
             HirFileIdRepr::File(file_id) => file_id,
-            HirFileIdRepr::Macro(macro_call_id) => {
-                let loc = macro_call_id.loc(db);
+            HirFileIdRepr::Macro(macro_file) => {
+                let loc = macro_file.macro_call_id.loc(db);
                 loc.ast_id.file_id().original_file(db)
             }
         }
@@ -56,16 +56,17 @@ impl HirFileId {
         }
     }
 
-    pub(crate) fn hir_parse_query(
+    pub(crate) fn parse_or_expand_query(
         db: &impl DefDatabase,
         file_id: HirFileId,
-    ) -> TreeArc<SourceFile> {
+    ) -> Option<TreeArc<SyntaxNode>> {
         match file_id.0 {
-            HirFileIdRepr::File(file_id) => db.parse(file_id),
-            HirFileIdRepr::Macro(macro_call_id) => {
-                match db.macro_expand(macro_call_id) {
-                    Ok(tt) => mbe::token_tree_to_ast_item_list(&tt),
-                    Err(err) => {
+            HirFileIdRepr::File(file_id) => Some(db.parse(file_id).syntax().to_owned()),
+            HirFileIdRepr::Macro(macro_file) => {
+                let macro_call_id = macro_file.macro_call_id;
+                let tt = db
+                    .macro_expand(macro_call_id)
+                    .map_err(|err| {
                         // Note:
                         // The final goal we would like to make all parse_macro success,
                         // such that the following log will not call anyway.
@@ -74,9 +75,14 @@ impl HirFileId {
                             err,
                             macro_call_id.debug_dump(db)
                         );
-
-                        // returning an empty string looks fishy...
-                        SourceFile::parse("")
+                    })
+                    .ok()?;
+                match macro_file.macro_file_kind {
+                    MacroFileKind::Items => {
+                        Some(mbe::token_tree_to_ast_item_list(&tt).syntax().to_owned())
+                    }
+                    MacroFileKind::Expr => {
+                        mbe::token_tree_to_expr(&tt).ok().map(|it| it.syntax().to_owned())
                     }
                 }
             }
@@ -87,7 +93,19 @@ impl HirFileId {
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 enum HirFileIdRepr {
     File(FileId),
-    Macro(MacroCallId),
+    Macro(MacroFile),
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+struct MacroFile {
+    macro_call_id: MacroCallId,
+    macro_file_kind: MacroFileKind,
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub(crate) enum MacroFileKind {
+    Items,
+    Expr,
 }
 
 impl From<FileId> for HirFileId {
@@ -96,12 +114,6 @@ impl From<FileId> for HirFileId {
     }
 }
 
-impl From<MacroCallId> for HirFileId {
-    fn from(macro_call_id: MacroCallId) -> HirFileId {
-        HirFileId(HirFileIdRepr::Macro(macro_call_id))
-    }
-}
-
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct MacroDefId(pub(crate) AstId<ast::MacroCall>);
 
@@ -173,6 +185,11 @@ impl MacroCallId {
     pub(crate) fn loc(self, db: &impl DefDatabase) -> MacroCallLoc {
         db.lookup_intern_macro(self)
     }
+
+    pub(crate) fn as_file(self, kind: MacroFileKind) -> HirFileId {
+        let macro_file = MacroFile { macro_call_id: self, macro_file_kind: kind };
+        HirFileId(HirFileIdRepr::Macro(macro_file))
+    }
 }
 
 impl MacroCallLoc {
@@ -342,7 +359,7 @@ impl MacroCallId {
         let syntax_str = node.syntax().text().chunks().collect::<Vec<_>>().join(" ");
 
         // dump the file name
-        let file_id: HirFileId = self.clone().into();
+        let file_id: HirFileId = self.loc(db).ast_id.file_id();
         let original = file_id.original_file(db);
         let macro_rules = db.macro_def(loc.def);
 

crates/ra_hir/src/lib.rs

@@ -53,6 +53,7 @@ use crate::{
     name::{AsName, KnownName},
     source_id::{FileAstId, AstId},
     resolve::Resolver,
+    ids::MacroFileKind,
 };
 
 pub use self::{

crates/ra_hir/src/nameres/collector.rs

@@ -15,7 +15,7 @@ use crate::{
         diagnostics::DefDiagnostic,
         raw,
     },
-    ids::{AstItemDef, LocationCtx, MacroCallLoc, MacroCallId, MacroDefId},
+    ids::{AstItemDef, LocationCtx, MacroCallLoc, MacroCallId, MacroDefId, MacroFileKind},
     AstId,
 };
 
@@ -371,7 +371,7 @@ where
         self.macro_stack_monitor.increase(macro_def_id);
 
         if !self.macro_stack_monitor.is_poison(macro_def_id) {
-            let file_id: HirFileId = macro_call_id.into();
+            let file_id: HirFileId = macro_call_id.as_file(MacroFileKind::Items);
             let raw_items = self.db.raw_items(file_id);
             ModCollector { def_collector: &mut *self, file_id, module_id, raw_items: &raw_items }
                 .collect(raw_items.items());

crates/ra_hir/src/nameres/raw.rs

@@ -75,8 +75,11 @@ impl RawItems {
             source_ast_id_map: db.ast_id_map(file_id.into()),
             source_map: ImportSourceMap::default(),
         };
-        let source_file = db.hir_parse(file_id);
-        collector.process_module(None, &*source_file);
+        if let Some(node) = db.parse_or_expand(file_id) {
+            if let Some(source_file) = ast::SourceFile::cast(&node) {
+                collector.process_module(None, &*source_file);
+            }
+        }
         (Arc::new(collector.raw_items), Arc::new(collector.source_map))
     }
 

crates/ra_hir/src/source_id.rs

@@ -81,24 +81,28 @@ pub struct ErasedFileAstId(RawId);
 impl_arena_id!(ErasedFileAstId);
 
 /// Maps items' `SyntaxNode`s to `ErasedFileAstId`s and back.
-#[derive(Debug, PartialEq, Eq)]
+#[derive(Debug, PartialEq, Eq, Default)]
 pub struct AstIdMap {
     arena: Arena<ErasedFileAstId, SyntaxNodePtr>,
 }
 
 impl AstIdMap {
     pub(crate) fn ast_id_map_query(db: &impl DefDatabase, file_id: HirFileId) -> Arc<AstIdMap> {
-        let source_file = db.hir_parse(file_id);
-        Arc::new(AstIdMap::from_source(source_file.syntax()))
+        let map = if let Some(node) = db.parse_or_expand(file_id) {
+            AstIdMap::from_source(&*node)
+        } else {
+            AstIdMap::default()
+        };
+        Arc::new(map)
     }
 
     pub(crate) fn file_item_query(
         db: &impl DefDatabase,
         file_id: HirFileId,
         ast_id: ErasedFileAstId,
     ) -> TreeArc<SyntaxNode> {
-        let source_file = db.hir_parse(file_id);
-        db.ast_id_map(file_id).arena[ast_id].to_node(source_file.syntax()).to_owned()
+        let node = db.parse_or_expand(file_id).unwrap();
+        db.ast_id_map(file_id).arena[ast_id].to_node(&*node).to_owned()
     }
 
     pub(crate) fn ast_id<N: AstNode>(&self, item: &N) -> FileAstId<N> {

crates/ra_ide_api/src/change.rs

@@ -222,7 +222,7 @@ impl RootDatabase {
 
         self.query(ra_db::ParseQuery).sweep(sweep);
 
-        self.query(hir::db::HirParseQuery).sweep(sweep);
+        self.query(hir::db::ParseOrExpandQuery).sweep(sweep);
         self.query(hir::db::AstIdMapQuery).sweep(sweep);
         self.query(hir::db::AstIdToNodeQuery).sweep(sweep);