Reviewers: wingo, rossberg,
Message:
wingo, rossberg, ptal.
Description:
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 the 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 and
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.
BUG=
Please review this at https://codereview.chromium.org/485473004/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+36, -31 lines):
M src/parser.h
M src/parser.cc
M src/preparser.h
M src/preparser.cc
Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index
41667c2eacd66793dcadc43b65547f807a8c9db3..db4e0fed2416554afc676609979d00e0317acf46
100644
--- 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();
Index: src/parser.h
diff --git a/src/parser.h b/src/parser.h
index
93af6998681e1e251cd225179282433af33a06c2..a6ba8c0e553e92663668d284ace55f065d1d9cb8
100644
--- 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.
Index: src/preparser.cc
diff --git a/src/preparser.cc b/src/preparser.cc
index
7ce8e3d91aa4628d1d5dde2474ac10cc9164bab7..04907d3c1cab9f3d378b347e47992a0717300a70
100644
--- 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,
Index: src/preparser.h
diff --git a/src/preparser.h b/src/preparser.h
index
78bfd65499a074d12ac105df260f75a152d6932a..9cbcb8369668eb649f1870b78afca122e74de078
100644
--- a/src/preparser.h
+++ b/src/preparser.h
@@ -115,7 +115,7 @@ class ParserBase : public Traits {
}
protected:
- friend class Traits::Type::Checkpoint;
+ friend class Traits::Checkpoint;
enum AllowEvalOrArgumentsAsIdentifier {
kAllowEvalOrArguments,
@@ -127,7 +127,7 @@ class ParserBase : public Traits {
PARSE_EAGERLY
};
- class ParserCheckpoint;
+ class CheckpointBase;
//
---------------------------------------------------------------------------
// FunctionState and BlockState together implement the parser's scope
stack.
@@ -224,18 +224,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_;
@@ -244,7 +242,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_;
@@ -1021,13 +1018,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.
@@ -1051,6 +1041,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
@@ -1997,7 +1989,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);
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
