Revision: 19386
Author: [email protected]
Date: Fri Feb 14 15:33:10 2014 UTC
Log: (Pre)Parser: Simplify NewExpression handling.
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.
- It might have been a copy-paste artifact (ParseLeftHandSideExpression ->
ParseMemberWithNewPrefixesExpression) that the logic was so complicated
before.
[email protected]
BUG=v8:3126
LOG=N
Review URL: https://codereview.chromium.org/166943002
http://code.google.com/p/v8/source/detail?r=19386
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 12:13:33 2014 UTC
+++ /branches/bleeding_edge/src/parser.cc Fri Feb 14 15:33:10 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,50 +3318,42 @@
}
-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.
- Expression* result;
+ // 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())
+
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);
+ return factory()->NewCallNew(result, args, new_pos);
+ }
+ // 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)*
@@ -3469,14 +3413,6 @@
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;
}
=======================================
--- /branches/bleeding_edge/src/parser.h Fri Feb 14 12:13:33 2014 UTC
+++ /branches/bleeding_edge/src/parser.h Fri Feb 14 15:33:10 2014 UTC
@@ -638,11 +638,8 @@
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* ParseArrayLiteral(bool* ok);
Expression* ParseObjectLiteral(bool* ok);
=======================================
--- /branches/bleeding_edge/src/preparser.cc Fri Feb 14 11:24:26 2014 UTC
+++ /branches/bleeding_edge/src/preparser.cc Fri Feb 14 15:33:10 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,35 +1047,28 @@
}
-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 {
- Consume(Token::NEW);
- new_count++;
- } while (peek() == Token::NEW);
+ // See Parser::ParseNewExpression.
- return ParseMemberWithNewPrefixesExpression(new_count, ok);
+ if (peek() == Token::NEW) {
+ Consume(Token::NEW);
+ ParseMemberWithNewPrefixesExpression(CHECK_OK);
+ if (peek() == Token::LPAREN) {
+ // NewExpression with arguments.
+ ParseArguments(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)*
@@ -1131,14 +1119,6 @@
}
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;
}
=======================================
--- /branches/bleeding_edge/src/preparser.h Fri Feb 14 11:24:26 2014 UTC
+++ /branches/bleeding_edge/src/preparser.h Fri Feb 14 15:33:10 2014 UTC
@@ -854,9 +854,8 @@
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 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 11:24:26
2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-parsing.cc Fri Feb 14 15:33:10
2014 UTC
@@ -2063,3 +2063,60 @@
// 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;",
+ 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.
