Auto merge of rust-lang#13156 - Veykril:macro-stmts, r=Veykril

Remove hir::Expr::MacroStmts

This hir expression isn't needed and only existed as it was simpler to
deal with at first as it gave us a direct mapping for the ast version of
the same construct. This PR removes it, properly handling the statements
that are introduced by macro call expressions.

Author: bors

crates/hir-def/src/body/lower.rs

@@ -550,20 +550,6 @@ impl ExprCollector<'_> {
                     None => self.alloc_expr(Expr::Missing, syntax_ptr),
                 }
             }
-            ast::Expr::MacroStmts(e) => {
-                let statements: Box<[_]> =
-                    e.statements().filter_map(|s| self.collect_stmt(s)).collect();
-                let tail = e.expr().map(|e| self.collect_expr(e));
-
-                if e.syntax().children().next().is_none() {
-                    // HACK: make sure that macros that expand to nothing aren't treated as a `()`
-                    // expression when used in block tail position.
-                    cov_mark::hit!(empty_macro_in_trailing_position_is_removed);
-                    return None;
-                }
-
-                self.alloc_expr(Expr::MacroStmts { tail, statements }, syntax_ptr)
-            }
             ast::Expr::UnderscoreExpr(_) => self.alloc_expr(Expr::Underscore, syntax_ptr),
         })
     }
@@ -640,11 +626,46 @@ impl ExprCollector<'_> {
         }
     }
 
