macros: Replace macro invocations with expanded nodes

Different parsing functions need to be called based on the context
surrounding the macro invocation. This commit adds a flowchart trying to
explain the base resolving rules

Macro expansion happens at the same level as stripping, where nodes
might get removed if they are gated behind an unmet predicate. We also
perform macro expansion during this visitor's pass.

What we can do is thus to replace macro invocations with new items that
might have resulted from macro expansion: Since we're already mutating
numerous elements by removing them if they should be stripped, we can
also add elements if they should be expanded.

This commit also "fixes" macro test cases so that they are now accepted
by the new parser, which is more strict than it should for now.

Co-authored-by: SimplyTheOther <simplytheother@gmail.com>
Co-authored-by: philberty <philip.herron@embecosm.com>

30 files changed, 475 insertions, 255 deletions

diff --git a/gcc/rust/ast/rust-ast.h b/gcc/rust/ast/rust-ast.h
index 08ecd18..d07501e 100644
--- a/gcc/rust/ast/rust-ast.h
+++ b/gcc/rust/ast/rust-ast.h
@@ -1491,21 +1491,21 @@ public:
   };
 
   SingleASTNode (std::unique_ptr<Expr> expr)
-    : type (EXPRESSION), expr (std::move (expr)), item (nullptr), stmt (nullptr)
+    : kind (EXPRESSION), expr (std::move (expr)), item (nullptr), stmt (nullptr)
   {}
 
   SingleASTNode (std::unique_ptr<Item> item)
-    : type (ITEM), expr (nullptr), item (std::move (item)), stmt (nullptr)
+    : kind (ITEM), expr (nullptr), item (std::move (item)), stmt (nullptr)
   {}
 
   SingleASTNode (std::unique_ptr<Stmt> stmt)
