Templatize AstVisitor with its subclass

src/ast/ast.h

Index: src/ast/ast.h
diff --git a/src/ast/ast.h b/src/ast/ast.h
index ea04a654d8d3165a7fb78cf91ee6eeae9a047d44..bb3c7d67d6f43f531490bc8cf7511df262c184b2 100644
--- a/src/ast/ast.h
+++ b/src/ast/ast.h
@@ -108,7 +108,6 @@ namespace internal {
 
 // Forward declarations
 class AstNodeFactory;
-class AstVisitor;
 class Declaration;
 class Module;
 class BreakableStatement;
@@ -129,8 +128,6 @@ typedef ZoneList<Handle<Object>> ZoneObjectList;
 
 
 #define DECLARE_NODE_TYPE(type)                                          \

[Igor Sheludko, 2016/07/14 16:39:54]

As an idea for a next CL: This macro does not make sense anymore.

[Toon Verwaest, 2016/07/15 07:07:19]

Acknowledged.

-  void Accept(AstVisitor* v) override;                                   \
-  AstNode::NodeType node_type() const final { return AstNode::k##type; } \
   friend class AstNodeFactory;
 
 
@@ -190,19 +187,15 @@ DEFINE_OPERATORS_FOR_FLAGS(AstProperties::Flags)
 class AstNode: public ZoneObject {
  public:
 #define DECLARE_TYPE_ENUM(type) k##type,
-  enum NodeType {
-    AST_NODE_LIST(DECLARE_TYPE_ENUM)
-    kInvalid = -1
-  };
+  enum NodeType : uint8_t { kModule = 0, AST_NODE_LIST(DECLARE_TYPE_ENUM) };

[vogelheim, 2016/07/14 11:27:35]

This is super weird: If Module is an AST node, shouldn't it really be in
AST_NODE_LIST? If other uses of AST_NODE_LIST aren't supposed to "see" Module
nodes, maybe keep AST_NODE_LIST but have another macro for the list of
really-truly-all-AstNode-instances?

[Admittedly this issue didn't originate in your CL. So feel free to ignore.]

[Toon Verwaest, 2016/07/15 07:07:19]

I agree, but it's a little involved. This should be done in a different CL I
think.

 #undef DECLARE_TYPE_ENUM
 
   void* operator new(size_t size, Zone* zone) { return zone->New(size); }
 
-  explicit AstNode(int position): position_(position) {}
-  virtual ~AstNode() {}
+  explicit AstNode(int position, NodeType type)

[vogelheim, 2016/07/14 11:27:35]

nitpick: drop explicit (not needed w/ 2 args)

[Here & additional instances below]

[vogelheim, 2016/07/14 11:27:35]

more nitpick: This used to be an abstract class, so you couldn't (inadvertently)
instantiate it. Maybe make the constructor protected, to emulate the this?

[Here, and other formerly uninstantiable classes.]

[Toon Verwaest, 2016/07/15 07:07:19]

Done.

+      : position_(position), node_type_(type) {}
 
-  virtual void Accept(AstVisitor* v) = 0;
-  virtual NodeType node_type() const = 0;
+  NodeType node_type() const { return node_type_; }
   int position() const { return position_; }
 
 #ifdef DEBUG
@@ -230,12 +223,14 @@ class AstNode: public ZoneObject {
   friend class CaseClause;  // Generates AST IDs.
 
   int position_;
+  NodeType node_type_;
 };
 
 
 class Statement : public AstNode {
  public:
-  explicit Statement(Zone* zone, int position) : AstNode(position) {}
+  explicit Statement(Zone* zone, int position, NodeType type)
+      : AstNode(position, type) {}
 
   bool IsEmpty() { return AsEmptyStatement() != NULL; }
   bool IsJump() const;
@@ -350,8 +345,8 @@ class Expression : public AstNode {
   TypeFeedbackId test_id() const { return TypeFeedbackId(local_id(1)); }
 
  protected:
-  Expression(Zone* zone, int pos)
-      : AstNode(pos),
+  Expression(Zone* zone, int pos, NodeType type)
+      : AstNode(pos, type),
         base_id_(BailoutId::None().ToInt()),
         bit_field_(0) {}
   static int parent_num_ids() { return 0; }
@@ -401,8 +396,8 @@ class BreakableStatement : public Statement {
 
  protected:
   BreakableStatement(Zone* zone, ZoneList<const AstRawString*>* labels,
-                     BreakableType breakable_type, int position)
-      : Statement(zone, position),
+                     BreakableType breakable_type, int position, NodeType type)
+      : Statement(zone, position, type),
         labels_(labels),
         breakable_type_(breakable_type),
         base_id_(BailoutId::None().ToInt()) {
@@ -446,7 +441,7 @@ class Block final : public BreakableStatement {
  protected:
   Block(Zone* zone, ZoneList<const AstRawString*>* labels, int capacity,
         bool ignore_completion_value, int pos)
-      : BreakableStatement(zone, labels, TARGET_FOR_NAMED_ONLY, pos),
+      : BreakableStatement(zone, labels, TARGET_FOR_NAMED_ONLY, pos, kBlock),
         statements_(capacity, zone),
         ignore_completion_value_(ignore_completion_value),
         scope_(NULL) {}
@@ -472,7 +467,7 @@ class DoExpression final : public Expression {
 
  protected:
   DoExpression(Zone* zone, Block* block, VariableProxy* result, int pos)
-      : Expression(zone, pos), block_(block), result_(result) {
+      : Expression(zone, pos, kDoExpression), block_(block), result_(result) {
     DCHECK_NOT_NULL(block_);
     DCHECK_NOT_NULL(result_);
   }
@@ -495,8 +490,8 @@ class Declaration : public AstNode {
 
  protected:
   Declaration(Zone* zone, VariableProxy* proxy, VariableMode mode, Scope* scope,
-              int pos)
-      : AstNode(pos), mode_(mode), proxy_(proxy), scope_(scope) {
+              int pos, NodeType type)
+      : AstNode(pos, type), mode_(mode), proxy_(proxy), scope_(scope) {
     DCHECK(IsDeclaredVariableMode(mode));
   }
 
@@ -520,7 +515,7 @@ class VariableDeclaration final : public Declaration {
  protected:
   VariableDeclaration(Zone* zone, VariableProxy* proxy, VariableMode mode,
                       Scope* scope, int pos)
-      : Declaration(zone, proxy, mode, scope, pos) {}
+      : Declaration(zone, proxy, mode, scope, pos, kVariableDeclaration) {}
 };
 
 
@@ -533,13 +528,9 @@ class FunctionDeclaration final : public Declaration {
   InitializationFlag initialization() const { return kCreatedInitialized; }
 
  protected:
-  FunctionDeclaration(Zone* zone,
-                      VariableProxy* proxy,
-                      VariableMode mode,
-                      FunctionLiteral* fun,
-                      Scope* scope,
-                      int pos)
-      : Declaration(zone, proxy, mode, scope, pos),
+  FunctionDeclaration(Zone* zone, VariableProxy* proxy, VariableMode mode,
+                      FunctionLiteral* fun, Scope* scope, int pos)
+      : Declaration(zone, proxy, mode, scope, pos, kFunctionDeclaration),
         fun_(fun) {
     DCHECK(mode == VAR || mode == LET || mode == CONST);
     DCHECK(fun != NULL);
@@ -566,7 +557,7 @@ class ImportDeclaration final : public Declaration {
   ImportDeclaration(Zone* zone, VariableProxy* proxy,
                     const AstRawString* import_name,
                     const AstRawString* module_specifier, Scope* scope, int pos)
-      : Declaration(zone, proxy, CONST, scope, pos),
+      : Declaration(zone, proxy, CONST, scope, pos, kImportDeclaration),
         import_name_(import_name),
         module_specifier_(module_specifier) {}
 
@@ -575,17 +566,18 @@ class ImportDeclaration final : public Declaration {
   const AstRawString* module_specifier_;
 };
 
-
-class Module : public AstNode {
+class Module final : public AstNode {
  public:
   ModuleDescriptor* descriptor() const { return descriptor_; }
   Block* body() const { return body_; }
 
  protected:
   Module(Zone* zone, int pos)
-      : AstNode(pos), descriptor_(ModuleDescriptor::New(zone)), body_(NULL) {}
+      : AstNode(pos, kModule),
+        descriptor_(ModuleDescriptor::New(zone)),
+        body_(NULL) {}
   Module(Zone* zone, ModuleDescriptor* descriptor, int pos, Block* body = NULL)
-      : AstNode(pos), descriptor_(descriptor), body_(body) {}
+      : AstNode(pos, kModule), descriptor_(descriptor), body_(body) {}
 
  private:
   ModuleDescriptor* descriptor_;
@@ -612,8 +604,9 @@ class IterationStatement : public BreakableStatement {
   Label* continue_target()  { return &continue_target_; }
 
  protected:
-  IterationStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos)
-      : BreakableStatement(zone, labels, TARGET_FOR_ANONYMOUS, pos),
+  IterationStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos,
+                     NodeType type)
+      : BreakableStatement(zone, labels, TARGET_FOR_ANONYMOUS, pos, type),
         body_(NULL),
         yield_count_(0),
         first_yield_id_(0) {}
@@ -649,7 +642,7 @@ class DoWhileStatement final : public IterationStatement {
 
  protected:
   DoWhileStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos)
-      : IterationStatement(zone, labels, pos), cond_(NULL) {}
+      : IterationStatement(zone, labels, pos, kDoWhileStatement), cond_(NULL) {}
   static int parent_num_ids() { return IterationStatement::num_ids(); }
 
  private:
@@ -678,7 +671,7 @@ class WhileStatement final : public IterationStatement {
 
  protected:
   WhileStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos)
-      : IterationStatement(zone, labels, pos), cond_(NULL) {}
+      : IterationStatement(zone, labels, pos, kWhileStatement), cond_(NULL) {}
   static int parent_num_ids() { return IterationStatement::num_ids(); }
 
  private:
@@ -717,7 +710,7 @@ class ForStatement final : public IterationStatement {
 
  protected:
   ForStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos)
-      : IterationStatement(zone, labels, pos),
+      : IterationStatement(zone, labels, pos, kForStatement),
         init_(NULL),
         cond_(NULL),
         next_(NULL) {}
@@ -746,8 +739,9 @@ class ForEachStatement : public IterationStatement {
   }
 
  protected:
-  ForEachStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos)
-      : IterationStatement(zone, labels, pos) {}
+  ForEachStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos,
+                   NodeType type)
+      : IterationStatement(zone, labels, pos, type) {}
 };
 
 
@@ -796,7 +790,7 @@ class ForInStatement final : public ForEachStatement {
 
  protected:
   ForInStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos)
-      : ForEachStatement(zone, labels, pos),
+      : ForEachStatement(zone, labels, pos, kForInStatement),
         each_(nullptr),
         subject_(nullptr),
         for_in_type_(SLOW_FOR_IN) {}
@@ -865,7 +859,7 @@ class ForOfStatement final : public ForEachStatement {
 
  protected:
   ForOfStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos)
-      : ForEachStatement(zone, labels, pos),
+      : ForEachStatement(zone, labels, pos, kForOfStatement),
         iterator_(NULL),
         assign_iterator_(NULL),
         next_result_(NULL),
@@ -894,7 +888,7 @@ class ExpressionStatement final : public Statement {
 
  protected:
   ExpressionStatement(Zone* zone, Expression* expression, int pos)
-      : Statement(zone, pos), expression_(expression) { }
+      : Statement(zone, pos, kExpressionStatement), expression_(expression) {}
 
  private:
   Expression* expression_;
@@ -906,7 +900,8 @@ class JumpStatement : public Statement {
   bool IsJump() const { return true; }
 
  protected:
-  explicit JumpStatement(Zone* zone, int pos) : Statement(zone, pos) {}
+  explicit JumpStatement(Zone* zone, int pos, NodeType type)
+      : Statement(zone, pos, type) {}
 };
 
 
@@ -918,7 +913,7 @@ class ContinueStatement final : public JumpStatement {
 
  protected:
   explicit ContinueStatement(Zone* zone, IterationStatement* target, int pos)
-      : JumpStatement(zone, pos), target_(target) { }
+      : JumpStatement(zone, pos, kContinueStatement), target_(target) {}
 
  private:
   IterationStatement* target_;
@@ -933,7 +928,7 @@ class BreakStatement final : public JumpStatement {
 
  protected:
   explicit BreakStatement(Zone* zone, BreakableStatement* target, int pos)
-      : JumpStatement(zone, pos), target_(target) { }
+      : JumpStatement(zone, pos, kBreakStatement), target_(target) {}
 
  private:
   BreakableStatement* target_;
@@ -950,7 +945,7 @@ class ReturnStatement final : public JumpStatement {
 
  protected:
   explicit ReturnStatement(Zone* zone, Expression* expression, int pos)
-      : JumpStatement(zone, pos), expression_(expression) { }
+      : JumpStatement(zone, pos, kReturnStatement), expression_(expression) {}
 
  private:
   Expression* expression_;
@@ -975,7 +970,7 @@ class WithStatement final : public Statement {
  protected:
   WithStatement(Zone* zone, Scope* scope, Expression* expression,
                 Statement* statement, int pos)
-      : Statement(zone, pos),
+      : Statement(zone, pos, kWithStatement),
         scope_(scope),
         expression_(expression),
         statement_(statement),
@@ -1048,7 +1043,8 @@ class SwitchStatement final : public BreakableStatement {
 
  protected:
   SwitchStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos)
-      : BreakableStatement(zone, labels, TARGET_FOR_ANONYMOUS, pos),
+      : BreakableStatement(zone, labels, TARGET_FOR_ANONYMOUS, pos,
+                           kSwitchStatement),
         tag_(NULL),
         cases_(NULL) {}
 
@@ -1092,7 +1088,7 @@ class IfStatement final : public Statement {
  protected:
   IfStatement(Zone* zone, Expression* condition, Statement* then_statement,
               Statement* else_statement, int pos)
-      : Statement(zone, pos),
+      : Statement(zone, pos, kIfStatement),
         condition_(condition),
         then_statement_(then_statement),
         else_statement_(else_statement),
@@ -1120,8 +1116,8 @@ class TryStatement : public Statement {
   void set_try_block(Block* b) { try_block_ = b; }
 
  protected:
-  TryStatement(Zone* zone, Block* try_block, int pos)
-      : Statement(zone, pos), try_block_(try_block) {}
+  TryStatement(Zone* zone, Block* try_block, int pos, NodeType type)
+      : Statement(zone, pos, type), try_block_(try_block) {}
 
  private:
   Block* try_block_;
@@ -1153,7 +1149,7 @@ class TryCatchStatement final : public TryStatement {
   TryCatchStatement(Zone* zone, Block* try_block, Scope* scope,
                     Variable* variable, Block* catch_block,
                     bool clear_pending_message, int pos)
-      : TryStatement(zone, try_block, pos),
+      : TryStatement(zone, try_block, pos, kTryCatchStatement),
         scope_(scope),
         variable_(variable),
         catch_block_(catch_block),
@@ -1177,7 +1173,8 @@ class TryFinallyStatement final : public TryStatement {
  protected:
   TryFinallyStatement(Zone* zone, Block* try_block, Block* finally_block,
                       int pos)
-      : TryStatement(zone, try_block, pos), finally_block_(finally_block) {}
+      : TryStatement(zone, try_block, pos, kTryFinallyStatement),
+        finally_block_(finally_block) {}
 
  private:
   Block* finally_block_;
@@ -1194,7 +1191,8 @@ class DebuggerStatement final : public Statement {
 
  protected:
   explicit DebuggerStatement(Zone* zone, int pos)
-      : Statement(zone, pos), base_id_(BailoutId::None().ToInt()) {}
+      : Statement(zone, pos, kDebuggerStatement),
+        base_id_(BailoutId::None().ToInt()) {}
   static int parent_num_ids() { return 0; }
 
   int base_id() const {
@@ -1214,7 +1212,8 @@ class EmptyStatement final : public Statement {
   DECLARE_NODE_TYPE(EmptyStatement)
 
  protected:
-  explicit EmptyStatement(Zone* zone, int pos): Statement(zone, pos) {}
+  explicit EmptyStatement(Zone* zone, int pos)
+      : Statement(zone, pos, kEmptyStatement) {}
 };
 
 
@@ -1232,7 +1231,7 @@ class SloppyBlockFunctionStatement final : public Statement {
 
  private:
   SloppyBlockFunctionStatement(Zone* zone, Statement* statement, Scope* scope)
-      : Statement(zone, kNoSourcePosition),
+      : Statement(zone, kNoSourcePosition, kSloppyBlockFunctionStatement),
         statement_(statement),
         scope_(scope) {}
 
@@ -1275,7 +1274,7 @@ class Literal final : public Expression {
 
  protected:
   Literal(Zone* zone, const AstValue* value, int position)
-      : Expression(zone, position), value_(value) {}
+      : Expression(zone, position, kLiteral), value_(value) {}
   static int parent_num_ids() { return Expression::num_ids(); }
 
  private:
@@ -1299,8 +1298,8 @@ class MaterializedLiteral : public Expression {
   }
 
  protected:
-  MaterializedLiteral(Zone* zone, int literal_index, int pos)
-      : Expression(zone, pos),
+  MaterializedLiteral(Zone* zone, int literal_index, int pos, NodeType type)
+      : Expression(zone, pos, type),
         literal_index_(literal_index),
         is_simple_(false),
         depth_(0) {}
@@ -1483,7 +1482,7 @@ class ObjectLiteral final : public MaterializedLiteral {
  protected:
   ObjectLiteral(Zone* zone, ZoneList<Property*>* properties, int literal_index,
                 int boilerplate_properties, int pos)
-      : MaterializedLiteral(zone, literal_index, pos),
+      : MaterializedLiteral(zone, literal_index, pos, kObjectLiteral),
         properties_(properties),
         boilerplate_properties_(boilerplate_properties),
         fast_elements_(false),
@@ -1536,7 +1535,7 @@ class RegExpLiteral final : public MaterializedLiteral {
  protected:
   RegExpLiteral(Zone* zone, const AstRawString* pattern, int flags,
                 int literal_index, int pos)
-      : MaterializedLiteral(zone, literal_index, pos),
+      : MaterializedLiteral(zone, literal_index, pos, kRegExpLiteral),
         pattern_(pattern),
         flags_(flags) {
     set_depth(1);
@@ -1610,7 +1609,7 @@ class ArrayLiteral final : public MaterializedLiteral {
  protected:
   ArrayLiteral(Zone* zone, ZoneList<Expression*>* values,
                int first_spread_index, int literal_index, int pos)
-      : MaterializedLiteral(zone, literal_index, pos),
+      : MaterializedLiteral(zone, literal_index, pos, kArrayLiteral),
         values_(values),
         first_spread_index_(first_spread_index) {}
   static int parent_num_ids() { return MaterializedLiteral::num_ids(); }
@@ -1796,7 +1795,7 @@ class Property final : public Expression {
 
  protected:
   Property(Zone* zone, Expression* obj, Expression* key, int pos)
-      : Expression(zone, pos),
+      : Expression(zone, pos, kProperty),
         bit_field_(IsForCallField::encode(false) |
                    IsStringAccessField::encode(false) |
                    InlineCacheStateField::encode(UNINITIALIZED)),
@@ -1916,7 +1915,7 @@ class Call final : public Expression {
  protected:
   Call(Zone* zone, Expression* expression, ZoneList<Expression*>* arguments,
        int pos)
-      : Expression(zone, pos),
+      : Expression(zone, pos, kCall),
         expression_(expression),
         arguments_(arguments),
         bit_field_(IsUninitializedField::encode(false)) {
@@ -1987,7 +1986,7 @@ class CallNew final : public Expression {
  protected:
   CallNew(Zone* zone, Expression* expression, ZoneList<Expression*>* arguments,
           int pos)
-      : Expression(zone, pos),
+      : Expression(zone, pos, kCallNew),
         expression_(expression),
         arguments_(arguments),
         is_monomorphic_(false) {}
@@ -2036,11 +2035,13 @@ class CallRuntime final : public Expression {
  protected:
   CallRuntime(Zone* zone, const Runtime::Function* function,
               ZoneList<Expression*>* arguments, int pos)
-      : Expression(zone, pos), function_(function), arguments_(arguments) {}
+      : Expression(zone, pos, kCallRuntime),
+        function_(function),
+        arguments_(arguments) {}
 
   CallRuntime(Zone* zone, int context_index, ZoneList<Expression*>* arguments,
               int pos)
-      : Expression(zone, pos),
+      : Expression(zone, pos, kCallRuntime),
         function_(NULL),
         context_index_(context_index),
         arguments_(arguments) {}
@@ -2074,7 +2075,9 @@ class UnaryOperation final : public Expression {
 
  protected:
   UnaryOperation(Zone* zone, Token::Value op, Expression* expression, int pos)
-      : Expression(zone, pos), op_(op), expression_(expression) {
+      : Expression(zone, pos, kUnaryOperation),
+        op_(op),
+        expression_(expression) {
     DCHECK(Token::IsUnaryOp(op));
   }
   static int parent_num_ids() { return Expression::num_ids(); }
@@ -2133,7 +2136,7 @@ class BinaryOperation final : public Expression {
  protected:
   BinaryOperation(Zone* zone, Token::Value op, Expression* left,
                   Expression* right, int pos)
-      : Expression(zone, pos),
+      : Expression(zone, pos, kBinaryOperation),
         op_(static_cast<byte>(op)),
         has_fixed_right_arg_(false),
         fixed_right_arg_value_(0),
@@ -2204,7 +2207,7 @@ class CountOperation final : public Expression {
  protected:
   CountOperation(Zone* zone, Token::Value op, bool is_prefix, Expression* expr,
                  int pos)
-      : Expression(zone, pos),
+      : Expression(zone, pos, kCountOperation),
         bit_field_(
             IsPrefixField::encode(is_prefix) | KeyTypeField::encode(ELEMENT) |
             StoreModeField::encode(STANDARD_STORE) | TokenField::encode(op)),
@@ -2257,7 +2260,7 @@ class CompareOperation final : public Expression {
  protected:
   CompareOperation(Zone* zone, Token::Value op, Expression* left,
                    Expression* right, int pos)
-      : Expression(zone, pos),
+      : Expression(zone, pos, kCompareOperation),
         op_(op),
         left_(left),
         right_(right),
@@ -2290,7 +2293,9 @@ class Spread final : public Expression {
 
  protected:
   Spread(Zone* zone, Expression* expression, int pos, int expr_pos)
-      : Expression(zone, pos), expression_(expression), expr_pos_(expr_pos) {}
+      : Expression(zone, pos, kSpread),
+        expression_(expression),
+        expr_pos_(expr_pos) {}
   static int parent_num_ids() { return Expression::num_ids(); }
 
  private:
@@ -2325,7 +2330,7 @@ class Conditional final : public Expression {
  protected:
   Conditional(Zone* zone, Expression* condition, Expression* then_expression,
               Expression* else_expression, int position)
-      : Expression(zone, position),
+      : Expression(zone, position, kConditional),
         condition_(condition),
         then_expression_(then_expression),
         else_expression_(else_expression) {}
@@ -2452,7 +2457,7 @@ class RewritableExpression : public Expression {
 
  protected:
   RewritableExpression(Zone* zone, Expression* expression)
-      : Expression(zone, expression->position()),
+      : Expression(zone, expression->position(), kRewritableExpression),
         is_rewritten_(false),
         expr_(expression) {
     DCHECK(!expression->IsRewritableExpression());
@@ -2483,7 +2488,7 @@ class Yield final : public Expression {
  protected:
   Yield(Zone* zone, Expression* generator_object, Expression* expression,
         int pos)
-      : Expression(zone, pos),
+      : Expression(zone, pos, kYield),
         generator_object_(generator_object),
         expression_(expression),
         yield_id_(-1) {}
@@ -2504,7 +2509,7 @@ class Throw final : public Expression {
 
  protected:
   Throw(Zone* zone, Expression* exception, int pos)
-      : Expression(zone, pos), exception_(exception) {}
+      : Expression(zone, pos, kThrow), exception_(exception) {}
 
  private:
   Expression* exception_;
@@ -2653,7 +2658,7 @@ class FunctionLiteral final : public Expression {
                   ParameterFlag has_duplicate_parameters,
                   EagerCompileHint eager_compile_hint, FunctionKind kind,
                   int position, bool is_function)
-      : Expression(zone, position),
+      : Expression(zone, position, kFunctionLiteral),
         raw_name_(name),
         scope_(scope),
         body_(body),
@@ -2755,7 +2760,7 @@ class ClassLiteral final : public Expression {
                Expression* extends, FunctionLiteral* constructor,
                ZoneList<Property*>* properties, int start_position,
                int end_position)
-      : Expression(zone, start_position),
+      : Expression(zone, start_position, kClassLiteral),
         scope_(scope),
         class_variable_proxy_(class_variable_proxy),
         extends_(extends),
@@ -2789,7 +2794,9 @@ class NativeFunctionLiteral final : public Expression {
  protected:
   NativeFunctionLiteral(Zone* zone, const AstRawString* name,
                         v8::Extension* extension, int pos)
-      : Expression(zone, pos), name_(name), extension_(extension) {}
+      : Expression(zone, pos, kNativeFunctionLiteral),
+        name_(name),
+        extension_(extension) {}
 
  private:
   const AstRawString* name_;
@@ -2802,7 +2809,7 @@ class ThisFunction final : public Expression {
   DECLARE_NODE_TYPE(ThisFunction)
 
  protected:
-  ThisFunction(Zone* zone, int pos) : Expression(zone, pos) {}
+  ThisFunction(Zone* zone, int pos) : Expression(zone, pos, kThisFunction) {}
 };
 
 
@@ -2818,7 +2825,9 @@ class SuperPropertyReference final : public Expression {
  protected:
   SuperPropertyReference(Zone* zone, VariableProxy* this_var,
                          Expression* home_object, int pos)
-      : Expression(zone, pos), this_var_(this_var), home_object_(home_object) {
+      : Expression(zone, pos, kSuperPropertyReference),
+        this_var_(this_var),
+        home_object_(home_object) {
     DCHECK(this_var->is_this());
     DCHECK(home_object->IsProperty());
   }
@@ -2844,7 +2853,7 @@ class SuperCallReference final : public Expression {
   SuperCallReference(Zone* zone, VariableProxy* this_var,
                      VariableProxy* new_target_var,
                      VariableProxy* this_function_var, int pos)
-      : Expression(zone, pos),
+      : Expression(zone, pos, kSuperCallReference),
         this_var_(this_var),
         new_target_var_(new_target_var),
         this_function_var_(this_function_var) {
@@ -2867,7 +2876,8 @@ class EmptyParentheses final : public Expression {
   DECLARE_NODE_TYPE(EmptyParentheses)
 
  private:
-  EmptyParentheses(Zone* zone, int pos) : Expression(zone, pos) {}
+  EmptyParentheses(Zone* zone, int pos)
+      : Expression(zone, pos, kEmptyParentheses) {}
 };
 
 
@@ -2878,30 +2888,63 @@ class EmptyParentheses final : public Expression {
 // Basic visitor
 // - leaf node visitors are abstract.

[vogelheim, 2016/07/14 11:27:35]

nitpick: comment is untrue now, is it? 

It might be good to have a 1-line comment on how this is intended to be used,
e.g.:
// Sub-class should parameterize AstVisitor with itself. E.g.:
//   class SpecificVisitor : public AstVisitor<SpecificVisitor> { .. }

[Toon Verwaest, 2016/07/15 07:07:19]

Done.

 
+template <class Subclass>
 class AstVisitor BASE_EMBEDDED {
  public:
-  AstVisitor() {}
-  virtual ~AstVisitor() {}
-
-  // Stack overflow check and dynamic dispatch.
-  virtual void Visit(AstNode* node) = 0;
+  void Visit(AstNode* node) { This()->Visit(node); }
 
   // Iteration left-to-right.

[vogelheim, 2016/07/14 11:27:35]

nitpick: In the previous version, I read the comment as applying to all 3
following Visit*(..) methods. Here, it looks like it only applied to
VisitDeclarations, which - while technically correct - doesn't make that much
sense.

[Toon Verwaest, 2016/07/15 07:07:19]

Done.

-  virtual void VisitDeclarations(ZoneList<Declaration*>* declarations);
-  virtual void VisitStatements(ZoneList<Statement*>* statements);
-  virtual void VisitExpressions(ZoneList<Expression*>* expressions);
+  void VisitDeclarations(ZoneList<Declaration*>* declarations) {
+    for (int i = 0; i < declarations->length(); i++) {
+      Visit(declarations->at(i));
+    }
+  }
 
-  // Individual AST nodes.
-#define DEF_VISIT(type)                         \
-  virtual void Visit##type(type* node) = 0;
-  AST_NODE_LIST(DEF_VISIT)
-#undef DEF_VISIT
+  void VisitStatements(ZoneList<Statement*>* statements) {
+    for (int i = 0; i < statements->length(); i++) {
+      Statement* stmt = statements->at(i);
+      Visit(stmt);
+      if (stmt->IsJump()) break;
+    }
+  }
+
+  void VisitExpressions(ZoneList<Expression*>* expressions) {
+    for (int i = 0; i < expressions->length(); i++) {
+      // The variable statement visiting code may pass NULL expressions
+      // to this code. Maybe this should be handled by introducing an
+      // undefined expression or literal?  Revisit this code if this
+      // changes
+      Expression* expression = expressions->at(i);
+      if (expression != NULL) Visit(expression);
+    }
+  }
+
+ private:
+  Subclass* This() { return static_cast<Subclass*>(this); }
 };
 
+#define GENERATE_VISIT_CASE(NodeType) \
+  case AstNode::k##NodeType:          \
+    return Visit##NodeType(static_cast<NodeType*>(node));
+
+#define GENERATE_AST_VISITOR_SWITCH()  \
+  switch (node->node_type()) {         \
+    AST_NODE_LIST(GENERATE_VISIT_CASE) \
+    case AstNode::kModule:             \
+      UNREACHABLE();                   \
+  }
+
+#define DEFINE_AST_VISITOR_MEMBERS_WITHOUT_STACKOVERFLOW()    \

[vogelheim, 2016/07/14 11:27:35]

Super nitpick:
- I was wondering about the name of the macro, and it really only makes any
sense if one has read the DEFINE_AST_VISITOR_SUBCLASS_MEMBERS one below...
- Maybe WITHOUT_STACKOVERFLOW_CHECK? Or just move it below the other definition?

[Toon Verwaest, 2016/07/15 07:07:19]

Done. This macro should probably go away. We could probably pass in the stack
limit directly rather than the isolate. I think we already do this somewhere.
Different CL though.

+ public:                                                      \
+  void Visit(AstNode* node) { GENERATE_AST_VISITOR_SWITCH() } \
+                                                              \
+ private:
+
 #define DEFINE_AST_VISITOR_SUBCLASS_MEMBERS()               \
  public:                                                    \
-  void Visit(AstNode* node) final {                         \
-    if (!CheckStackOverflow()) node->Accept(this);          \
+  void Visit(AstNode* node) {                               \
+    if (CheckStackOverflow()) return;                       \
+    GENERATE_AST_VISITOR_SWITCH()                           \
   }                                                         \
                                                             \
   void SetStackOverflow() { stack_overflow_ = true; }       \
@@ -2959,7 +3002,6 @@ class AstVisitor BASE_EMBEDDED {
                                                       \
  protected:                                           \
   AstNode* replacement_
-
 // Generic macro for rewriting things; `GET` is the expression to be
 // rewritten; `SET` is a command that should do the rewriting, i.e.
 // something sensible with the variable called `replacement`.
@@ -2999,18 +3041,18 @@ class AstVisitor BASE_EMBEDDED {
 // Traversing visitor
 // - fully traverses the entire AST.
 
-class AstTraversalVisitor : public AstVisitor {
+class AstTraversalVisitor : public AstVisitor<AstTraversalVisitor> {
  public:
   explicit AstTraversalVisitor(Isolate* isolate);
   explicit AstTraversalVisitor(uintptr_t stack_limit);
   virtual ~AstTraversalVisitor() {}
 
   // Iteration left-to-right.
-  void VisitDeclarations(ZoneList<Declaration*>* declarations) override;
-  void VisitStatements(ZoneList<Statement*>* statements) override;
+  void VisitDeclarations(ZoneList<Declaration*>* declarations);
+  void VisitStatements(ZoneList<Statement*>* statements);
 
 // Individual nodes
-#define DECLARE_VISIT(type) void Visit##type(type* node) override;
+#define DECLARE_VISIT(type) virtual void Visit##type(type* node);
   AST_NODE_LIST(DECLARE_VISIT)
 #undef DECLARE_VISIT