-    fn collect_stmt(&mut self, s: ast::Stmt) -> Option<Statement> {
+    fn collect_macro_as_stmt(
+        &mut self,
+        statements: &mut Vec<Statement>,
+        mac: ast::MacroExpr,
+    ) -> Option<ExprId> {
+        let mac_call = mac.macro_call()?;
+        let syntax_ptr = AstPtr::new(&ast::Expr::from(mac));
+        let macro_ptr = AstPtr::new(&mac_call);
+        let expansion = self.collect_macro_call(
+            mac_call,
+            macro_ptr,
+            false,
+            |this, expansion: Option<ast::MacroStmts>| match expansion {
+                Some(expansion) => {
+                    expansion.statements().for_each(|stmt| this.collect_stmt(statements, stmt));
+                    expansion.expr().and_then(|expr| match expr {
+                        ast::Expr::MacroExpr(mac) => this.collect_macro_as_stmt(statements, mac),
+                        expr => Some(this.collect_expr(expr)),
+                    })
+                }
+                None => None,
+            },
+        );
+        match expansion {
+            Some(tail) => {
+                // Make the macro-call point to its expanded expression so we can query
+                // semantics on syntax pointers to the macro
+                let src = self.expander.to_source(syntax_ptr);
+                self.source_map.expr_map.insert(src, tail);
+                Some(tail)
+            }
+            None => None,
+        }
+    }
+
+    fn collect_stmt(&mut self, statements: &mut Vec<Statement>, s: ast::Stmt) {
         match s {
             ast::Stmt::LetStmt(stmt) => {
                 if self.check_cfg(&stmt).is_none() {
-                    return None;
+                    return;
                 }
                 let pat = self.collect_pat_opt(stmt.pat());
                 let type_ref =
@@ -654,61 +675,26 @@ impl ExprCollector<'_> {
                     .let_else()
                     .and_then(|let_else| let_else.block_expr())
                     .map(|block| self.collect_block(block));
-                Some(Statement::Let { pat, type_ref, initializer, else_branch })
+                statements.push(Statement::Let { pat, type_ref, initializer, else_branch });
             }
             ast::Stmt::ExprStmt(stmt) => {
                 let expr = stmt.expr();
-                if let Some(expr) = &expr {
-                    if self.check_cfg(expr).is_none() {
-                        return None;
-                    }
+                match &expr {
+                    Some(expr) if self.check_cfg(expr).is_none() => return,
+                    _ => (),
                 }
                 let has_semi = stmt.semicolon_token().is_some();
                 // Note that macro could be expanded to multiple statements
-                if let Some(expr @ ast::Expr::MacroExpr(mac)) = &expr {
-                    let mac_call = mac.macro_call()?;
-                    let syntax_ptr = AstPtr::new(expr);
-                    let macro_ptr = AstPtr::new(&mac_call);
-                    let stmt = self.collect_macro_call(
-                        mac_call,
-                        macro_ptr,
-                        false,
-                        |this, expansion: Option<ast::MacroStmts>| match expansion {
-                            Some(expansion) => {
-                                let statements = expansion
-                                    .statements()
-                                    .filter_map(|stmt| this.collect_stmt(stmt))
-                                    .collect();
-                                let tail = expansion.expr().map(|expr| this.collect_expr(expr));
-
-                                let mac_stmts = this.alloc_expr(
-                                    Expr::MacroStmts { tail, statements },
-                                    AstPtr::new(&ast::Expr::MacroStmts(expansion)),
-                                );
-
-                                Some(mac_stmts)
-                            }
-                            None => None,
-                        },
-                    );
-
-                    let expr = match stmt {
-                        Some(expr) => {
-                            // Make the macro-call point to its expanded expression so we can query
-                            // semantics on syntax pointers to the macro
-                            let src = self.expander.to_source(syntax_ptr);
-                            self.source_map.expr_map.insert(src, expr);
-                            expr
-                        }
-                        None => self.alloc_expr(Expr::Missing, syntax_ptr),
-                    };
-                    Some(Statement::Expr { expr, has_semi })
+                if let Some(ast::Expr::MacroExpr(mac)) = expr {
+                    if let Some(expr) = self.collect_macro_as_stmt(statements, mac) {
+                        statements.push(Statement::Expr { expr, has_semi })
+                    }
                 } else {
                     let expr = self.collect_expr_opt(expr);
-                    Some(Statement::Expr { expr, has_semi })
+                    statements.push(Statement::Expr { expr, has_semi });
                 }
             }
-            ast::Stmt::Item(_item) => None,
+            ast::Stmt::Item(_item) => (),
         }
     }
 
@@ -729,9 +715,12 @@ impl ExprCollector<'_> {
         let prev_def_map = mem::replace(&mut self.expander.def_map, def_map);
         let prev_local_module = mem::replace(&mut self.expander.module, module);
 
-        let mut statements: Vec<_> =
-            block.statements().filter_map(|s| self.collect_stmt(s)).collect();
-        let tail = block.tail_expr().and_then(|e| self.maybe_collect_expr(e));
+        let mut statements = Vec::new();
+        block.statements().for_each(|s| self.collect_stmt(&mut statements, s));
+        let tail = block.tail_expr().and_then(|e| match e {
+            ast::Expr::MacroExpr(mac) => self.collect_macro_as_stmt(&mut statements, mac),
+            expr => self.maybe_collect_expr(expr),
+        });
         let tail = tail.or_else(|| {
             let stmt = statements.pop()?;
             if let Statement::Expr { expr, has_semi: false } = stmt {

crates/hir-def/src/body/pretty.rs

@@ -422,19 +422,6 @@ impl<'a> Printer<'a> {
                 }
                 w!(self, "}}");
             }
-            Expr::MacroStmts { statements, tail } => {
-                w!(self, "{{ // macro statements");
-                self.indented(|p| {
-                    for stmt in statements.iter() {
-                        p.print_stmt(stmt);
-                    }
-                    if let Some(tail) = tail {
-                        p.print_expr(*tail);
-                    }
-                });
-                self.newline();
-                w!(self, "}}");
-            }
         }
     }
 

crates/hir-def/src/body/scope.rs

@@ -176,9 +176,6 @@ fn compute_expr_scopes(expr: ExprId, body: &Body, scopes: &mut ExprScopes, scope
 
     scopes.set_scope(expr, *scope);
     match &body[expr] {
-        Expr::MacroStmts { statements, tail } => {
-            compute_block_scopes(statements, *tail, body, scopes, scope);
-        }
         Expr::Block { statements, tail, id, label } => {
             let mut scope = scopes.new_block_scope(*scope, *id, make_label(label));
             // Overwrite the old scope for the block expr, so that every block scope can be found

crates/hir-def/src/expr.rs

@@ -206,10 +206,6 @@ pub enum Expr {
     Unsafe {
         body: ExprId,
     },
-    MacroStmts {
-        statements: Box<[Statement]>,
-        tail: Option<ExprId>,
-    },
     Array(Array),
     Literal(Literal),
     Underscore,
@@ -263,7 +259,7 @@ impl Expr {
             Expr::Let { expr, .. } => {
                 f(*expr);
             }
-            Expr::MacroStmts { tail, statements } | Expr::Block { statements, tail, .. } => {
+            Expr::Block { statements, tail, .. } => {
                 for stmt in statements.iter() {
                     match stmt {
                         Statement::Let { initializer, .. } => {

crates/hir-expand/src/lib.rs

@@ -969,7 +969,7 @@ impl ExpandTo {
         if parent.kind() == MACRO_EXPR
             && parent
                 .parent()
-                .map_or(true, |p| matches!(p.kind(), EXPR_STMT | STMT_LIST | MACRO_STMTS))
+                .map_or(false, |p| matches!(p.kind(), EXPR_STMT | STMT_LIST | MACRO_STMTS))
         {
             return ExpandTo::Statements;
         }

crates/hir-ty/src/infer/expr.rs

@@ -794,9 +794,6 @@ impl<'a> InferenceContext<'a> {
                     None => self.table.new_float_var(),
                 },
             },
-            Expr::MacroStmts { tail, statements } => {
-                self.infer_block(tgt_expr, statements, *tail, expected)
-            }
             Expr::Underscore => {
                 // Underscore expressions may only appear in assignee expressions,
                 // which are handled by `infer_assignee_expr()`, so any underscore

crates/hir-ty/src/tests/macros.rs

@@ -193,8 +193,6 @@ fn expr_macro_def_expanded_in_various_places() {
             !0..6 '1isize': isize
             !0..6 '1isize': isize
             !0..6 '1isize': isize
-            !0..6 '1isize': isize
-            !0..6 '1isize': isize
             39..442 '{     ...!(); }': ()
             73..94 'spam!(...am!())': {unknown}
             100..119 'for _ ...!() {}': ()
@@ -276,8 +274,6 @@ fn expr_macro_rules_expanded_in_various_places() {
             !0..6 '1isize': isize
             !0..6 '1isize': isize
             !0..6 '1isize': isize
-            !0..6 '1isize': isize
-            !0..6 '1isize': isize
             53..456 '{     ...!(); }': ()
             87..108 'spam!(...am!())': {unknown}
             114..133 'for _ ...!() {}': ()
@@ -312,7 +308,6 @@ fn expr_macro_expanded_in_stmts() {
         }
         "#,
         expect![[r#"
-            !0..8 'leta=();': ()
             !3..4 'a': ()
             !5..7 '()': ()
             57..84 '{     ...); } }': ()
@@ -321,7 +316,7 @@ fn expr_macro_expanded_in_stmts() {
 }
 
 #[test]
-fn recurisve_macro_expanded_in_stmts() {
+fn recursive_macro_expanded_in_stmts() {
     check_infer(
         r#"
         macro_rules! ng {
@@ -340,11 +335,6 @@ fn recurisve_macro_expanded_in_stmts() {
         }
         "#,
         expect![[r#"
-            !0..7 'leta=3;': ()
-            !0..13 'ng!{[leta=3]}': ()
-            !0..13 'ng!{[leta=]3}': ()
-            !0..13 'ng!{[leta]=3}': ()
-            !0..13 'ng!{[let]a=3}': ()
             !3..4 'a': i32
             !5..6 '3': i32
             196..237 '{     ...= a; }': ()
@@ -369,8 +359,6 @@ fn recursive_inner_item_macro_rules() {
         "#,
         expect![[r#"
             !0..1 '1': i32
-            !0..7 'mac!($)': ()
-            !0..26 'macro_...>{1};}': ()
             107..143 '{     ...!(); }': ()
             129..130 'a': i32
         "#]],

crates/hir-ty/src/tests/regression.rs

@@ -573,7 +573,6 @@ fn issue_6811() {
         }
         "#,
         expect![[r#"
-            !0..16 'let_a=...t_b=1;': ()
             !3..5 '_a': i32
             !6..7 '1': i32
             !11..13 '_b': i32
@@ -1679,7 +1678,6 @@ fn main() {
 
 #[test]
 fn trailing_empty_macro() {
-    cov_mark::check!(empty_macro_in_trailing_position_is_removed);
     check_no_mismatches(
         r#"
 macro_rules! m2 {

crates/hir-ty/src/tests/simple.rs

@@ -2549,7 +2549,6 @@ impl B for Astruct {}
         expect![[r#"
             569..573 'self': Box<[T], A>
             602..634 '{     ...     }': Vec<T, A>
-            612..628 'unimpl...ted!()': Vec<T, A>
             648..761 '{     ...t]); }': ()
             658..661 'vec': Vec<i32, Global>
             664..679 '<[_]>::into_vec': fn into_vec<i32, Global>(Box<[i32], Global>) -> Vec<i32, Global>

crates/hir/src/source_analyzer.rs

@@ -140,11 +140,19 @@ impl SourceAnalyzer {
     ) -> Option<InFile<ast::Expr>> {
         let macro_file = self.body_source_map()?.node_macro_file(expr.as_ref())?;
         let expanded = db.parse_or_expand(macro_file)?;
-
-        let res = match ast::MacroCall::cast(expanded.clone()) {
-            Some(call) => self.expand_expr(db, InFile::new(macro_file, call))?,
-            _ => InFile::new(macro_file, ast::Expr::cast(expanded)?),
+        let res = if let Some(stmts) = ast::MacroStmts::cast(expanded.clone()) {
+            match stmts.expr()? {
+                ast::Expr::MacroExpr(mac) => {
+                    self.expand_expr(db, InFile::new(macro_file, mac.macro_call()?))?
+                }
+                expr => InFile::new(macro_file, expr),
+            }
+        } else if let Some(call) = ast::MacroCall::cast(expanded.clone()) {
+            self.expand_expr(db, InFile::new(macro_file, call))?
+        } else {
+            InFile::new(macro_file, ast::Expr::cast(expanded)?)
         };
+
         Some(res)
     }