Merge #1027 #1032

1027: parser: Allow parsing stmts without closing semicolon r=CohenArthur a=CohenArthur

In certain cases such as macro matching or macro expansion, it is
important to allow the parser to return a valid statement even if no
closing semicolon is given. This commit adds an optional parameter to
the concerned functions to allow a lack of semicolon those special cases

Closes #1011 
Closes #1010 

1032: Add AST kind information r=CohenArthur a=CohenArthur

Closes #1001 

This PR adds a base for adding node information to our AST types. It can be used when requiring to differentiate between multiple kinds of nodes, while not necessarily wanting to do a full static cast. This will open up a lot of cleanup issues and good first issues for Project Pineapple

Co-authored-by: Arthur Cohen <arthur.cohen@embecosm.com>

gcc/rust/ast/rust-ast.h

@@ -36,6 +36,28 @@ namespace AST {
 class ASTVisitor;
 using AttrVec = std::vector<Attribute>;
 
+// The available kinds of AST Nodes
+enum Kind
+{
+  UNKNOWN,
+  MACRO_RULES_DEFINITION,
+  MACRO_INVOCATION,
+};
+
+// Abstract base class for all AST elements
+class Node
+{
+public:
+  /**
+   * Get the kind of Node this is. This is used to differentiate various AST
+   * elements with very little overhead when extracting the derived type through
+   * static casting is not necessary.
+   */
+  // FIXME: Mark this as `= 0` in the future to make sure every node implements
+  // it
+  virtual Kind get_ast_kind () const { return Kind::UNKNOWN; }
+};
+
 // Delimiter types - used in macros and whatever.
 enum DelimType
 {
@@ -814,7 +836,7 @@ class MetaListNameValueStr;
 
 /* Base statement abstract class. Note that most "statements" are not allowed in
  * top-level module scope - only a subclass of statements called "items" are. */
-class Stmt
+class Stmt : public Node
 {
 public:
   // Unique pointer custom clone function
@@ -880,7 +902,7 @@ protected:
 class ExprWithoutBlock;
 
 // Base expression AST node - abstract
-class Expr
+class Expr : public Node
 {
 public:
   // Unique pointer custom clone function
@@ -1053,7 +1075,7 @@ protected:
 class TraitBound;
 
 // Base class for types as represented in AST - abstract
-class Type
+class Type : public Node
 {
 public:
   // Unique pointer custom clone function

gcc/rust/ast/rust-macro.h

@@ -441,6 +441,8 @@ public:
     is_builtin_rule = true;
   }
 
+  Kind get_ast_kind () const override { return Kind::MACRO_RULES_DEFINITION; }
+
 protected:
   /* Use covariance to implement clone function as returning this object rather
    * than base */
@@ -505,6 +507,8 @@ public:
     return ExprWithoutBlock::get_node_id ();
   }
 
+  Kind get_ast_kind () const override { return Kind::MACRO_INVOCATION; }
+
   NodeId get_macro_node_id () const { return node_id; }
 
   MacroInvocData &get_invoc_data () { return invoc_data; }

gcc/rust/expand/rust-macro-expand.cc

@@ -478,7 +478,7 @@ MacroExpander::match_fragment (Parser<MacroInvocLexer> &parser,
       break;
 
     case AST::MacroFragSpec::STMT:
-      parser.parse_stmt ();
+      parser.parse_stmt (/* allow_no_semi */ true);
       break;
 
     case AST::MacroFragSpec::LIFETIME:
@@ -505,6 +505,9 @@ MacroExpander::match_fragment (Parser<MacroInvocLexer> &parser,
       return false;
     }
 
+  for (const auto &error : parser.get_errors ())
+    error.emit_error ();
+
   // it matches if the parser did not produce errors trying to parse that type
   // of item
   return !parser.has_errors ();
@@ -824,7 +827,7 @@ transcribe_many_stmts (Parser<MacroInvocLexer> &parser, TokenId &delimiter)
   // 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 ();
+    auto stmt = parser.parse_stmt (/* allow_no_semi */ true);
     return AST::SingleASTNode (std::move (stmt));
   });
 }

gcc/rust/hir/rust-ast-lower-expr.h

@@ -100,14 +100,6 @@ public:
     return resolver.translated;
   }
 