-    : type (STMT), expr (nullptr), item (nullptr), stmt (std::move (stmt))
+    : kind (STMT), expr (nullptr), item (nullptr), stmt (std::move (stmt))
   {}
 
   SingleASTNode (SingleASTNode const &other)
   {
-    type = other.type;
-    switch (type)
+    kind = other.kind;
+    switch (kind)
       {
       case EXPRESSION:
 	expr = other.expr->clone_expr ();
@@ -1523,8 +1523,8 @@ public:
 
   SingleASTNode operator= (SingleASTNode const &other)
   {
-    type = other.type;
-    switch (type)
+    kind = other.kind;
+    switch (kind)
       {
       case EXPRESSION:
 	expr = other.expr->clone_expr ();
@@ -1544,27 +1544,52 @@ public:
   SingleASTNode (SingleASTNode &&other) = default;
   SingleASTNode &operator= (SingleASTNode &&other) = default;
 
-  std::unique_ptr<Expr> &get_expr ()
+  NodeType get_kind () const { return kind; }
+
+  std::unique_ptr<Expr> &get_inner ()
   {
-    rust_assert (type == EXPRESSION);
+    rust_assert (kind == EXPRESSION);
     return expr;
   }
 
   std::unique_ptr<Item> &get_item ()
   {
-    rust_assert (type == ITEM);
+    rust_assert (kind == ITEM);
     return item;
   }
 
   std::unique_ptr<Stmt> &get_stmt ()
   {
-    rust_assert (type == STMT);
+    rust_assert (kind == STMT);
     return stmt;
   }
 
+  /**
+   * Access the inner nodes and take ownership of them.
+   * You can only call these functions once per node
+   */
+
+  std::unique_ptr<Stmt> take_stmt ()
+  {
+    rust_assert (!is_error ());
+    return std::move (stmt);
+  }
+
+  std::unique_ptr<Expr> take_expr ()
+  {
+    rust_assert (!is_error ());
+    return std::move (expr);
+  }
+
+  std::unique_ptr<Item> take_item ()
+  {
+    rust_assert (!is_error ());
+    return std::move (item);
+  }
+
   void accept_vis (ASTVisitor &vis)
   {
-    switch (type)
+    switch (kind)
       {
       case EXPRESSION:
 	expr->accept_vis (vis);
@@ -1580,8 +1605,38 @@ public:
       }
   }
 
+  bool is_error ()
+  {
+    switch (kind)
+      {
+      case EXPRESSION:
+	return expr == nullptr;
+      case ITEM:
+	return item == nullptr;
+      case STMT:
+	return stmt == nullptr;
+      default:
+	return true;
+      }
+  }
+
+  std::string as_string ()
+  {
+    switch (kind)
+      {
+      case EXPRESSION:
+	return "Expr: " + expr->as_string ();
+      case ITEM:
+	return "Item: " + item->as_string ();
+      case STMT:
+	return "Stmt: " + stmt->as_string ();
+      default:
+	return "";
+      }
+  }
+
 private:
-  NodeType type;
+  NodeType kind;
 
   // FIXME make this a union
   std::unique_ptr<Expr> expr;
@@ -1604,11 +1659,18 @@ private:
    * ability for a macro to expand to two statements, for instance. */
 
   std::vector<SingleASTNode> nodes;
+  bool fragment_is_error;
 
 public:
-  ASTFragment (std::vector<SingleASTNode> nodes) : nodes (std::move (nodes)) {}
+  ASTFragment (std::vector<SingleASTNode> nodes, bool fragment_is_error = false)
+    : nodes (std::move (nodes)), fragment_is_error (fragment_is_error)
+  {
+    if (fragment_is_error)
+      rust_assert (nodes.empty ());
+  }
 
   ASTFragment (ASTFragment const &other)
+    : fragment_is_error (other.fragment_is_error)
   {
     nodes.clear ();
     nodes.reserve (other.nodes.size ());
@@ -1620,18 +1682,47 @@ public:
 
   ASTFragment &operator= (ASTFragment const &other)
   {
+    fragment_is_error = other.fragment_is_error;
     nodes.clear ();
     nodes.reserve (other.nodes.size ());
     for (auto &n : other.nodes)
       {
 	nodes.push_back (n);
       }
+
     return *this;
   }
 
   static ASTFragment create_empty () { return ASTFragment ({}); }
+  static ASTFragment create_error () { return ASTFragment ({}, true); }
 
   std::vector<SingleASTNode> &get_nodes () { return nodes; }
+  bool is_error () const { return fragment_is_error; }
+
+  bool should_expand () const { return !is_error () && !nodes.empty (); }
+
+  /**
+   * We need to make a special case for Expression fragments as only one
+   * Node will be extracted from the `nodes` vector
+   */
+
+  bool is_expression_fragment () const
+  {
+    return nodes.size () == 1
+	   && nodes[0].get_kind () == SingleASTNode::NodeType::EXPRESSION;
+  }
+
+  std::unique_ptr<Expr> take_expression_fragment ()
+  {
+    rust_assert (is_expression_fragment ());
+    return nodes[0].take_expr ();
+  }
+
+  void accept_vis (ASTVisitor &vis)
+  {
+    for (auto &node : nodes)
+      node.accept_vis (vis);
+  }
 };
 
 // A crate AST object - holds all the data for a single compilation unit
diff --git a/gcc/rust/ast/rust-macro.h b/gcc/rust/ast/rust-macro.h
index d625ad2d..bf07dd0 100644
--- a/gcc/rust/ast/rust-macro.h
+++ b/gcc/rust/ast/rust-macro.h
@@ -464,9 +464,6 @@ class MacroInvocation : public TypeNoBounds,
   MacroInvocData invoc_data;
   Location locus;
 
-  // this is the expanded macro
-  ASTFragment fragment;
-
   // Important for when we actually expand the macro
   bool is_semi_coloned;
 
@@ -480,7 +477,6 @@ public:
 		   bool is_semi_coloned = false)
     : outer_attrs (std::move (outer_attrs)),
       invoc_data (std::move (invoc_data)), locus (locus),
-      fragment (ASTFragment::create_empty ()),
       is_semi_coloned (is_semi_coloned),
       node_id (Analysis::Mappings::get ()->get_next_node_id ())
   {}
@@ -513,10 +509,6 @@ public:
 
   MacroInvocData &get_invoc_data () { return invoc_data; }
 
-  ASTFragment &get_fragment () { return fragment; }
-
-  void set_fragment (ASTFragment &&f) { fragment = std::move (f); }
-
   bool has_semicolon () const { return is_semi_coloned; }
 
 protected:
diff --git a/gcc/rust/expand/rust-macro-expand.cc b/gcc/rust/expand/rust-macro-expand.cc
index 84a526c..fab7f56 100644
--- a/gcc/rust/expand/rust-macro-expand.cc
+++ b/gcc/rust/expand/rust-macro-expand.cc
@@ -331,12 +331,6 @@ public:
       expander.expand_invoc_semi (macro_invoc);
     else
       expander.expand_invoc (macro_invoc);
-
-    // we need to visit the expanded fragments since it may need cfg
-    // expansion
-    // and it may be recursive
-    for (auto &node : macro_invoc.get_fragment ().get_nodes ())
-      node.accept_vis (*this);
   }
 
   void visit (AST::PathInExpression &path) override
@@ -531,10 +525,25 @@ public:
 
     /* should have no possibility for outer attrs as would be parsed
      * with outer expr */
-    expr.get_left_expr ()->accept_vis (*this);
+    auto &l_expr = expr.get_left_expr ();
+    l_expr->accept_vis (*this);
+    auto l_fragment = expander.take_expanded_fragment ();
+    if (l_fragment.should_expand ())
+      {
+	l_fragment.accept_vis (*this);
+	l_expr = l_fragment.take_expression_fragment ();
+      }
+
     /* should syntactically not have outer attributes, though this may
      * not have worked in practice */
-    expr.get_right_expr ()->accept_vis (*this);
+    auto &r_expr = expr.get_right_expr ();
+    r_expr->accept_vis (*this);
+    auto r_fragment = expander.take_expanded_fragment ();
+    if (r_fragment.should_expand ())
+      {
+	r_fragment.accept_vis (*this);
+	r_expr = r_fragment.take_expression_fragment ();
+      }
 
     // ensure that they are not marked for strip
     if (expr.get_left_expr ()->is_marked_for_strip ())
@@ -645,10 +654,25 @@ public:
 
     /* should have no possibility for outer attrs as would be parsed
      * with outer expr */
-    expr.get_left_expr ()->accept_vis (*this);
+    auto &l_expr = expr.get_left_expr ();
+    l_expr->accept_vis (*this);
+    auto l_frag = expander.take_expanded_fragment ();
+    if (l_frag.should_expand ())
+      {
+	l_frag.accept_vis (*this);
+	l_expr = l_frag.take_expression_fragment ();
+      }
+
     /* should syntactically not have outer attributes, though this may
      * not have worked in practice */
-    expr.get_right_expr ()->accept_vis (*this);
+    auto &r_expr = expr.get_right_expr ();
+    r_expr->accept_vis (*this);
+    auto r_frag = expander.take_expanded_fragment ();
+    if (r_frag.should_expand ())
+      {
+	r_frag.accept_vis (*this);
+	r_expr = r_frag.take_expression_fragment ();
+      }
 
     // ensure that they are not marked for strip
     if (expr.get_left_expr ()->is_marked_for_strip ())
@@ -975,7 +999,33 @@ public:
 
     /* spec says outer attributes are specifically allowed for elements
      * of call expressions, so full stripping possible */
+    // FIXME: Arthur: Figure out how to refactor this - This is similar to
+    // expanding items in the crate or stmts in blocks
     expand_pointer_allow_strip (expr.get_params ());
+    auto &params = expr.get_params ();
+    for (auto it = params.begin (); it != params.end ();)
+      {
+	auto &stmt = *it;
+
+	stmt->accept_vis (*this);
+
+	auto fragment = expander.take_expanded_fragment ();
+	if (fragment.should_expand ())
+	  {
+	    fragment.accept_vis (*this);
+	    // Remove the current expanded invocation
+	    it = params.erase (it);
+	    for (auto &node : fragment.get_nodes ())
+	      {
+		it = params.insert (it, node.take_expr ());
+		it++;
+	      }
+	  }
+	else if (stmt->is_marked_for_strip ())
+	  it = params.erase (it);
+	else
+	  it++;
+      }
   }
   void visit (AST::MethodCallExpr &expr) override
   {
@@ -1072,7 +1122,31 @@ public:
       }
 
     // strip all statements
-    expand_pointer_allow_strip (expr.get_statements ());
+    auto &stmts = expr.get_statements ();
+    for (auto it = stmts.begin (); it != stmts.end ();)
+      {
+	auto &stmt = *it;
+
+	stmt->accept_vis (*this);
+
+	auto fragment = expander.take_expanded_fragment ();
+	if (fragment.should_expand ())
+	  {
+	    fragment.accept_vis (*this);
+	    // Remove the current expanded invocation
+	    it = stmts.erase (it);
+	    for (auto &node : fragment.get_nodes ())
+	      {
+		it = stmts.insert (it, node.take_stmt ());
+		it++;
+	      }
+	  }
+
+	else if (stmt->is_marked_for_strip ())
+	  it = stmts.erase (it);
+	else
+	  it++;
+      }
 
     // strip tail expression if exists - can actually fully remove it
     if (expr.has_tail_expr ())
@@ -1080,6 +1154,12 @@ public:
 	auto &tail_expr = expr.get_tail_expr ();
 
 	tail_expr->accept_vis (*this);
+	auto fragment = expander.take_expanded_fragment ();
+	if (fragment.should_expand ())
+	  {
+	    fragment.accept_vis (*this);
+	    tail_expr = fragment.take_expression_fragment ();
+	  }
 
 	if (tail_expr->is_marked_for_strip ())
 	  expr.strip_tail_expr ();
@@ -2819,10 +2899,18 @@ public:
       {
 	auto &init_expr = stmt.get_init_expr ();
 	init_expr->accept_vis (*this);
+
 	if (init_expr->is_marked_for_strip ())
 	  rust_error_at (init_expr->get_locus (),
 			 "cannot strip expression in this position - outer "
 			 "attributes not allowed");
+
+	auto fragment = expander.take_expanded_fragment ();
+	if (fragment.should_expand ())
+	  {
+	    fragment.accept_vis (*this);
+	    init_expr = fragment.take_expression_fragment ();
+	  }
       }
   }
   void visit (AST::ExprStmtWithoutBlock &stmt) override
