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

src/parsing/parser.h

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef V8_PARSING_PARSER_H_
 #define V8_PARSING_PARSER_H_
 
 #include "src/ast/ast.h"
 #include "src/ast/scopes.h"
 #include "src/parsing/parser-base.h"
@@ skipping 139 matching lines @@
   typedef v8::internal::Expression* Expression;
   typedef v8::internal::FunctionLiteral* FunctionLiteral;
   typedef ObjectLiteral::Property* ObjectLiteralProperty;
   typedef ClassLiteral::Property* ClassLiteralProperty;
   typedef ZoneList<v8::internal::Expression*>* ExpressionList;
   typedef ZoneList<ObjectLiteral::Property*>* PropertyList;
   typedef ParserFormalParameters FormalParameters;
   typedef v8::internal::Statement* Statement;
   typedef ZoneList<v8::internal::Statement*>* StatementList;
   typedef v8::internal::Block* Block;
+  typedef v8::internal::BreakableStatement* BreakableStatementT;
+  typedef v8::internal::IterationStatement* IterationStatementT;
 
   // For constructing objects returned by the traversing functions.
   typedef AstNodeFactory Factory;
 
   typedef ParserTarget Target;
   typedef ParserTargetScope TargetScope;
 };
 
 class Parser : public ParserBase<Parser> {
  public:
@@ skipping 26 matching lines @@
 
   // Runtime encoding of different completion modes.
   enum CompletionKind {
     kNormalCompletion,
     kThrowCompletion,
     kAbruptCompletion
   };
 
   enum class FunctionBodyType { kNormal, kSingleExpression };
 
-  DeclarationScope* GetDeclarationScope() const {
-    return scope()->GetDeclarationScope();
-  }
-  DeclarationScope* GetClosureScope() const {
-    return scope()->GetClosureScope();
-  }
   Variable* NewTemporary(const AstRawString* name) {
     return scope()->NewTemporary(name);
   }
 
   // Limit the allowed number of local variables in a function. The hard limit
   // is that offsets computed by FullCodeGenerator::StackOperand and similar
   // functions are ints, and they should not overflow. In addition, accessing
   // local variables creates user-controlled constants in the generated code,
   // and we don't want too much user-controlled memory inside the code (this was
   // the reason why this limit was introduced in the first place; see
@@ skipping 56 matching lines @@
   Statement* ParseClassDeclaration(ZoneList<const AstRawString*>* names,
                                    bool default_export, bool* ok);
   Statement* ParseNativeDeclaration(bool* ok);
   Block* BuildInitializationBlock(DeclarationParsingResult* parsing_result,
                                   ZoneList<const AstRawString*>* names,
                                   bool* ok);
   void DeclareAndInitializeVariables(
       Block* block, const DeclarationDescriptor* declaration_descriptor,
       const DeclarationParsingResult::Declaration* declaration,
       ZoneList<const AstRawString*>* names, bool* ok);
+  ZoneList<const AstRawString*>* DeclareLabel(
+      ZoneList<const AstRawString*>* labels, Expression* expr, bool* ok);
+  bool ContainsLabel(ZoneList<const AstRawString*>* labels,
+                     const AstRawString* label);
+  Expression* RewriteReturn(Expression* return_value, int pos);
 
   DoExpression* ParseDoExpression(bool* ok);
   Expression* ParseYieldStarExpression(bool* ok);
 
   class PatternRewriter final : public AstVisitor<PatternRewriter> {
    public:
     static void DeclareAndInitializeVariables(
         Parser* parser, Block* block,
         const DeclarationDescriptor* declaration_descriptor,
         const DeclarationParsingResult::Declaration* declaration,
@@ skipping 65 matching lines @@
     Block* block_;
     const DeclarationDescriptor* descriptor_;
     ZoneList<const AstRawString*>* names_;
     Expression* current_value_;
     int recursion_level_;
     bool* ok_;
 
     DEFINE_AST_VISITOR_MEMBERS_WITHOUT_STACKOVERFLOW()
   };
 
-  Statement* ParseExpressionOrLabelledStatement(
-      ZoneList<const AstRawString*>* labels,
-      AllowLabelledFunctionStatement allow_function, bool* ok);
-  IfStatement* 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);
   CaseClause* ParseCaseClause(bool* default_seen_ptr, bool* ok);
   Statement* ParseSwitchStatement(ZoneList<const AstRawString*>* labels,
                                   bool* ok);
   DoWhileStatement* ParseDoWhileStatement(ZoneList<const AstRawString*>* labels,
                                           bool* ok);
   WhileStatement* ParseWhileStatement(ZoneList<const AstRawString*>* labels,
                                       bool* ok);
   Statement* ParseForStatement(ZoneList<const AstRawString*>* labels, bool* ok);
   Statement* ParseThrowStatement(bool* ok);
   Expression* MakeCatchContext(Handle<String> id, VariableProxy* value);
@@ skipping 240 matching lines @@
   }
 
   // Returns true if the expression is of type "this.foo".
   V8_INLINE static bool IsThisProperty(Expression* expression) {
     DCHECK(expression != NULL);
     Property* property = expression->AsProperty();
     return property != NULL && property->obj()->IsVariableProxy() &&
            property->obj()->AsVariableProxy()->is_this();
   }
 
