Refactor parser Checkpoints.

Why this is better:

1) Not needing an extra template parameter for Checkpoints ctors. This was
especially confusing since the template parameter was named Parser and Parser is
also used as a type name and is also a concrete type. This CL makes it clear
that ParserTraits::Checkpoint is consturcted with ParserBase<ParserTraits> -
that's the only sensemaking type for the ctor param anyway.

2) This CL makes ParserBase define a Checkpoint base class (which knows how
to create and restore a checkpoint with ParserBase) which
PreParserTraits::Checkpoint and ParserTraits::Checkpoint inherit, and not the
other way around.

This is a more intuitive way to implement the "base functionality + extending
it" concept than the previous solution. The previous solution was to allow
Traits to define a Checkpoint class and make ParserBase<Traits>::ParserCheckpoint
(which defines the base functionality) inherit from it.

3) This CL moves the Checkpoint class definitions out of the SomeTraits::Type
struct; SomeTraits::Type is supposed to be a collection of typedefs and not
contain anything else.

Checkpoints were introduced in r22925 ( https://codereview.chromium.org/443903003 ).

BUG=
R=wingo@igalia.com

Review URL: https://codereview.chromium.org/485473004

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@23266 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/parser.cc b/src/parser.cc
index 41667c2..db4e0fe 100644 (file)
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -341,6 +341,26 @@ class TargetScope BASE_EMBEDDED {
 // ----------------------------------------------------------------------------
 // Implementation of Parser
 
+class ParserTraits::Checkpoint
+    : public ParserBase<ParserTraits>::CheckpointBase {
+ public:
+  explicit Checkpoint(ParserBase<ParserTraits>* parser)
+      : CheckpointBase(parser) {
+    isolate_ = parser->zone()->isolate();
+    saved_ast_node_id_ = isolate_->ast_node_id();
+  }
+
+  void Restore() {
+    CheckpointBase::Restore();
+    isolate_->set_ast_node_id(saved_ast_node_id_);
+  }
+
+ private:
+  Isolate* isolate_;
+  int saved_ast_node_id_;
+};
+
+
 bool ParserTraits::IsEvalOrArguments(const AstRawString* identifier) const {
   return identifier == parser_->ast_value_factory_->eval_string() ||
          identifier == parser_->ast_value_factory_->arguments_string();

diff --git a/src/parser.h b/src/parser.h
index 4bc9b1d..4aa6f71 100644 (file)
--- a/src/parser.h
+++ b/src/parser.h
@@ -355,21 +355,6 @@ class ParserTraits {
     typedef Variable GeneratorVariable;
     typedef v8::internal::Zone Zone;
 
-    class Checkpoint BASE_EMBEDDED {
-     public:
-      template <typename Parser>
-      explicit Checkpoint(Parser* parser) {
-        isolate_ = parser->zone()->isolate();
-        saved_ast_node_id_ = isolate_->ast_node_id();
-      }
-
-      void Restore() { isolate_->set_ast_node_id(saved_ast_node_id_); }
-
-     private:
-      Isolate* isolate_;
-      int saved_ast_node_id_;
-    };
-
     typedef v8::internal::AstProperties AstProperties;
     typedef Vector<VariableProxy*> ParameterIdentifierVector;
 
@@ -388,6 +373,8 @@ class ParserTraits {
     typedef AstNodeFactory<AstConstructionVisitor> Factory;
   };
 
+  class Checkpoint;
+
   explicit ParserTraits(Parser* parser) : parser_(parser) {}
 
   // Custom operations executed when FunctionStates are created and destructed.

diff --git a/src/preparser.cc b/src/preparser.cc
index 7ce8e3d..04907d3 100644 (file)
--- a/src/preparser.cc
+++ b/src/preparser.cc
@@ -32,6 +32,12 @@ int isfinite(double value);
 namespace v8 {
 namespace internal {
 
+class PreParserTraits::Checkpoint
+    : public ParserBase<PreParserTraits>::CheckpointBase {
+ public:
+  explicit Checkpoint(ParserBase<PreParserTraits>* parser)
+      : ParserBase<PreParserTraits>::CheckpointBase(parser) {}
+};
 
 void PreParserTraits::ReportMessageAt(Scanner::Location location,
                                       const char* message,

diff --git a/src/preparser.h b/src/preparser.h
index 5ca161d..f903f64 100644 (file)
--- a/src/preparser.h
+++ b/src/preparser.h
@@ -117,7 +117,7 @@ class ParserBase : public Traits {
   }
 
  protected:
-  friend class Traits::Type::Checkpoint;
+  friend class Traits::Checkpoint;
 
   enum AllowEvalOrArgumentsAsIdentifier {
     kAllowEvalOrArguments,
@@ -129,7 +129,7 @@ class ParserBase : public Traits {
     PARSE_EAGERLY
   };
 
-  class ParserCheckpoint;
+  class CheckpointBase;
 
   // ---------------------------------------------------------------------------
   // FunctionState and BlockState together implement the parser's scope stack.
@@ -226,18 +226,16 @@ class ParserBase : public Traits {
     typename Traits::Type::Factory factory_;
 
     friend class ParserTraits;
-    friend class ParserCheckpoint;
+    friend class CheckpointBase;
   };
 
   // Annoyingly, arrow functions first parse as comma expressions, then when we
   // see the => we have to go back and reinterpret the arguments as being formal
   // parameters.  To do so we need to reset some of the parser state back to
   // what it was before the arguments were first seen.
-  class ParserCheckpoint : public Traits::Type::Checkpoint {
+  class CheckpointBase BASE_EMBEDDED {
    public:
-    template <typename Parser>
-    explicit ParserCheckpoint(Parser* parser)
-        : Traits::Type::Checkpoint(parser) {
+    explicit CheckpointBase(ParserBase* parser) {
       function_state_ = parser->function_state_;
       next_materialized_literal_index_ =
           function_state_->next_materialized_literal_index_;
@@ -246,7 +244,6 @@ class ParserBase : public Traits {
     }
 
     void Restore() {
-      Traits::Type::Checkpoint::Restore();
       function_state_->next_materialized_literal_index_ =
           next_materialized_literal_index_;
       function_state_->next_handler_index_ = next_handler_index_;
@@ -1024,13 +1021,6 @@ class PreParserTraits {
     typedef PreParserScope Scope;
     typedef PreParserScope ScopePtr;
 
-    class Checkpoint BASE_EMBEDDED {
-     public:
-      template <typename Parser>
-      explicit Checkpoint(Parser* parser) {}
-      void Restore() {}
-    };
-
     // PreParser doesn't need to store generator variables.
     typedef void GeneratorVariable;
     // No interaction with Zones.
@@ -1054,6 +1044,8 @@ class PreParserTraits {
     typedef PreParserFactory Factory;
   };
 
+  class Checkpoint;
+
   explicit PreParserTraits(PreParser* pre_parser) : pre_parser_(pre_parser) {}
 
   // Custom operations executed when FunctionStates are created and
@@ -1979,7 +1971,7 @@ ParserBase<Traits>::ParseAssignmentExpression(bool accept_IN, bool* ok) {
   }
 
   if (fni_ != NULL) fni_->Enter();
-  ParserCheckpoint checkpoint(this);
+  typename Traits::Checkpoint checkpoint(this);
   ExpressionT expression =
       this->ParseConditionalExpression(accept_IN, CHECK_OK);