@@ -3125,7 +3213,7 @@ MacroExpander::expand_decl_macro (Location invoc_locus,
       RichLocation r (invoc_locus);
       r.add_range (rules_def.get_locus ());
       rust_error_at (r, "Failed to match any rule within macro");
-      return AST::ASTFragment::create_empty ();
+      return AST::ASTFragment::create_error ();
     }
 
   return transcribe_rule (*matched_rule, invoc_token_tree, matched_fragments,
@@ -3187,10 +3275,10 @@ MacroExpander::expand_invoc (AST::MacroInvocation &invoc)
     fragment
       = expand_decl_macro (invoc.get_locus (), invoc_data, *rules_def, false);
 
-  // lets attach this fragment to the invocation
-  invoc.set_fragment (std::move (fragment));
+  set_expanded_fragment (std::move (fragment));
 }
 
+// FIXME: Arthur: Refactor these two functions, they're really similar
 void
 MacroExpander::expand_invoc_semi (AST::MacroInvocation &invoc)
 {
@@ -3228,8 +3316,7 @@ MacroExpander::expand_invoc_semi (AST::MacroInvocation &invoc)
     fragment
       = expand_decl_macro (invoc.get_locus (), invoc_data, *rules_def, true);
 
-  // lets attach this fragment to the invocation
-  invoc.set_fragment (std::move (fragment));
+  set_expanded_fragment (std::move (fragment));
 }
 
 /* Determines whether any cfg predicate is false and hence item with attributes
@@ -3363,10 +3450,22 @@ MacroExpander::expand_crate ()
       // mark for stripping if required
       item->accept_vis (attr_visitor);
 
-      if (item->is_marked_for_strip ())
+      auto fragment = take_expanded_fragment ();
+      if (fragment.should_expand ())
+	{
+	  fragment.accept_vis (attr_visitor);
+	  // Remove the current expanded invocation
+	  it = items.erase (it);
+	  for (auto &node : fragment.get_nodes ())
+	    {
+	      it = items.insert (it, node.take_item ());
+	      it++;
+	    }
+	}
+      else if (item->is_marked_for_strip ())
 	it = items.erase (it);
       else
-	++it;
+	it++;
     }
 
   pop_context ();
@@ -3756,6 +3855,73 @@ MacroExpander::match_repetition (Parser<MacroInvocLexer> &parser,
   return res;
 }
 
+/**
+ * Helper function to refactor calling a parsing function 0 or more times
+ */
+static std::vector<AST::SingleASTNode>
+parse_many (Parser<MacroInvocLexer> &parser, TokenId &delimiter,
+	    std::function<AST::SingleASTNode ()> parse_fn)
+{
+  std::vector<AST::SingleASTNode> nodes;
+
+  while (true)
+    {
+      if (parser.peek_current_token ()->get_id () == delimiter)
+	break;
+
+      auto node = parse_fn ();
+      nodes.emplace_back (std::move (node));
+    }
+
+  return nodes;
+}
+
+/**
+ * Transcribe 0 or more items from a macro invocation
+ *
+ * @param parser Parser to extract items from
+ * @param delimiter Id of the token on which parsing should stop
+ */
+static std::vector<AST::SingleASTNode>
+transcribe_many_items (Parser<MacroInvocLexer> &parser, TokenId &delimiter)
+{
+  return parse_many (parser, delimiter, [&parser] () {
+    auto item = parser.parse_item (true);
+    return AST::SingleASTNode (std::move (item));
+  });
+}
+
+/**
+ * Transcribe 0 or more statements from a macro invocation
+ *
+ * @param parser Parser to extract statements from
+ * @param delimiter Id of the token on which parsing should stop
+ */
+static std::vector<AST::SingleASTNode>
+transcribe_many_stmts (Parser<MacroInvocLexer> &parser, TokenId &delimiter)
+{
+  // FIXME: This is invalid! It needs to also handle cases where the macro
+  // transcriber is an expression, but since the macro call is followed by
+  // a semicolon, it's a valid ExprStmt
+  return parse_many (parser, delimiter, [&parser] () {
+    auto stmt = parser.parse_stmt ();
+    return AST::SingleASTNode (std::move (stmt));
+  });
+}
+
+/**
+ * Transcribe one expression from a macro invocation
+ *
+ * @param parser Parser to extract statements from
+ */
+static std::vector<AST::SingleASTNode>
+transcribe_expression (Parser<MacroInvocLexer> &parser)
+{
+  auto expr = parser.parse_expr ();
+
+  return {AST::SingleASTNode (std::move (expr))};
+}
+
 AST::ASTFragment
 MacroExpander::transcribe_rule (
   AST::MacroRule &match_rule, AST::DelimTokenTree &invoc_token_tree,
@@ -3775,20 +3941,24 @@ MacroExpander::transcribe_rule (
   std::vector<std::unique_ptr<AST::Token>> substituted_tokens
     = substitute_context.substitute_tokens ();
 
-  // // handy for debugging
+  // parse it to an ASTFragment
+  MacroInvocLexer lex (std::move (substituted_tokens));
+  Parser<MacroInvocLexer> parser (std::move (lex));
+
+  // handy for debugging
   // for (auto &tok : substituted_tokens)
   //   {
   //     rust_debug ("tok: [%s]", tok->as_string ().c_str ());
   //   }
 
-  // parse it to an ASTFragment
-  MacroInvocLexer lex (std::move (substituted_tokens));
-  Parser<MacroInvocLexer> parser (std::move (lex));
+  auto last_token_id = TokenId::RIGHT_CURLY;
+  std::vector<AST::SingleASTNode> nodes;
 
   // this is used so we can check that we delimit the stream correctly.
   switch (transcribe_tree.get_delim_type ())
     {
     case AST::DelimType::PARENS:
+      last_token_id = TokenId::RIGHT_PAREN;
       rust_assert (parser.skip_token (LEFT_PAREN));
       break;
 
@@ -3797,6 +3967,7 @@ MacroExpander::transcribe_rule (
       break;
 
     case AST::DelimType::SQUARE:
+      last_token_id = TokenId::RIGHT_SQUARE;
       rust_assert (parser.skip_token (LEFT_SQUARE));
       break;
     }
@@ -3811,79 +3982,44 @@ MacroExpander::transcribe_rule (
   //   as a statement (either via ExpressionStatement or
   //   MacroInvocationWithSemi)
 
-  // parse the item
-  std::vector<AST::SingleASTNode> nodes;
-  switch (invoc_token_tree.get_delim_type ())
-    {
-    case AST::DelimType::PARENS:
-      case AST::DelimType::SQUARE: {
-	switch (ctx)
-	  {
-	    case ContextType::ITEM: {
-	      auto item = parser.parse_item (true);
-	      if (item != nullptr && !parser.has_errors ())
-		{
-		  rust_debug ("HELLO WORLD: [%s]", item->as_string ().c_str ());
-		  nodes.push_back (std::move (item));
-		}
-	    }
-	    break;
-
-	    case ContextType::BLOCK: {
-	      auto expr = parser.parse_expr ();
-	      if (expr != nullptr && !parser.has_errors ())
-		nodes.push_back (std::move (expr));
-	    }
-	    break;
-	  }
+  // The flow-chart in order to choose a parsing function is as follows:
+  //
+  // [is in item context?]
+  //     -- Yes --> parser.parse_item();
+  //     -- No --> [has semicolon?]
+  //                 -- Yes --> parser.parse_stmt();
+  //                 -- No --> [switch invocation.delimiter()]
+  //                             -- { } --> parser.parse_stmt();
+  //                             -- _ --> parser.parse_expr();
+
+  // If there is a semicolon OR we are expanding a MacroInvocationSemi, then
+  // we can parse multiple items. Otherwise, parse *one* expression
+
+  if (ctx == ContextType::ITEM)
+    nodes = transcribe_many_items (parser, last_token_id);
+  else if (semicolon)
+    nodes = transcribe_many_stmts (parser, last_token_id);
+  else
+    switch (invoc_token_tree.get_delim_type ())
+      {
+      case AST::CURLY:
+	nodes = transcribe_many_stmts (parser, last_token_id);
+	break;
+      default:
+	nodes = transcribe_expression (parser);
+	break;
       }
-      break;
-
-      case AST::DelimType::CURLY: {
-	switch (ctx)
-	  {
-	    case ContextType::ITEM: {
-	      auto item = parser.parse_item (true);
-	      if (item != nullptr && !parser.has_errors ())
-		nodes.push_back (std::move (item));
-	    }
-	    break;
-
-	    case ContextType::BLOCK: {
-	      auto stmt = parser.parse_stmt ();
-	      if (stmt != nullptr && !parser.has_errors ())
-		nodes.push_back (std::move (stmt));
-	    }
-	    break;
-	  }
-      }
-      break;
-    }
 
   // emit any errors
   if (parser.has_errors ())
     {
       for (auto &err : parser.get_errors ())
-	{
-	  rust_error_at (err.locus, "%s", err.message.c_str ());
-	}
-      return AST::ASTFragment::create_empty ();
+	rust_error_at (err.locus, "%s", err.message.c_str ());
+      return AST::ASTFragment::create_error ();
     }
 
   // are all the tokens used?
-  bool did_delimit = false;
-  switch (transcribe_tree.get_delim_type ())
-    {
-    case AST::DelimType::PARENS:
-      did_delimit = parser.skip_token (RIGHT_PAREN);
-      break;
-    case AST::DelimType::SQUARE:
-      did_delimit = parser.skip_token (RIGHT_SQUARE);
-      break;
-    case AST::DelimType::CURLY:
-      did_delimit = parser.skip_token (RIGHT_CURLY);
-      break;
-    }
+  bool did_delimit = parser.skip_token (last_token_id);
 
   bool reached_end_of_stream = did_delimit && parser.skip_token (END_OF_FILE);
   if (!reached_end_of_stream)
diff --git a/gcc/rust/expand/rust-macro-expand.h b/gcc/rust/expand/rust-macro-expand.h
index 88d0e6e..3433e64 100644
--- a/gcc/rust/expand/rust-macro-expand.h
+++ b/gcc/rust/expand/rust-macro-expand.h
@@ -137,7 +137,9 @@ struct MacroExpander
 
   MacroExpander (AST::Crate &crate, ExpansionCfg cfg, Session &session)
     : cfg (cfg), crate (crate), session (session),
-      sub_stack (SubstitutionScope ()), resolver (Resolver::Resolver::get ()),
+      sub_stack (SubstitutionScope ()),
+      expanded_fragment (AST::ASTFragment::create_empty ()),
+      resolver (Resolver::Resolver::get ()),
       mappings (Analysis::Mappings::get ())
   {}
 
@@ -223,11 +225,25 @@ struct MacroExpander
 
   ContextType peek_context () { return context.back (); }
 
+  void set_expanded_fragment (AST::ASTFragment &&fragment)
+  {
+    expanded_fragment = std::move (fragment);
+  }
+
+  AST::ASTFragment take_expanded_fragment ()
+  {
+    AST::ASTFragment old_fragment = std::move (expanded_fragment);
+    expanded_fragment = AST::ASTFragment::create_empty ();
+
+    return old_fragment;
+  }
+
 private:
   AST::Crate &crate;
   Session &session;
   SubstitutionScope sub_stack;
   std::vector<ContextType> context;
+  AST::ASTFragment expanded_fragment;
 
 public:
   Resolver::Resolver *resolver;
diff --git a/gcc/rust/hir/rust-ast-lower-expr.h b/gcc/rust/hir/rust-ast-lower-expr.h
index df836fc..e316868 100644
--- a/gcc/rust/hir/rust-ast-lower-expr.h
+++ b/gcc/rust/hir/rust-ast-lower-expr.h
@@ -102,12 +102,10 @@ public:
 
   void visit (AST::MacroInvocation &expr) override
   {
-    AST::ASTFragment &fragment = expr.get_fragment ();
-
-    // FIXME
-    // this assertion might go away, maybe on failure's to expand a macro?
-    rust_assert (!fragment.get_nodes ().empty ());
-    fragment.get_nodes ().at (0).accept_vis (*this);
+    rust_fatal_error (
+      expr.get_locus (),
+      "macro expansion failed: No macro invocation should get lowered to HIR "
+      "as they should disappear during expansion");
   }
 
   void visit (AST::TupleIndexExpr &expr) override
diff --git a/gcc/rust/hir/rust-ast-lower-implitem.h b/gcc/rust/hir/rust-ast-lower-implitem.h
index 3b901f6..5d674ca 100644
--- a/gcc/rust/hir/rust-ast-lower-implitem.h
+++ b/gcc/rust/hir/rust-ast-lower-implitem.h
@@ -54,15 +54,10 @@ public:
 
   void visit (AST::MacroInvocation &invoc) override
   {
-    if (!invoc.has_semicolon ())
-      return;
-
-    AST::ASTFragment &fragment = invoc.get_fragment ();
-
-    // FIXME
-    // this assertion might go away, maybe on failure's to expand a macro?
-    rust_assert (!fragment.get_nodes ().empty ());
-    fragment.get_nodes ().at (0).accept_vis (*this);
+    rust_fatal_error (
+      invoc.get_locus (),
+      "macro expansion failed: No macro invocation should get lowered to HIR "
+      "as they should disappear during expansion");
   }
 
   void visit (AST::TypeAlias &alias) override
@@ -323,15 +318,10 @@ public:
 
   void visit (AST::MacroInvocation &invoc) override
   {
-    if (!invoc.has_semicolon ())
-      return;
-
-    AST::ASTFragment &fragment = invoc.get_fragment ();
-
-    // FIXME
-    // this assertion might go away, maybe on failure's to expand a macro?
-    rust_assert (!fragment.get_nodes ().empty ());
-    fragment.get_nodes ().at (0).accept_vis (*this);
+    rust_fatal_error (
+      invoc.get_locus (),
+      "macro expansion failed: No macro invocation should get lowered to HIR "
+      "as they should disappear during expansion");
   }
 
   void visit (AST::TraitItemFunc &func) override
diff --git a/gcc/rust/hir/rust-ast-lower-item.h b/gcc/rust/hir/rust-ast-lower-item.h
index 660a30c..6974ffc 100644
--- a/gcc/rust/hir/rust-ast-lower-item.h
+++ b/gcc/rust/hir/rust-ast-lower-item.h
@@ -53,15 +53,10 @@ public:
 
   void visit (AST::MacroInvocation &invoc) override
   {
-    if (!invoc.has_semicolon ())
-      return;
-
-    AST::ASTFragment &fragment = invoc.get_fragment ();
-
-    // FIXME
-    // this assertion might go away, maybe on failure's to expand a macro?
-    rust_assert (!fragment.get_nodes ().empty ());
-    fragment.get_nodes ().at (0).accept_vis (*this);
+    rust_fatal_error (
+      invoc.get_locus (),
+      "macro expansion failed: No macro invocation should get lowered to HIR "
+      "as they should disappear during expansion");
   }
 
   void visit (AST::Module &module) override
diff --git a/gcc/rust/hir/rust-ast-lower-stmt.h b/gcc/rust/hir/rust-ast-lower-stmt.h
index 484c638..d0a1d38 100644
--- a/gcc/rust/hir/rust-ast-lower-stmt.h
+++ b/gcc/rust/hir/rust-ast-lower-stmt.h
@@ -47,15 +47,10 @@ public:
 
   void visit (AST::MacroInvocation &invoc) override
   {
-    if (!invoc.has_semicolon ())
-      return;
-
-    AST::ASTFragment &fragment = invoc.get_fragment ();
-
-    // FIXME
-    // this assertion might go away, maybe on failure's to expand a macro?
-    rust_assert (!fragment.get_nodes ().empty ());
-    fragment.get_nodes ().at (0).accept_vis (*this);
+    rust_fatal_error (
+      invoc.get_locus (),
+      "macro expansion failed: No macro invocation should get lowered to HIR "
+      "as they should disappear during expansion");
   }
 
   void visit (AST::ExprStmtWithBlock &stmt) override
diff --git a/gcc/rust/resolve/rust-ast-resolve-expr.cc b/gcc/rust/resolve/rust-ast-resolve-expr.cc
index 19f8169..e551802 100644
--- a/gcc/rust/resolve/rust-ast-resolve-expr.cc
+++ b/gcc/rust/resolve/rust-ast-resolve-expr.cc
@@ -35,14 +35,6 @@ ResolveExpr::go (AST::Expr *expr, NodeId parent, const CanonicalPath &prefix,
 }
 
 void
-ResolveExpr::visit (AST::MacroInvocation &expr)
-{
-  AST::ASTFragment &fragment = expr.get_fragment ();
-  for (auto &node : fragment.get_nodes ())
-    node.accept_vis (*this);
-}
-
-void
 ResolveExpr::visit (AST::TupleIndexExpr &expr)
 {
   resolve_expr (expr.get_tuple_expr ().get (), expr.get_node_id ());
diff --git a/gcc/rust/resolve/rust-ast-resolve-expr.h b/gcc/rust/resolve/rust-ast-resolve-expr.h
index d8bd203..a049ba1 100644
--- a/gcc/rust/resolve/rust-ast-resolve-expr.h
+++ b/gcc/rust/resolve/rust-ast-resolve-expr.h
@@ -62,8 +62,6 @@ public:
   static void go (AST::Expr *expr, NodeId parent, const CanonicalPath &prefix,
 		  const CanonicalPath &canonical_prefix);
 
-  void visit (AST::MacroInvocation &expr) override;
-
   void visit (AST::TupleIndexExpr &expr) override;
 
   void visit (AST::TupleExpr &expr) override;
diff --git a/gcc/rust/resolve/rust-ast-resolve-implitem.h b/gcc/rust/resolve/rust-ast-resolve-implitem.h
index 7393393..f17b222 100644
--- a/gcc/rust/resolve/rust-ast-resolve-implitem.h
+++ b/gcc/rust/resolve/rust-ast-resolve-implitem.h
@@ -49,16 +49,6 @@ public:
     item->accept_vis (resolver);
   }
 
-  void visit (AST::MacroInvocation &invoc) override
-  {
-    if (!invoc.has_semicolon ())
-      return;
-
-    AST::ASTFragment &fragment = invoc.get_fragment ();
-    for (auto &node : fragment.get_nodes ())
-      node.accept_vis (*this);
-  }
-
   void visit (AST::TypeAlias &type) override
   {
     auto decl
@@ -147,16 +137,6 @@ public:
     item->accept_vis (resolver);
   };
 
-  void visit (AST::MacroInvocation &invoc) override
-  {
-    if (!invoc.has_semicolon ())
-      return;
-
-    AST::ASTFragment &fragment = invoc.get_fragment ();
-    for (auto &node : fragment.get_nodes ())
-      node.accept_vis (*this);
-  }
-
   void visit (AST::TraitItemFunc &function) override
   {
     auto decl = ResolveTraitItemFunctionToCanonicalPath::resolve (function);
@@ -260,16 +240,6 @@ public:
     item->accept_vis (resolver);
   };
 
-  void visit (AST::MacroInvocation &invoc) override
-  {
-    if (!invoc.has_semicolon ())
-      return;
-
-    AST::ASTFragment &fragment = invoc.get_fragment ();
-    for (auto &node : fragment.get_nodes ())
-      node.accept_vis (*this);
-  }
-
   void visit (AST::ExternalFunctionItem &function) override
   {
     auto decl = CanonicalPath::new_seg (function.get_node_id (),
diff --git a/gcc/rust/resolve/rust-ast-resolve-item.h b/gcc/rust/resolve/rust-ast-resolve-item.h
index 2cb0006..5d32c00 100644
--- a/gcc/rust/resolve/rust-ast-resolve-item.h
+++ b/gcc/rust/resolve/rust-ast-resolve-item.h
@@ -45,16 +45,6 @@ public:
     item->accept_vis (resolver);
   };
 
-  void visit (AST::MacroInvocation &invoc) override
-  {
-    if (!invoc.has_semicolon ())
-      return;
-
-    AST::ASTFragment &fragment = invoc.get_fragment ();
-    for (auto &node : fragment.get_nodes ())
-      node.accept_vis (*this);
-  }
-
   void visit (AST::TraitItemType &type) override
   {
     auto decl = ResolveTraitItemTypeToCanonicalPath::resolve (type);
@@ -237,16 +227,6 @@ public:
     item->accept_vis (resolver);
   };
 
-  void visit (AST::MacroInvocation &invoc) override
-  {
-    if (!invoc.has_semicolon ())
-      return;
-
-    AST::ASTFragment &fragment = invoc.get_fragment ();
-    for (auto &node : fragment.get_nodes ())
-      node.accept_vis (*this);
-  }
-
   void visit (AST::TypeAlias &alias) override
   {
     auto talias = CanonicalPath::new_seg (alias.get_node_id (),
diff --git a/gcc/rust/resolve/rust-ast-resolve-stmt.h b/gcc/rust/resolve/rust-ast-resolve-stmt.h
index 785f3de..3afed53 100644
--- a/gcc/rust/resolve/rust-ast-resolve-stmt.h
+++ b/gcc/rust/resolve/rust-ast-resolve-stmt.h
@@ -44,17 +44,6 @@ public:
     stmt->accept_vis (resolver);
   };
 
-  void visit (AST::MacroInvocation &invoc) override
-  {
-    if (!invoc.has_semicolon ())
-      return;
-
-    AST::ASTFragment &fragment = invoc.get_fragment ();
-
-    for (auto &node : fragment.get_nodes ())
-      node.accept_vis (*this);
-  }
-
   void visit (AST::ExprStmtWithBlock &stmt) override
   {
     ResolveExpr::go (stmt.get_expr ().get (), stmt.get_node_id (), prefix,
diff --git a/gcc/rust/resolve/rust-ast-resolve-toplevel.h b/gcc/rust/resolve/rust-ast-resolve-toplevel.h
index 1f528fe..7aba67f 100644
--- a/gcc/rust/resolve/rust-ast-resolve-toplevel.h
+++ b/gcc/rust/resolve/rust-ast-resolve-toplevel.h
@@ -43,13 +43,6 @@ public:
     item->accept_vis (resolver);
   };
 
-  void visit (AST::MacroInvocation &invoc) override
-  {
-    AST::ASTFragment &fragment = invoc.get_fragment ();
-    for (auto &node : fragment.get_nodes ())
-      node.accept_vis (*this);
-  }
-
   void visit (AST::Module &module) override
   {
     auto mod
diff --git a/gcc/testsuite/rust/compile/macro11.rs b/gcc/testsuite/rust/compile/macro11.rs
new file mode 100644
index 0000000..97b89a1
--- /dev/null
+++ b/gcc/testsuite/rust/compile/macro11.rs
@@ -0,0 +1,11 @@
+macro_rules! call_f {
+    ($($f:ident)*) => { $($f();)* }
+}
+
+fn f() {}
+
+// This is valid and should parse items
+fn main() {
+    call_f!(f f f f);
+}
+
diff --git a/gcc/testsuite/rust/compile/macro12.rs b/gcc/testsuite/rust/compile/macro12.rs
new file mode 100644
index 0000000..b75fbad
--- /dev/null
+++ b/gcc/testsuite/rust/compile/macro12.rs
@@ -0,0 +1,8 @@
+// { dg-additional-options "-w" }
+macro_rules! define_vars {
+    ($($v:ident)*) => { $(let $v = 15;)* }
+}
+
+fn main() {
+    define_vars!(a0 b f __some_identifier);
+}
diff --git a/gcc/testsuite/rust/compile/macro13.rs b/gcc/testsuite/rust/compile/macro13.rs
new file mode 100644
index 0000000..eb8dfbb
--- /dev/null
+++ b/gcc/testsuite/rust/compile/macro13.rs
@@ -0,0 +1,12 @@
+// { dg-additional-options "-w" }
+macro_rules! create_type {
+    ($s:ident) => {
+        struct $s;
+    };
+}
+
+fn main() {
+    create_type!(A);
+
+    let a = A;
+}
diff --git a/gcc/testsuite/rust/compile/macro14.rs b/gcc/testsuite/rust/compile/macro14.rs
new file mode 100644
index 0000000..b18c56e
--- /dev/null
+++ b/gcc/testsuite/rust/compile/macro14.rs
@@ -0,0 +1,10 @@
+// { dg-additional-options "-w" }
+macro_rules! define_vars {
+    ($($v:ident)*) => { $(let $v = 15;)* }
+}
+
+fn main() -> i32 {
+    define_vars!(a0 b f __some_identifier);
+
+    b
+}
diff --git a/gcc/testsuite/rust/compile/macro15.rs b/gcc/testsuite/rust/compile/macro15.rs
new file mode 100644
index 0000000..02c739e
--- /dev/null
+++ b/gcc/testsuite/rust/compile/macro15.rs
@@ -0,0 +1,12 @@
+// { dg-additional-options "-w" }
+macro_rules! create_type {
+    ($s:ident) => {
+        struct $s;
+    };
+}
+
+create_type!(SomeOuterType);
+
+fn main() {
+    let a = SomeOuterType;
+}
diff --git a/gcc/testsuite/rust/compile/macro6.rs b/gcc/testsuite/rust/compile/macro6.rs
index e59155c..0ca35ba 100644
--- a/gcc/testsuite/rust/compile/macro6.rs
+++ b/gcc/testsuite/rust/compile/macro6.rs
@@ -1,6 +1,6 @@
 macro_rules! zero_or_one {
     ($($a:literal)?) => { // { dg-error "invalid amount of matches for macro invocation. Expected between 0 and 1, got 2" }
-        f()
+        f();
     }
 }
 
diff --git a/gcc/testsuite/rust/compile/macro7.rs b/gcc/testsuite/rust/compile/macro7.rs
index b57c5cb..abc4805 100644
--- a/gcc/testsuite/rust/compile/macro7.rs
+++ b/gcc/testsuite/rust/compile/macro7.rs
@@ -2,8 +2,8 @@ fn f() {}
 
 macro_rules! one_or_more {
     ($($a:literal)+) => { // { dg-error "invalid amount of matches for macro invocation" }
-        f()
-    }
+        f();
+    };
 }
 
 fn main() {
diff --git a/gcc/testsuite/rust/compile/macro8.rs b/gcc/testsuite/rust/compile/macro8.rs
index 756d5b0..d3e8af9 100644
--- a/gcc/testsuite/rust/compile/macro8.rs
+++ b/gcc/testsuite/rust/compile/macro8.rs
@@ -2,8 +2,8 @@ fn f() {}
 
 macro_rules! expr {
     ($($a:expr)?) => {
-        f()
-    }
+        f();
+    };
 }
 
 fn main() {
diff --git a/gcc/testsuite/rust/execute/torture/macros11.rs b/gcc/testsuite/rust/execute/torture/macros11.rs
index 7ce7d80..5bde97d 100644
--- a/gcc/testsuite/rust/execute/torture/macros11.rs
+++ b/gcc/testsuite/rust/execute/torture/macros11.rs
@@ -7,7 +7,9 @@ fn print_int(value: i32) {
     let s = "%d\n\0";
     let s_p = s as *const str;
     let c_p = s_p as *const i8;
-    unsafe { printf(c_p, value); }
+    unsafe {
+        printf(c_p, value);
+    }
 }
 
 macro_rules! add_exprs {
@@ -16,7 +18,8 @@ macro_rules! add_exprs {
 
 fn main() -> i32 {
     // 2
-    print_int(add_exprs!(2));
+    let a = add_exprs!(2);
+    print_int(a);
 
     0
 }
diff --git a/gcc/testsuite/rust/execute/torture/macros17.rs b/gcc/testsuite/rust/execute/torture/macros17.rs
index e007bb3..390352e 100644
--- a/gcc/testsuite/rust/execute/torture/macros17.rs
+++ b/gcc/testsuite/rust/execute/torture/macros17.rs
@@ -5,9 +5,9 @@ macro_rules! two {
 }
 
 macro_rules! one {
-    (1) => {
+    (1) => {{
         two!(2)
-    };
+    }};
 }
 
 fn main() -> i32 {
diff --git a/gcc/testsuite/rust/execute/torture/macros2.rs b/gcc/testsuite/rust/execute/torture/macros2.rs
index 0116bd1..ba50987 100644
--- a/gcc/testsuite/rust/execute/torture/macros2.rs
+++ b/gcc/testsuite/rust/execute/torture/macros2.rs
@@ -15,19 +15,19 @@ fn f() {
 
 macro_rules! kw0 {
     (keyword) => {
-        f()
+        f();
     };
 }
 
 macro_rules! kw1 {
     (fn) => {
-        f()
+        f();
     };
 }
 
 macro_rules! kw2 {
     (kw0 kw1 kw3) => {
-        f()
+        f();
     };
 }
 
diff --git a/gcc/testsuite/rust/execute/torture/macros22.rs b/gcc/testsuite/rust/execute/torture/macros22.rs
new file mode 100644
index 0000000..973af23
--- /dev/null
+++ b/gcc/testsuite/rust/execute/torture/macros22.rs
@@ -0,0 +1,23 @@
+// { dg-output "1\n2\nNaN\n3\n" }
+
+macro_rules! print_num {
+    ($l:literal) => {
+        printf("%d\n\0" as *const str as *const i8, $l);
+    };
+}
+
+extern "C" {
+    fn printf(s: *const i8, ...);
+}
+
+// Check to make sure that expanding macros does not break the flow of calls
+fn main() -> i32 {
+    print_num!(1);
+    print_num!(2);
+
+    printf("NaN\n\0" as *const str as *const i8);
+
+    print_num!(3);
+
+    0
+}
diff --git a/gcc/testsuite/rust/execute/torture/macros3.rs b/gcc/testsuite/rust/execute/torture/macros3.rs
index c1f5a53..00f6d25 100644
--- a/gcc/testsuite/rust/execute/torture/macros3.rs
+++ b/gcc/testsuite/rust/execute/torture/macros3.rs
@@ -15,7 +15,7 @@ fn f() {
 
 macro_rules! invocation0 {
     (valid) => {
-        f()
+        f();
     };
     () => {};
 }
@@ -23,27 +23,27 @@ macro_rules! invocation0 {
 macro_rules! invocation1 {
     (valid) => {};
     () => {
-        f()
+        f();
     };
 }
 
 macro_rules! invocation2 {
     (valid) => {
-        f()
+        f();
     };
     (invalid) => {};
 }
 
 macro_rules! invocation3 {
     (this is a valid invocation) => {
-        f()
+        f();
     };
     (not this one) => {};
 }
 
 macro_rules! invocation4 {
     (fn f() {}) => {
-        f()
+        f();
     };
     (not a keyword) => {};
 }
diff --git a/gcc/testsuite/rust/execute/torture/macros7.rs b/gcc/testsuite/rust/execute/torture/macros7.rs
index c1e13e3..ed1f922 100644
--- a/gcc/testsuite/rust/execute/torture/macros7.rs
+++ b/gcc/testsuite/rust/execute/torture/macros7.rs
@@ -8,13 +8,15 @@ fn f() {
     let s_p = r_s as *const str;
     let c_p = s_p as *const i8;
 
-    unsafe { printf(c_p); }
+    unsafe {
+        printf(c_p);
+    }
 }
 
 macro_rules! any {
     ($($a:expr)*) => {
-        f()
-    }
+        f();
+    };
 }
 
 fn main() -> i32 {
diff --git a/gcc/testsuite/rust/execute/torture/macros8.rs b/gcc/testsuite/rust/execute/torture/macros8.rs
index 2f1e238..a12aca4 100644
--- a/gcc/testsuite/rust/execute/torture/macros8.rs
+++ b/gcc/testsuite/rust/execute/torture/macros8.rs
@@ -8,13 +8,15 @@ fn f() {
     let s_p = r_s as *const str;
     let c_p = s_p as *const i8;
 
-    unsafe { printf(c_p); }
+    unsafe {
+        printf(c_p);
+    }
 }
 
 macro_rules! zero_or_one {
     ($($a:expr)?) => {
-        f()
-    }
+        f();
+    };
 }
 
 fn main() -> i32 {
diff --git a/gcc/testsuite/rust/execute/torture/macros9.rs b/gcc/testsuite/rust/execute/torture/macros9.rs
index 22dec2a..0e3fd24 100644
--- a/gcc/testsuite/rust/execute/torture/macros9.rs
+++ b/gcc/testsuite/rust/execute/torture/macros9.rs
@@ -8,13 +8,15 @@ fn f() {
     let s_p = r_s as *const str;
     let c_p = s_p as *const i8;
 
-    unsafe { printf(c_p); }
+    unsafe {
+        printf(c_p);
+    }
 }
 
 macro_rules! one_or_more {
     ($($a:expr)+) => {
-        f()
-    }
+        f();
+    };
 }
 
 fn main() -> i32 {