-  void visit (AST::MacroInvocation &expr) override
-  {
-    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
   {
     HIR::Expr *tuple_expr

gcc/rust/hir/rust-ast-lower-implitem.h

@@ -52,14 +52,6 @@ public:
     return resolver.translated;
   }
 
-  void visit (AST::MacroInvocation &invoc) override
-  {
-    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
   {
     std::vector<std::unique_ptr<HIR::WhereClauseItem> > where_clause_items;
@@ -316,14 +308,6 @@ public:
     return resolver.translated;
   }
 
-  void visit (AST::MacroInvocation &invoc) override
-  {
-    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
   {
     AST::TraitFunctionDecl &ref = func.get_trait_function_decl ();

gcc/rust/hir/rust-ast-lower-item.h

@@ -51,14 +51,6 @@ public:
     return resolver.translated;
   }
 
-  void visit (AST::MacroInvocation &invoc) override
-  {
-    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
   {
     auto crate_num = mappings->get_current_crate ();

gcc/rust/hir/rust-ast-lower-stmt.h

@@ -45,14 +45,6 @@ public:
     return resolver.translated;
   }
 
-  void visit (AST::MacroInvocation &invoc) override
-  {
-    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
   {
     HIR::ExprWithBlock *expr

gcc/rust/hir/rust-ast-lower.cc

@@ -67,6 +67,16 @@ ASTLoweringBlock::visit (AST::BlockExpr &expr)
 
   for (auto &s : expr.get_statements ())
     {
+      if (s->get_ast_kind () == AST::Kind::MACRO_RULES_DEFINITION)
+	continue;
+
+      if (s->get_ast_kind () == AST::Kind::MACRO_INVOCATION)
+	rust_fatal_error (
+	  s->get_locus (),
+	  "macro invocations should not get lowered to HIR - At "
+	  "this point in "
+	  "the pipeline, they should all have been expanded");
+
       if (block_did_terminate)
 	rust_warning_at (s->get_locus (), 0, "unreachable statement");
 

gcc/rust/parse/rust-parse-impl.h

@@ -6034,9 +6034,10 @@ Parser<ManagedTokenSource>::parse_named_function_param (
 // Parses a statement (will further disambiguate any statement).
 template <typename ManagedTokenSource>
 std::unique_ptr<AST::Stmt>
-Parser<ManagedTokenSource>::parse_stmt ()
+Parser<ManagedTokenSource>::parse_stmt (bool allow_no_semi)
 {
   // quick exit for empty statement
+  // FIXME: Can we have empty statements without semicolons? Just nothing?
   const_TokenPtr t = lexer.peek_token ();
   if (t->get_id () == SEMICOLON)
     {
@@ -6058,7 +6059,7 @@ Parser<ManagedTokenSource>::parse_stmt ()
     {
     case LET:
       // let statement
-      return parse_let_stmt (std::move (outer_attrs));
+      return parse_let_stmt (std::move (outer_attrs), allow_no_semi);
     case PUB:
     case MOD:
     case EXTERN_TOK:
@@ -6113,7 +6114,7 @@ Parser<ManagedTokenSource>::parse_stmt ()
       // TODO: find out how to disable gcc "implicit fallthrough" warning
     default:
       // fallback: expression statement
-      return parse_expr_stmt (std::move (outer_attrs));
+      return parse_expr_stmt (std::move (outer_attrs), allow_no_semi);
       break;
     }
 }
@@ -6121,7 +6122,8 @@ Parser<ManagedTokenSource>::parse_stmt ()
 // Parses a let statement.
 template <typename ManagedTokenSource>
 std::unique_ptr<AST::LetStmt>
-Parser<ManagedTokenSource>::parse_let_stmt (AST::AttrVec outer_attrs)
+Parser<ManagedTokenSource>::parse_let_stmt (AST::AttrVec outer_attrs,
+					    bool allow_no_semi)
 {
   Location locus = lexer.peek_token ()->get_locus ();
   skip_token (LET);
@@ -6176,12 +6178,12 @@ Parser<ManagedTokenSource>::parse_let_stmt (AST::AttrVec outer_attrs)
 	}
     }
 
-  if (!skip_token (SEMICOLON))
+  if (!maybe_skip_token (SEMICOLON) && !allow_no_semi)
     {
       // skip after somewhere
       return nullptr;
-      /* TODO: how wise is it to ditch a mostly-valid let statement just because
-       * a semicolon is missing? */
+      /* TODO: how wise is it to ditch a mostly-valid let statement just
+       * because a semicolon is missing? */
     }
 
   return std::unique_ptr<AST::LetStmt> (
@@ -7016,7 +7018,8 @@ Parser<ManagedTokenSource>::parse_method ()
  * block statement). */
 template <typename ManagedTokenSource>
 std::unique_ptr<AST::ExprStmt>
-Parser<ManagedTokenSource>::parse_expr_stmt (AST::AttrVec outer_attrs)
+Parser<ManagedTokenSource>::parse_expr_stmt (AST::AttrVec outer_attrs,
+					     bool allow_no_semi)
 {
   /* potential thoughts - define new virtual method "has_block()" on expr. parse
    * expr and then determine whether semicolon is needed as a result of this
@@ -7055,7 +7058,8 @@ Parser<ManagedTokenSource>::parse_expr_stmt (AST::AttrVec outer_attrs)
 	  }
 	else
 	  {
-	    return parse_expr_stmt_without_block (std::move (outer_attrs));
+	    return parse_expr_stmt_without_block (std::move (outer_attrs),
+						  allow_no_semi);
 	  }
       }
       case UNSAFE: {
@@ -7068,7 +7072,8 @@ Parser<ManagedTokenSource>::parse_expr_stmt (AST::AttrVec outer_attrs)
 	  }
 	else
 	  {
-	    return parse_expr_stmt_without_block (std::move (outer_attrs));
+	    return parse_expr_stmt_without_block (std::move (outer_attrs),
+						  allow_no_semi);
 	  }
       }
     default:
@@ -7076,7 +7081,8 @@ Parser<ManagedTokenSource>::parse_expr_stmt (AST::AttrVec outer_attrs)
       /* TODO: if possible, be more selective about possible expr without block
        * initial tokens in order to prevent more syntactical errors at parse
        * time. */
-      return parse_expr_stmt_without_block (std::move (outer_attrs));
+      return parse_expr_stmt_without_block (std::move (outer_attrs),
+					    allow_no_semi);
     }
 }
 
@@ -7192,7 +7198,7 @@ Parser<ManagedTokenSource>::parse_expr_stmt_with_block (
 template <typename ManagedTokenSource>
 std::unique_ptr<AST::ExprStmtWithoutBlock>
 Parser<ManagedTokenSource>::parse_expr_stmt_without_block (
-  AST::AttrVec outer_attrs)
+  AST::AttrVec outer_attrs, bool allow_no_semi)
 {
   /* TODO: maybe move more logic for expr without block in here for better error
    * handling */
@@ -7217,7 +7223,7 @@ Parser<ManagedTokenSource>::parse_expr_stmt_without_block (
     }
 
   // skip semicolon at end that is required
-  if (!skip_token (SEMICOLON))
+  if (!maybe_skip_token (SEMICOLON) && !allow_no_semi)
     {
       // skip somewhere?
       return nullptr;
@@ -12219,6 +12225,18 @@ Parser<ManagedTokenSource>::skip_token (TokenId token_id)
   return expect_token (token_id) != const_TokenPtr ();
 }
 
+/* Checks if current token has inputted id - skips it and returns true if so,
+ * returns false otherwise without diagnosing an error */
+template <typename ManagedTokenSource>
+bool
+Parser<ManagedTokenSource>::maybe_skip_token (TokenId token_id)
+{
+  if (lexer.peek_token ()->get_id () != token_id)
+    return false;
+  else
+    return skip_token (token_id);
+}
+
 /* Checks the current token - if id is same as expected, skips and returns it,
  * otherwise diagnoses error and returns null. */
 template <typename ManagedTokenSource>

gcc/rust/parse/rust-parse.h

@@ -88,8 +88,25 @@ struct ParseRestrictions
 template <typename ManagedTokenSource> class Parser
 {
 public:
+  /**
+   * Consume a token, reporting an error if it isn't the next token
+   *
+   * @param t ID of the token to consume
+   *
+   * @return true if the token was next, false if it wasn't found
+   */
   bool skip_token (TokenId t);
 
+  /**
+   * Same as `skip_token` but allows for failure without necessarily reporting
+   * an error
+   *
+   * @param t ID of the token to consume
+   *
+   * @return true if the token was next, false if it wasn't found
+   */
+  bool maybe_skip_token (TokenId t);
+
   std::unique_ptr<AST::Expr>
   parse_expr (AST::AttrVec outer_attrs = AST::AttrVec (),
 	      ParseRestrictions restrictions = ParseRestrictions ());
@@ -103,7 +120,20 @@ public:
 
   std::unique_ptr<AST::Item> parse_item (bool called_from_statement);
   std::unique_ptr<AST::Pattern> parse_pattern ();
-  std::unique_ptr<AST::Stmt> parse_stmt ();
+
+  /**
+   * Parse a statement
+   *
+   * Statement : ';'
+   *    | Item
+   *    | LetStatement
+   *    | ExpressionStatement
+   *    | MacroInvocationSemi
+   *
+   * @param allow_no_semi Allow the parser to not parse a semicolon after
+   * 		the statement without erroring out
+   */
+  std::unique_ptr<AST::Stmt> parse_stmt (bool allow_no_semi = false);
   std::unique_ptr<AST::Type> parse_type ();
   AST::PathInExpression parse_path_in_expression ();
   std::vector<std::unique_ptr<AST::LifetimeParam> > parse_lifetime_params ();
@@ -575,12 +605,25 @@ private:
   AST::MaybeNamedParam parse_maybe_named_param (AST::AttrVec outer_attrs);
 
   // Statement-related
-  std::unique_ptr<AST::LetStmt> parse_let_stmt (AST::AttrVec outer_attrs);
-  std::unique_ptr<AST::ExprStmt> parse_expr_stmt (AST::AttrVec outer_attrs);
+
+  /**
+   *Parse a let-statement
+   * LetStatement :
+   * 	OuterAttribute*
+   * 		'let' PatternNoTopAlt ( ':' Type )? ('=' Expression )? ';'
+   *
+   * @param allow_no_semi Allow parsing a let-statement without expecting a
+   * 		semicolon to follow it
+   */
+  std::unique_ptr<AST::LetStmt> parse_let_stmt (AST::AttrVec outer_attrs,
+						bool allow_no_semi = false);
+  std::unique_ptr<AST::ExprStmt> parse_expr_stmt (AST::AttrVec outer_attrs,
+						  bool allow_no_semi = false);
   std::unique_ptr<AST::ExprStmtWithBlock>
   parse_expr_stmt_with_block (AST::AttrVec outer_attrs);
   std::unique_ptr<AST::ExprStmtWithoutBlock>
-  parse_expr_stmt_without_block (AST::AttrVec outer_attrs);
+  parse_expr_stmt_without_block (AST::AttrVec outer_attrs,
+				 bool allow_no_semi = false);
   ExprOrStmt parse_stmt_or_expr_without_block ();
   ExprOrStmt parse_stmt_or_expr_with_block (AST::AttrVec outer_attrs);
   ExprOrStmt parse_macro_invocation_maybe_semi (AST::AttrVec outer_attrs);

gcc/testsuite/rust/compile/macro16.rs

@@ -0,0 +1,11 @@
+fn main() {
+    macro_rules! create_type {
+        ($s:ident) => {
+            struct $s(i32);
+        };
+    }
+
+    create_type!(Wrapper);
+
+    let _ = Wrapper(15);
+}

gcc/testsuite/rust/compile/macro18.rs

@@ -0,0 +1,14 @@
+// { dg-additional-options "-w" }
+
+macro_rules! take_stmt {
+    ($s:stmt) => {
+        $s;
+    };
+}
+
+fn main() -> i32 {
+    take_stmt!(let complete = 15;);
+    take_stmt!(let lacking = 14);
+
+    0
+}

gcc/testsuite/rust/compile/macro19.rs

@@ -0,0 +1,19 @@
+// { dg-additional-options "-w" }
+
+macro_rules! call_without_semi {
+    () => {
+        f()
+    };
+    (block) => {{
+        f()
+    }};
+}
+
+fn f() {}
+
+fn main() -> i32 {
+    call_without_semi!();
+    call_without_semi!(block);
+
+    0
+}

gcc/testsuite/rust/compile/macro9.rs

@@ -12,6 +12,7 @@ fn main() -> i32 {
     let b = add!(15);
     let b = add!(15 14); // { dg-error "Failed to match any rule within macro" }
     let b = add!(15, 14,); // { dg-error "Failed to match any rule within macro" }
+                           // { dg-error "found unexpected token" "" { target *-*-* } .-1 }
 
     0
 }