[parser] Refactor AstTraversalVisitor

src/ast/ast.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_AST_AST_H_
 #define V8_AST_AST_H_
 
 #include "src/ast/ast-value-factory.h"
 #include "src/ast/modules.h"
 #include "src/ast/variables.h"
@@ skipping 2868 matching lines @@
 
 
 // ----------------------------------------------------------------------------
 // Basic visitor
 // Sub-class should parametrize AstVisitor with itself, e.g.:
 //   class SpecificVisitor : public AstVisitor<SpecificVisitor> { ... }
 
 template <class Subclass>
 class AstVisitor BASE_EMBEDDED {
  public:
-  void Visit(AstNode* node) { This()->Visit(node); }
+  void Visit(AstNode* node) { impl()->Visit(node); }
 
   void VisitDeclarations(ZoneList<Declaration*>* declarations) {
     for (int i = 0; i < declarations->length(); i++) {
       Visit(declarations->at(i));
     }
   }
 
   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); }
+ protected:
+  Subclass* impl() { return static_cast<Subclass*>(this); }
 };
 
-#define GENERATE_VISIT_CASE(NodeType) \
-  case AstNode::k##NodeType:          \
-    return Visit##NodeType(static_cast<NodeType*>(node));
+#define GENERATE_VISIT_CASE(NodeType)                                   \
+  case AstNode::k##NodeType:                                            \
+    return this->impl()->Visit##NodeType(static_cast<NodeType*>(node));

[Toon Verwaest, 2016/07/21 11:16:59]

This change isn't necessary if you add DEFINE_AST_VISITOR_SUBCLASS_MEMBERS() to
all leaf classes. That also means we don't need to make the private superclass
public in the Rewriter. (Granted, that's even a style-guide violation; I only
care because it may change semantics.)

I do like the idea of using this->impl() with the specific visit methods; but if
we decide to do this we should probably move this to AstVisitor<Subclass> in a
different CL, and remove it from these macros.

[nickie, 2016/07/22 08:40:18]

This was explored in https://codereview.chromium.org/2166333002/

 
 #define GENERATE_AST_VISITOR_SWITCH()  \
   switch (node->node_type()) {         \
     AST_NODE_LIST(GENERATE_VISIT_CASE) \
     case AstNode::kModule:             \
       UNREACHABLE();                   \
   }
 
 #define DEFINE_AST_VISITOR_SUBCLASS_MEMBERS()               \
  public:                                                    \
@@ skipping 92 matching lines @@
 // `at` and `Set`.
 #define AST_REWRITE_LIST_ELEMENT(Type, list, index)                        \
   do {                                                                     \
     auto _list = (list);                                                   \
     auto _index = (index);                                                 \
     AST_REWRITE(Type, _list->at(_index), _list->Set(_index, replacement)); \
   } while (false)
 
 
 // ----------------------------------------------------------------------------
-// Traversing visitor
-// - fully traverses the entire AST.
-
-// This AstVistor is not final, and provides the AstVisitor methods as virtual
-// methods so they can be specialized by subclasses.
-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);
-  void VisitStatements(ZoneList<Statement*>* statements);
-
-// Individual nodes
-#define DECLARE_VISIT(type) virtual void Visit##type(type* node);
-  AST_NODE_LIST(DECLARE_VISIT)
-#undef DECLARE_VISIT
-
- protected:
-  int depth() { return depth_; }
-
- private:
-  DEFINE_AST_VISITOR_SUBCLASS_MEMBERS();
-
-  int depth_;
-
-  DISALLOW_COPY_AND_ASSIGN(AstTraversalVisitor);
-};
-
-// ----------------------------------------------------------------------------
 // AstNode factory
 
 class AstNodeFactory final BASE_EMBEDDED {
  public:
   explicit AstNodeFactory(AstValueFactory* ast_value_factory)
       : local_zone_(ast_value_factory->zone()),
         parser_zone_(ast_value_factory->zone()),
         ast_value_factory_(ast_value_factory) {}
 
   AstValueFactory* ast_value_factory() const { return ast_value_factory_; }
@@ skipping 466 matching lines @@
                                     : NULL;                                   \
   }
 AST_NODE_LIST(DECLARE_NODE_FUNCTIONS)
 #undef DECLARE_NODE_FUNCTIONS
 
 
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_AST_AST_H_