Merge #11904

11904: internal: Wrap macros in expr position in `MacroExpr` node r=jonas-schievink a=jonas-schievink

This lets us distinguish them from macros in item position just by looking at the syntax tree.

Fixes #11854

bors r+

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

Author: bors[bot]

crates/hir/src/source_analyzer.rs

@@ -110,8 +110,8 @@ impl SourceAnalyzer {
 
     fn expr_id(&self, db: &dyn HirDatabase, expr: &ast::Expr) -> Option<ExprId> {
         let src = match expr {
-            ast::Expr::MacroCall(call) => {
-                self.expand_expr(db, InFile::new(self.file_id, call.clone()))?
+            ast::Expr::MacroExpr(expr) => {
+                self.expand_expr(db, InFile::new(self.file_id, expr.macro_call()?.clone()))?
             }
             _ => InFile::new(self.file_id, expr.clone()),
         };

crates/hir_def/src/body/lower.rs

@@ -506,7 +506,8 @@ impl ExprCollector<'_> {
                     None => self.alloc_expr(Expr::Missing, syntax_ptr),
                 }
             }
-            ast::Expr::MacroCall(e) => {
+            ast::Expr::MacroExpr(e) => {
+                let e = e.macro_call()?;
                 let macro_ptr = AstPtr::new(&e);
                 let id = self.collect_macro_call(e, macro_ptr.clone(), true, |this, expansion| {
                     expansion.map(|it| this.collect_expr(it))
@@ -629,7 +630,11 @@ impl ExprCollector<'_> {
                 }
                 let has_semi = stmt.semicolon_token().is_some();
                 // Note that macro could be expended to multiple statements
-                if let Some(ast::Expr::MacroCall(m)) = stmt.expr() {
+                if let Some(ast::Expr::MacroExpr(e)) = stmt.expr() {
+                    let m = match e.macro_call() {
+                        Some(it) => it,
+                        None => return,
+                    };
                     let macro_ptr = AstPtr::new(&m);
                     let syntax_ptr = AstPtr::new(&stmt.expr().unwrap());
 

crates/hir_def/src/body/tests.rs

@@ -79,6 +79,34 @@ fn main() { n_nuple!(1,2,3); }
     );
 }
 
+#[test]
+fn issue_3642_bad_macro_stackover() {
+    lower(
+        r#"
+#[macro_export]
+macro_rules! match_ast {
+    (match $node:ident { $($tt:tt)* }) => { match_ast!(match ($node) { $($tt)* }) };
+
+    (match ($node:expr) {
+        $( ast::$ast:ident($it:ident) => $res:expr, )*
+        _ => $catch_all:expr $(,)?
+    }) => {{
+        $( if let Some($it) = ast::$ast::cast($node.clone()) { $res } else )*
+        { $catch_all }
+    }};
+}
+
+fn main() {
+    let anchor = match_ast! {
+        match parent {
+            as => {},
+            _ => return None
+        }
+    };
+}"#,
+    );
+}
+
 #[test]
 fn macro_resolve() {
     // Regression test for a path resolution bug introduced with inner item handling.

crates/hir_def/src/body/tests/block.rs

@@ -371,3 +371,27 @@ fn outer() {
         "#]],
     );
 }
+
+#[test]
+fn stmt_macro_expansion_with_trailing_expr() {
+    cov_mark::check!(macro_stmt_with_trailing_macro_expr);
+    check_at(
+        r#"
+macro_rules! mac {
+    () => { mac!($) };
+    ($x:tt) => { fn inner() {} };
+}
+fn foo() {
+    mac!();
+    $0
+}
+        "#,
+        expect![[r#"
+            block scope
+            inner: v
+
+            crate
+            foo: v
+        "#]],
+    )
+}

crates/hir_def/src/item_tree/lower.rs

@@ -48,22 +48,33 @@ impl<'a> Ctx<'a> {
     pub(super) fn lower_macro_stmts(mut self, stmts: ast::MacroStmts) -> ItemTree {
         self.tree.top_level = stmts
             .statements()
-            .filter_map(|stmt| match stmt {
-                ast::Stmt::Item(item) => Some(item),
-                // Macro calls can be both items and expressions. The syntax library always treats
-                // them as expressions here, so we undo that.
-                ast::Stmt::ExprStmt(es) => match es.expr()? {
-                    ast::Expr::MacroCall(call) => {
-                        cov_mark::hit!(macro_call_in_macro_stmts_is_added_to_item_tree);
-                        Some(call.into())
-                    }
+            .filter_map(|stmt| {
+                match stmt {
+                    ast::Stmt::Item(item) => Some(item),
+                    // Macro calls can be both items and expressions. The syntax library always treats
+                    // them as expressions here, so we undo that.
+                    ast::Stmt::ExprStmt(es) => match es.expr()? {
+                        ast::Expr::MacroExpr(expr) => {
+                            cov_mark::hit!(macro_call_in_macro_stmts_is_added_to_item_tree);
+                            Some(expr.macro_call()?.into())
+                        }
+                        _ => None,
+                    },
                     _ => None,
-                },
-                _ => None,
+                }
             })
             .flat_map(|item| self.lower_mod_item(&item))
             .collect();
 
+        if let Some(ast::Expr::MacroExpr(tail_macro)) = stmts.expr() {
+            if let Some(call) = tail_macro.macro_call() {
+                cov_mark::hit!(macro_stmt_with_trailing_macro_expr);
+                if let Some(mod_item) = self.lower_mod_item(&call.into()) {
+                    self.tree.top_level.push(mod_item);
+                }
+            }
+        }
+
         self.tree
     }
 
@@ -75,7 +86,7 @@ impl<'a> Ctx<'a> {
                 // Macro calls can be both items and expressions. The syntax library always treats
                 // them as expressions here, so we undo that.
                 ast::Stmt::ExprStmt(es) => match es.expr()? {
-                    ast::Expr::MacroCall(call) => self.lower_mod_item(&call.into()),
+                    ast::Expr::MacroExpr(expr) => self.lower_mod_item(&expr.macro_call()?.into()),
                     _ => None,
                 },
                 _ => None,

crates/hir_def/src/macro_expansion_tests/builtin_fn_macro.rs

@@ -249,8 +249,7 @@ macro_rules! format_args {
 
 fn main() {
     let _ =
-        // +errors
-        format_args!("{} {:?}", a.);
+        format_args!/*+errors*/("{} {:?}", a.);
 }
 "#,
         expect![[r##"

crates/hir_def/src/macro_expansion_tests/mbe.rs

@@ -530,8 +530,7 @@ macro_rules! m {
 }
 
 fn f() -> i32 {
-    // +tree
-    m!{}
+    m!/*+tree*/{}
 }
 "#,
         expect![[r#"

crates/hir_expand/src/lib.rs

@@ -885,6 +885,16 @@ impl ExpandTo {
             None => return ExpandTo::Statements,
         };
 
+        // FIXME: macros in statement position are treated as expression statements, they should
+        // probably be their own statement kind. The *grand*parent indicates what's valid.
+        if parent.kind() == MACRO_EXPR
+            && parent
+                .parent()
+                .map_or(true, |p| matches!(p.kind(), EXPR_STMT | STMT_LIST | MACRO_STMTS))
+        {
+            return ExpandTo::Statements;
+        }
+
         match parent.kind() {
             MACRO_ITEMS | SOURCE_FILE | ITEM_LIST => ExpandTo::Items,
             MACRO_STMTS | EXPR_STMT | STMT_LIST => ExpandTo::Statements,
@@ -895,23 +905,10 @@ impl ExpandTo {
             | CLOSURE_EXPR | FIELD_EXPR | FOR_EXPR | IF_EXPR | INDEX_EXPR | LET_EXPR
             | MATCH_ARM | MATCH_EXPR | MATCH_GUARD | METHOD_CALL_EXPR | PAREN_EXPR | PATH_EXPR
             | PREFIX_EXPR | RANGE_EXPR | RECORD_EXPR_FIELD | REF_EXPR | RETURN_EXPR | TRY_EXPR
-            | TUPLE_EXPR | WHILE_EXPR => ExpandTo::Expr,
+            | TUPLE_EXPR | WHILE_EXPR | MACRO_EXPR => ExpandTo::Expr,
             _ => {
-                match ast::LetStmt::cast(parent) {
-                    Some(let_stmt) => {
-                        if let Some(true) = let_stmt.initializer().map(|it| it.syntax() == syn) {
-                            ExpandTo::Expr
-                        } else if let Some(true) = let_stmt.ty().map(|it| it.syntax() == syn) {
-                            ExpandTo::Type
-                        } else {
-                            ExpandTo::Pattern
-                        }
-                    }
-                    None => {
-                        // Unknown , Just guess it is `Items`
-                        ExpandTo::Items
-                    }
-                }
+                // Unknown , Just guess it is `Items`
+                ExpandTo::Items
             }
         }
     }

crates/hir_ty/src/tests/regression.rs

@@ -444,34 +444,6 @@ fn test() {
     );
 }
 
-#[test]
-fn issue_3642_bad_macro_stackover() {
-    check_no_mismatches(
-        r#"
-#[macro_export]
-macro_rules! match_ast {
-    (match $node:ident { $($tt:tt)* }) => { match_ast!(match ($node) { $($tt)* }) };
-
-    (match ($node:expr) {
-        $( ast::$ast:ident($it:ident) => $res:expr, )*
-        _ => $catch_all:expr $(,)?
-    }) => {{
-        $( if let Some($it) = ast::$ast::cast($node.clone()) { $res } else )*
-        { $catch_all }
-    }};
-}
-
-fn main() {
-    let anchor = match_ast! {
-        match parent {
-            as => {},
-            _ => return None
-        }
-    };
-}"#,
-    );
-}
-
 #[test]
 fn issue_3999_slice() {
     check_infer(

crates/ide/src/highlight_related.rs

@@ -156,7 +156,7 @@ fn highlight_exit_points(
                     highlights.push(HighlightedRange { category: None, range: token.text_range() });
                 }
             }
-            ast::Expr::MethodCallExpr(_) | ast::Expr::CallExpr(_) | ast::Expr::MacroCall(_) => {
+            ast::Expr::MethodCallExpr(_) | ast::Expr::CallExpr(_) | ast::Expr::MacroExpr(_) => {
                 if sema.type_of_expr(&expr).map_or(false, |ty| ty.original.is_never()) {
                     highlights.push(HighlightedRange {
                         category: None,