+  // This returns true if the expression is an indentifier,
+  // excluding the case of 'this' which has been converted to
+  // a variable proxy.
   V8_INLINE static bool IsIdentifier(Expression* expression) {
+    DCHECK_NOT_NULL(expression);
     VariableProxy* operand = expression->AsVariableProxy();
-    return operand != NULL && !operand->is_this();
+    return operand != nullptr && !operand->is_this();

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

Can you add a TODO to remove this null check now that we have the DCHECK? I
agree that we'd rather not change behavior in this patch, but it seems odd.

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

The DCHECK is for a non-existent AST node.  I think I introduced it here and
removed a check from some call sites.  It's definitely good here.

The check for "operand != nullptr" will fail if the expression is not a variable
proxy.  In this case, the function should return false.  We want to keep this.

[adamk, 2016/09/09 17:14:47]

Ah, indeed, I forgot how AsWhatever works, this is fine.

   }
 
   V8_INLINE static const AstRawString* AsIdentifier(Expression* expression) {
     DCHECK(IsIdentifier(expression));
     return expression->AsVariableProxy()->raw_name();
   }
 
   V8_INLINE bool IsPrototype(const AstRawString* identifier) const {
     return identifier == ast_value_factory()->prototype_string();
   }
 
   V8_INLINE bool IsConstructor(const AstRawString* identifier) const {
     return identifier == ast_value_factory()->constructor_string();
   }
 
   V8_INLINE bool IsDirectEvalCall(Expression* expression) const {
     if (!expression->IsCall()) return false;
     expression = expression->AsCall()->expression();
     return IsIdentifier(expression) && IsEval(AsIdentifier(expression));
   }
 
   V8_INLINE static bool IsBoilerplateProperty(
       ObjectLiteral::Property* property) {
     return ObjectLiteral::IsBoilerplateProperty(property);
   }
 
+  V8_INLINE bool IsNative(Expression* expr) const {
+    return expr != nullptr && expr->AsVariableProxy() != nullptr &&

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

Can expr actually be NULL here?

[marja, 2016/09/09 08:44:59]

Style nit:

How about:

expr->IsVariableProxy() && expr->AsVariableProxy()-> ...

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

Done.  I also removed the !scanner()->literal_contains_escapes() and put it back
to the call site, where it belongs.  This method should not have unnecessary
side effects.

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

The DCHECK_NOT_NULL that I just introduced says no...  :-)
But we may not want to change the behavior.  Should I just add a TODO?

[adamk, 2016/09/09 17:14:47]

I'm fine without a TODO, this has very few callers so I think it's pretty safe
to change.

+           expr->AsVariableProxy()->raw_name() ==
+               ast_value_factory()->native_string() &&
+           !scanner()->literal_contains_escapes();
+  }
+
   V8_INLINE static bool IsArrayIndex(const AstRawString* string,
                                      uint32_t* index) {
     return string->AsArrayIndex(index);
   }
 
   V8_INLINE bool IsUseStrictDirective(Statement* statement) const {
     return IsStringLiteral(statement, ast_value_factory()->use_strict_string());
   }
 
   V8_INLINE bool IsUseAsmDirective(Statement* statement) const {
@@ skipping 133 matching lines @@
       // and we want to report the stack overflow later.
       return;
     }
     pending_error_handler_.ReportMessageAt(source_location.beg_pos,
                                            source_location.end_pos, message,
                                            arg, error_type);
   }
 
   // "null" return type creators.
   V8_INLINE static const AstRawString* EmptyIdentifier() { return nullptr; }
+  V8_INLINE static bool IsEmptyIdentifier(const AstRawString* name) {
+    return name == nullptr;
+  }
   V8_INLINE static Expression* EmptyExpression() { return nullptr; }
   V8_INLINE static Literal* EmptyLiteral() { return nullptr; }
   V8_INLINE static ObjectLiteralProperty* EmptyObjectLiteralProperty() {
     return nullptr;
   }
   V8_INLINE static ClassLiteralProperty* EmptyClassLiteralProperty() {
     return nullptr;
   }
   V8_INLINE static FunctionLiteral* EmptyFunctionLiteral() { return nullptr; }
   V8_INLINE static Block* NullBlock() { return nullptr; }
 
   V8_INLINE static bool IsEmptyExpression(Expression* expr) {
     return expr == nullptr;
   }
 
   // Used in error return values.
   V8_INLINE static ZoneList<Expression*>* NullExpressionList() {
     return nullptr;
   }
   V8_INLINE static bool IsNullExpressionList(ZoneList<Expression*>* exprs) {
     return exprs == nullptr;
   }
   V8_INLINE static ZoneList<Statement*>* NullStatementList() { return nullptr; }
   V8_INLINE static bool IsNullStatementList(ZoneList<Statement*>* stmts) {
     return stmts == nullptr;
   }
   V8_INLINE static Statement* NullStatement() { return nullptr; }
+  V8_INLINE bool IsNullStatement(Statement* stmt) { return stmt == nullptr; }
   V8_INLINE bool IsNullOrEmptyStatement(Statement* stmt) {
     return stmt == nullptr || stmt->IsEmpty();
   }
 
   // Non-NULL empty string.
   V8_INLINE const AstRawString* EmptyIdentifierString() const {
     return ast_value_factory()->empty_string();
   }
 
   // Odd-ball literal creators.
@@ skipping 186 matching lines @@
 
 #ifdef DEBUG
   void Print(AstNode* node);
 #endif  // DEBUG
 };
 
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_PARSING_PARSER_H_