Revision: 19417
Author: [email protected]
Date: Mon Feb 17 15:40:51 2014 UTC
Log: (Pre)Parser: Simplify NewExpression handling (fixed).
Notes:
- We use simple recursion to keep track of how many "new" operators we have
seen
and where.
- This makes the self-baked stack class PositionStack in parser.cc
unnecessary.
- Now the logic is also unified between Parser and PreParser.
- This is a fixed version of r19386.
[email protected]
BUG=v8:3126
LOG=N
Review URL: https://codereview.chromium.org/168583008
http://code.google.com/p/v8/source/detail?r=19417
Modified:
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/src/parser.h
/branches/bleeding_edge/src/preparser.cc
/branches/bleeding_edge/src/preparser.h
/branches/bleeding_edge/test/cctest/test-parsing.cc
=======================================
--- /branches/bleeding_edge/src/parser.cc Fri Feb 14 16:08:14 2014 UTC
+++ /branches/bleeding_edge/src/parser.cc Mon Feb 17 15:40:51 2014 UTC
@@ -46,49 +46,6 @@
namespace v8 {
namespace internal {
-// PositionStack is used for on-stack allocation of token positions for
-// new expressions. Please look at ParseNewExpression.
-
-class PositionStack {
- public:
- explicit PositionStack(bool* ok) : top_(NULL), ok_(ok) {}
- ~PositionStack() {
- ASSERT(!*ok_ || is_empty());
- USE(ok_);
- }
-
- class Element {
- public:
- Element(PositionStack* stack, int value) {
- previous_ = stack->top();
- value_ = value;
- stack->set_top(this);
- }
-
- private:
- Element* previous() { return previous_; }
- int value() { return value_; }
- friend class PositionStack;
- Element* previous_;
- int value_;
- };
-
- bool is_empty() { return top_ == NULL; }
- int pop() {
- ASSERT(!is_empty());
- int result = top_->value();
- top_ = top_->previous();
- return result;
- }
-
- private:
- Element* top() { return top_; }
- void set_top(Element* value) { top_ = value; }
- Element* top_;
- bool* ok_;
-};
-
-
RegExpBuilder::RegExpBuilder(Zone* zone)
: zone_(zone),
pending_empty_(false),
@@ -3292,12 +3249,7 @@
// LeftHandSideExpression ::
// (NewExpression | MemberExpression) ...
- Expression* result;
- if (peek() == Token::NEW) {
- result = ParseNewExpression(CHECK_OK);
- } else {
- result = ParseMemberExpression(CHECK_OK);
- }
+ Expression* result = ParseMemberWithNewPrefixesExpression(CHECK_OK);
while (true) {
switch (peek()) {
@@ -3366,54 +3318,54 @@
}
-Expression* Parser::ParseNewPrefix(PositionStack* stack, bool* ok) {
+Expression* Parser::ParseMemberWithNewPrefixesExpression(bool* ok) {
// NewExpression ::
// ('new')+ MemberExpression
- // The grammar for new expressions is pretty warped. The keyword
- // 'new' can either be a part of the new expression (where it isn't
- // followed by an argument list) or a part of the member expression,
- // where it must be followed by an argument list. To accommodate
- // this, we parse the 'new' keywords greedily and keep track of how
- // many we have parsed. This information is then passed on to the
- // member expression parser, which is only allowed to match argument
- // lists as long as it has 'new' prefixes left
- Expect(Token::NEW, CHECK_OK);
- PositionStack::Element pos(stack, position());
+ // The grammar for new expressions is pretty warped. We can have
several 'new'
+ // keywords following each other, and then a MemberExpression. When we
see '('
+ // after the MemberExpression, it's associated with the rightmost
unassociated
+ // 'new' to create a NewExpression with arguments. However, a
NewExpression
+ // can also occur without arguments.
+
+ // Examples of new expression:
+ // new foo.bar().baz means (new (foo.bar)()).baz
+ // new foo()() means (new foo())()
+ // new new foo()() means (new (new foo())())
+ // new new foo means new (new foo)
+ // new new foo() means new (new foo())
+ // new new foo().bar().baz means (new (new foo()).bar()).baz
- Expression* result;
if (peek() == Token::NEW) {
- result = ParseNewPrefix(stack, CHECK_OK);
- } else {
- result = ParseMemberWithNewPrefixesExpression(stack, CHECK_OK);
+ Consume(Token::NEW);
+ int new_pos = position();
+ Expression* result = ParseMemberWithNewPrefixesExpression(CHECK_OK);
+ if (peek() == Token::LPAREN) {
+ // NewExpression with arguments.
+ ZoneList<Expression*>* args = ParseArguments(CHECK_OK);
+ result = factory()->NewCallNew(result, args, new_pos);
+ // The expression can still continue with . or [ after the arguments.
+ result = ParseMemberExpressionContinuation(result, CHECK_OK);
+ return result;
+ }
+ // NewExpression without arguments.
+ return factory()->NewCallNew(
+ result, new(zone()) ZoneList<Expression*>(0, zone()), new_pos);
}
-
- if (!stack->is_empty()) {
- int last = stack->pop();
- result = factory()->NewCallNew(
- result, new(zone()) ZoneList<Expression*>(0, zone()), last);
- }
- return result;
-}
-
-
-Expression* Parser::ParseNewExpression(bool* ok) {
- PositionStack stack(ok);
- return ParseNewPrefix(&stack, ok);
+ // No 'new' keyword.
+ return ParseMemberExpression(ok);
}
Expression* Parser::ParseMemberExpression(bool* ok) {
- return ParseMemberWithNewPrefixesExpression(NULL, ok);
-}
-
-
-Expression* Parser::ParseMemberWithNewPrefixesExpression(PositionStack*
stack,
- bool* ok) {
// MemberExpression ::
// (PrimaryExpression | FunctionLiteral)
// ('[' Expression ']' | '.' Identifier | Arguments)*
+ // The '[' Expression ']' and '.' Identifier parts are parsed by
+ // ParseMemberExpressionContinuation, and the Arguments part is parsed
by the
+ // caller.
+
// Parse the initial primary or function expression.
Expression* result = NULL;
if (peek() == Token::FUNCTION) {
@@ -3442,13 +3394,22 @@
result = ParsePrimaryExpression(CHECK_OK);
}
+ result = ParseMemberExpressionContinuation(result, CHECK_OK);
+ return result;
+}
+
+
+Expression* Parser::ParseMemberExpressionContinuation(Expression*
expression,
+ bool* ok) {
+ // Parses this part of MemberExpression:
+ // ('[' Expression ']' | '.' Identifier)*
while (true) {
switch (peek()) {
case Token::LBRACK: {
Consume(Token::LBRACK);
int pos = position();
Expression* index = ParseExpression(true, CHECK_OK);
- result = factory()->NewProperty(result, index, pos);
+ expression = factory()->NewProperty(expression, index, pos);
if (fni_ != NULL) {
if (index->IsPropertyName()) {
fni_->PushLiteralName(index->AsLiteral()->AsPropertyName());
@@ -3464,23 +3425,17 @@
Consume(Token::PERIOD);
int pos = position();
Handle<String> name = ParseIdentifierName(CHECK_OK);
- result = factory()->NewProperty(
- result, factory()->NewLiteral(name, pos), pos);
+ expression = factory()->NewProperty(
+ expression, factory()->NewLiteral(name, pos), pos);
if (fni_ != NULL) fni_->PushLiteralName(name);
break;
}
- case Token::LPAREN: {
- if ((stack == NULL) || stack->is_empty()) return result;
- // Consume one of the new prefixes (already parsed).
- ZoneList<Expression*>* args = ParseArguments(CHECK_OK);
- int pos = stack->pop();
- result = factory()->NewCallNew(result, args, pos);
- break;
- }
default:
- return result;
+ return expression;
}
}
+ ASSERT(false);
+ return NULL;
}
=======================================
--- /branches/bleeding_edge/src/parser.h Fri Feb 14 16:08:14 2014 UTC
+++ /branches/bleeding_edge/src/parser.h Mon Feb 17 15:40:51 2014 UTC
@@ -638,11 +638,10 @@
Expression* ParseUnaryExpression(bool* ok);
Expression* ParsePostfixExpression(bool* ok);
Expression* ParseLeftHandSideExpression(bool* ok);
- Expression* ParseNewExpression(bool* ok);
+ Expression* ParseMemberWithNewPrefixesExpression(bool* ok);
Expression* ParseMemberExpression(bool* ok);
- Expression* ParseNewPrefix(PositionStack* stack, bool* ok);
- Expression* ParseMemberWithNewPrefixesExpression(PositionStack* stack,
- bool* ok);
+ Expression* ParseMemberExpressionContinuation(Expression* expression,
+ bool* ok);
Expression* ParseArrayLiteral(bool* ok);
Expression* ParseObjectLiteral(bool* ok);
=======================================
--- /branches/bleeding_edge/src/preparser.cc Fri Feb 14 16:08:14 2014 UTC
+++ /branches/bleeding_edge/src/preparser.cc Mon Feb 17 15:40:51 2014 UTC
@@ -1007,12 +1007,7 @@
// LeftHandSideExpression ::
// (NewExpression | MemberExpression) ...
- Expression result = Expression::Default();
- if (peek() == Token::NEW) {
- result = ParseNewExpression(CHECK_OK);
- } else {
- result = ParseMemberExpression(CHECK_OK);
- }
+ Expression result = ParseMemberWithNewPrefixesExpression(CHECK_OK);
while (true) {
switch (peek()) {
@@ -1052,39 +1047,38 @@
}
-PreParser::Expression PreParser::ParseNewExpression(bool* ok) {
+PreParser::Expression PreParser::ParseMemberWithNewPrefixesExpression(
+ bool* ok) {
// NewExpression ::
// ('new')+ MemberExpression
- // The grammar for new expressions is pretty warped. The keyword
- // 'new' can either be a part of the new expression (where it isn't
- // followed by an argument list) or a part of the member expression,
- // where it must be followed by an argument list. To accommodate
- // this, we parse the 'new' keywords greedily and keep track of how
- // many we have parsed. This information is then passed on to the
- // member expression parser, which is only allowed to match argument
- // lists as long as it has 'new' prefixes left
- unsigned new_count = 0;
- do {
+ // See Parser::ParseNewExpression.
+
+ if (peek() == Token::NEW) {
Consume(Token::NEW);
- new_count++;
- } while (peek() == Token::NEW);
-
- return ParseMemberWithNewPrefixesExpression(new_count, ok);
+ ParseMemberWithNewPrefixesExpression(CHECK_OK);
+ if (peek() == Token::LPAREN) {
+ // NewExpression with arguments.
+ ParseArguments(CHECK_OK);
+ // The expression can still continue with . or [ after the arguments.
+ ParseMemberExpressionContinuation(Expression::Default(), CHECK_OK);
+ }
+ return Expression::Default();
+ }
+ // No 'new' keyword.
+ return ParseMemberExpression(ok);
}
PreParser::Expression PreParser::ParseMemberExpression(bool* ok) {
- return ParseMemberWithNewPrefixesExpression(0, ok);
-}
-
-
-PreParser::Expression PreParser::ParseMemberWithNewPrefixesExpression(
- unsigned new_count, bool* ok) {
// MemberExpression ::
// (PrimaryExpression | FunctionLiteral)
// ('[' Expression ']' | '.' Identifier | Arguments)*
+ // The '[' Expression ']' and '.' Identifier parts are parsed by
+ // ParseMemberExpressionContinuation, and the Arguments part is parsed
by the
+ // caller.
+
// Parse the initial primary or function expression.
Expression result = Expression::Default();
if (peek() == Token::FUNCTION) {
@@ -1107,42 +1101,44 @@
} else {
result = ParsePrimaryExpression(CHECK_OK);
}
+ result = ParseMemberExpressionContinuation(result, CHECK_OK);
+ return result;
+}
+
+PreParser::Expression PreParser::ParseMemberExpressionContinuation(
+ PreParserExpression expression, bool* ok) {
+ // Parses this part of MemberExpression:
+ // ('[' Expression ']' | '.' Identifier)*
while (true) {
switch (peek()) {
case Token::LBRACK: {
Consume(Token::LBRACK);
ParseExpression(true, CHECK_OK);
Expect(Token::RBRACK, CHECK_OK);
- if (result.IsThis()) {
- result = Expression::ThisProperty();
+ if (expression.IsThis()) {
+ expression = Expression::ThisProperty();
} else {
- result = Expression::Default();
+ expression = Expression::Default();
}
break;
}
case Token::PERIOD: {
Consume(Token::PERIOD);
ParseIdentifierName(CHECK_OK);
- if (result.IsThis()) {
- result = Expression::ThisProperty();
+ if (expression.IsThis()) {
+ expression = Expression::ThisProperty();
} else {
- result = Expression::Default();
+ expression = Expression::Default();
}
break;
}
- case Token::LPAREN: {
- if (new_count == 0) return result;
- // Consume one of the new prefixes (already parsed).
- ParseArguments(CHECK_OK);
- new_count--;
- result = Expression::Default();
- break;
- }
default:
- return result;
+ return expression;
}
}
+ ASSERT(false);
+ return PreParserExpression::Default();
}
=======================================
--- /branches/bleeding_edge/src/preparser.h Fri Feb 14 16:08:14 2014 UTC
+++ /branches/bleeding_edge/src/preparser.h Mon Feb 17 15:40:51 2014 UTC
@@ -854,9 +854,10 @@
Expression ParseUnaryExpression(bool* ok);
Expression ParsePostfixExpression(bool* ok);
Expression ParseLeftHandSideExpression(bool* ok);
- Expression ParseNewExpression(bool* ok);
Expression ParseMemberExpression(bool* ok);
- Expression ParseMemberWithNewPrefixesExpression(unsigned new_count,
bool* ok);
+ Expression ParseMemberExpressionContinuation(PreParserExpression
expression,
+ bool* ok);
+ Expression ParseMemberWithNewPrefixesExpression(bool* ok);
Expression ParseArrayLiteral(bool* ok);
Expression ParseObjectLiteral(bool* ok);
Expression ParseV8Intrinsic(bool* ok);
=======================================
--- /branches/bleeding_edge/test/cctest/test-parsing.cc Fri Feb 14 16:08:14
2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-parsing.cc Mon Feb 17 15:40:51
2014 UTC
@@ -2063,3 +2063,61 @@
// or not.
RunParserSyncTest(context_data, statement_data, kSuccessOrError);
}
+
+
+TEST(NoErrorsNewExpression) {
+ const char* context_data[][2] = {
+ {"", ""},
+ {"var f =", ""},
+ { NULL, NULL }
+ };
+
+ const char* statement_data[] = {
+ "new foo",
+ "new foo();",
+ "new foo(1);",
+ "new foo(1, 2);",
+ // The first () will be processed as a part of the NewExpression and
the
+ // second () will be processed as part of LeftHandSideExpression.
+ "new foo()();",
+ // The first () will be processed as a part of the inner NewExpression
and
+ // the second () will be processed as a part of the outer
NewExpression.
+ "new new foo()();",
+ "new foo.bar;",
+ "new foo.bar();",
+ "new foo.bar.baz;",
+ "new foo.bar().baz;",
+ "new foo[bar];",
+ "new foo[bar]();",
+ "new foo[bar][baz];",
+ "new foo[bar]()[baz];",
+ "new foo[bar].baz(baz)()[bar].baz;",
+ "new \"foo\"", // Runtime error
+ "new 1", // Runtime error
+ "new foo++",
+ // This even runs:
+ "(new new Function(\"this.x = 1\")).x;",
+ "new new Test_Two(String, 2).v(0123).length;",
+ NULL
+ };
+
+ RunParserSyncTest(context_data, statement_data, kSuccess);
+}
+
+
+TEST(ErrorsNewExpression) {
+ const char* context_data[][2] = {
+ {"", ""},
+ {"var f =", ""},
+ { NULL, NULL }
+ };
+
+ const char* statement_data[] = {
+ "new foo bar",
+ "new ) foo",
+ "new ++foo",
+ NULL
+ };
+
+ RunParserSyncTest(context_data, statement_data, kError);
+}
--
--
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/groups/opt_out.
