[parser] Refactor of Parse*Statement*, part 4

src/parsing/preparser.h

Index: src/parsing/preparser.h
diff --git a/src/parsing/preparser.h b/src/parsing/preparser.h
index 969e3fbcb271365c906d6df7015fcbbf0209e990..f735f5cbe72e247799ae4a27e31c3ca26c07977e 100644
--- a/src/parsing/preparser.h
+++ b/src/parsing/preparser.h
@@ -25,6 +25,9 @@ class PreParserIdentifier {
   static PreParserIdentifier Default() {
     return PreParserIdentifier(kUnknownIdentifier);
   }
+  static PreParserIdentifier Empty() {
+    return PreParserIdentifier(kEmptyIdentifier);
+  }
   static PreParserIdentifier Eval() {
     return PreParserIdentifier(kEvalIdentifier);
   }
@@ -64,6 +67,7 @@ class PreParserIdentifier {
   static PreParserIdentifier Async() {
     return PreParserIdentifier(kAsyncIdentifier);
   }
+  bool IsEmpty() const { return type_ == kEmptyIdentifier; }
   bool IsEval() const { return type_ == kEvalIdentifier; }
   bool IsArguments() const { return type_ == kArgumentsIdentifier; }
   bool IsEvalOrArguments() const { return IsEval() || IsArguments(); }
@@ -91,6 +95,7 @@ class PreParserIdentifier {
 
  private:
   enum Type {
+    kEmptyIdentifier,
     kUnknownIdentifier,
     kFutureReservedIdentifier,
     kFutureStrictReservedIdentifier,
@@ -389,6 +394,14 @@ class PreParserStatement {
     return PreParserStatement(kUnknownStatement);
   }
 
+  static PreParserStatement Null() {
+    return PreParserStatement(kNullStatement);
+  }
+
+  static PreParserStatement Empty() {
+    return PreParserStatement(kEmptyStatement);
+  }
+
   static PreParserStatement Jump() {
     return PreParserStatement(kJumpStatement);
   }
@@ -433,6 +446,10 @@ class PreParserStatement {
     return code_ == kJumpStatement;
   }
 
+  bool IsNullStatement() { return code_ == kNullStatement; }
+
+  bool IsEmptyStatement() { return code_ == kEmptyStatement; }

[adamk, 2016/09/08 18:33:19]

Is this only called by IsNullOrEmptyStatement()? The two callers of that both
look suspicious to me, when could they return NULL? Having both Null and Empty
in the preparser seems possibly confusing so if we could get away without it I'd
be happier.

[nickie, 2016/09/09 09:42:27]

I think the confusion was originally created by "EmptyExpression", which the
parser implements as a nullptr.  This should rather be a "NullExpression" as
there's no such thing as an empty expression.

On the other hand, there's need for a "NullStatement" (which the parser
implements as a nullptr); this is returned, e.g., for *ok == false, or in other
exceptional cases.  And there's "EmptyStatement", which is a valid empty
statement (just the semicolon).

IsNullOrEmptyStatement() is used when checking the directive prologue.  I
thought it was not necessary there, but if you remove it some tests fail.  It
could be replaced there by "IsNullStatement() || IsEmptyStatement()", if you
prefer.

+
   // Dummy implementation for making statement->somefunc() work in both Parser
   // and PreParser.
   PreParserStatement* operator->() { return this; }
@@ -442,6 +459,8 @@ class PreParserStatement {
 
  private:
   enum Type {
+    kNullStatement,
+    kEmptyStatement,
     kUnknownStatement,
     kJumpStatement,
     kStringLiteralExpressionStatement,
@@ -573,7 +592,7 @@ class PreParserFactory {
   }
   PreParserStatement NewReturnStatement(PreParserExpression expression,
                                         int pos) {
-    return PreParserStatement::Default();
+    return PreParserStatement::Jump();
   }
   PreParserExpression NewFunctionLiteral(
       PreParserIdentifier name, Scope* scope, PreParserStatementList body,
@@ -609,6 +628,32 @@ class PreParserFactory {
     return PreParserStatement::Default();
   }
 
+  PreParserStatement NewExpressionStatement(PreParserExpression expr, int pos) {
+    return PreParserStatement::ExpressionStatement(expr);
+  }
+
+  PreParserStatement NewIfStatement(PreParserExpression condition,
+                                    PreParserStatement then_statement,
+                                    PreParserStatement else_statement,
+                                    int pos) {
+    // This must return a jump statement iff both clauses are jump statements.
+    return else_statement.IsJumpStatement() ? then_statement : else_statement;
+  }
+
+  PreParserStatement NewBreakStatement(PreParserStatement target, int pos) {
+    return PreParserStatement::Jump();
+  }
+
+  PreParserStatement NewContinueStatement(PreParserStatement target, int pos) {
+    return PreParserStatement::Jump();
+  }
+
+  PreParserStatement NewWithStatement(Scope* scope,
+                                      PreParserExpression expression,
+                                      PreParserStatement statement, int pos) {
+    return PreParserStatement::Default();
+  }
+
   // Return the object itself as AstVisitor and implement the needed
   // dummy method right in this class.
   PreParserFactory* visitor() { return this; }
@@ -662,6 +707,8 @@ struct ParserTypes<PreParser> {
   typedef PreParserStatement Statement;
   typedef PreParserStatementList StatementList;
   typedef PreParserStatement Block;
+  typedef PreParserStatement BreakableStatementT;
+  typedef PreParserStatement IterationStatementT;
 
   // For constructing objects returned by the traversing functions.
   typedef PreParserFactory Factory;
@@ -775,15 +822,6 @@ class PreParser : public ParserBase<PreParser> {
   Expression ParseAsyncFunctionExpression(bool* ok);
   Statement ParseClassDeclaration(ZoneList<const AstRawString*>* names,
                                   bool default_export, bool* ok);
-  Statement ParseExpressionOrLabelledStatement(
-      ZoneList<const AstRawString*>* names,
-      AllowLabelledFunctionStatement allow_function, bool* ok);
-  Statement ParseIfStatement(ZoneList<const AstRawString*>* labels, bool* ok);
-  Statement ParseContinueStatement(bool* ok);
-  Statement ParseBreakStatement(ZoneList<const AstRawString*>* labels,
-                                bool* ok);
-  Statement ParseReturnStatement(bool* ok);
-  Statement ParseWithStatement(ZoneList<const AstRawString*>* labels, bool* ok);
   Statement ParseSwitchStatement(ZoneList<const AstRawString*>* labels,
                                  bool* ok);
   Statement ParseDoWhileStatement(ZoneList<const AstRawString*>* labels,
@@ -884,6 +922,42 @@ class PreParser : public ParserBase<PreParser> {
       const DeclarationDescriptor* declaration_descriptor,
       const DeclarationParsingResult::Declaration* declaration,
       ZoneList<const AstRawString*>* names, bool* ok) {}
+  V8_INLINE ZoneList<const AstRawString*>* DeclareLabel(
+      ZoneList<const AstRawString*>* labels, PreParserExpression expr,
+      bool* ok) {
+    DCHECK(!expr.AsIdentifier().IsEnum());
+    DCHECK(!parsing_module_ || !expr.AsIdentifier().IsAwait());
+    DCHECK(is_sloppy(language_mode()) ||
+           !IsFutureStrictReserved(expr.AsIdentifier()));
+    return labels;
+  }
+
+  V8_INLINE PreParserStatement ParseNativeDeclaration(bool* ok) {
+    UNREACHABLE();
+    return PreParserStatement::Default();
+  }
+
+  // TODO(nikolaos): The preparser currently does not keep track of labels.
+  V8_INLINE bool ContainsLabel(ZoneList<const AstRawString*>* labels,
+                               PreParserIdentifier label) {
+    return false;
+  }
+
+  V8_INLINE PreParserExpression RewriteReturn(PreParserExpression return_value,
+                                              int pos) {
+    return return_value;
+  }
+
+  // TODO(nikolaos): The preparser currently does not keep track of labels
+  // and targets.
+  V8_INLINE PreParserStatement LookupBreakTarget(PreParserIdentifier label,
+                                                 bool* ok) {
+    return PreParserStatement::Default();
+  }
+  V8_INLINE PreParserStatement LookupContinueTarget(PreParserIdentifier label,
+                                                    bool* ok) {
+    return PreParserStatement::Default();
+  }
 
   V8_INLINE void QueueDestructuringAssignmentForRewriting(
       PreParserExpression assignment) {}
@@ -946,6 +1020,14 @@ class PreParser : public ParserBase<PreParser> {
     return false;
   }
 
+  V8_INLINE bool IsNative(PreParserExpression expr) const {
+    // Preparsing is disabled for extensions (because the extension
+    // details aren't passed to lazily compiled functions), so we
+    // don't accept "native function" in the preparser and there is
+    // no need to keep track of "native".
+    return false;
+  }
+
   V8_INLINE static bool IsArrayIndex(PreParserIdentifier string,
                                      uint32_t* index) {
     return false;
@@ -1028,14 +1110,17 @@ class PreParser : public ParserBase<PreParser> {
 
   V8_INLINE void ReportMessageAt(Scanner::Location source_location,
                                  MessageTemplate::Template message,
-                                 const AstRawString* arg,
+                                 PreParserIdentifier arg,
                                  ParseErrorType error_type = kSyntaxError) {
     UNREACHABLE();
   }
 
   // "null" return type creators.
   V8_INLINE static PreParserIdentifier EmptyIdentifier() {
-    return PreParserIdentifier::Default();
+    return PreParserIdentifier::Empty();
+  }
+  V8_INLINE static bool IsEmptyIdentifier(PreParserIdentifier name) {
+    return name.IsEmpty();
   }
   V8_INLINE static PreParserExpression EmptyExpression() {
     return PreParserExpression::Empty();
@@ -1077,9 +1162,12 @@ class PreParser : public ParserBase<PreParser> {
     return PreParserStatement::Default();
   }
 
+  V8_INLINE bool IsNullStatement(PreParserStatement stmt) {
+    return stmt.IsNullStatement();
+  }
+
   V8_INLINE bool IsNullOrEmptyStatement(PreParserStatement stmt) {
-    // TODO(nikolaos): See if this needs to be consistent for the preparser.
-    return false;
+    return stmt.IsNullStatement() || stmt.IsEmptyStatement();
   }
 
   V8_INLINE static PreParserStatement NullBlock() {