From c532977da3ee1fbd131c38a5450f80a615f7573e Mon Sep 17 00:00:00 2001 From: "marja@chromium.org" Date: Fri, 14 Feb 2014 15:33:10 +0000 Subject: [PATCH] (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. R=ulan@chromium.org BUG=v8:3126 LOG=N Review URL: https://codereview.chromium.org/166943002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19386 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/parser.cc | 118 +++++++++--------------------------- src/parser.h | 5 +- src/preparser.cc | 50 +++++---------- src/preparser.h | 3 +- test/cctest/test-parsing.cc | 57 +++++++++++++++++ 5 files changed, 101 insertions(+), 132 deletions(-) diff --git a/src/parser.cc b/src/parser.cc index 3ef220478d..afeb013b06 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -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 @@ Expression* Parser::ParseLeftHandSideExpression(bool* ok) { // 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::ParseLeftHandSideExpression(bool* ok) { } -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()) - 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* args = ParseArguments(CHECK_OK); + return factory()->NewCallNew(result, args, new_pos); + } + // NewExpression without arguments. + return factory()->NewCallNew( + result, new(zone()) ZoneList(0, zone()), new_pos); } - - if (!stack->is_empty()) { - int last = stack->pop(); - result = factory()->NewCallNew( - result, new(zone()) ZoneList(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 @@ Expression* Parser::ParseMemberWithNewPrefixesExpression(PositionStack* stack, 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* args = ParseArguments(CHECK_OK); - int pos = stack->pop(); - result = factory()->NewCallNew(result, args, pos); - break; - } default: return result; } diff --git a/src/parser.h b/src/parser.h index b21c2754d5..d1d9b61570 100644 --- a/src/parser.h +++ b/src/parser.h @@ -638,11 +638,8 @@ class Parser : public ParserBase { 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); diff --git a/src/preparser.cc b/src/preparser.cc index 6759550eb8..018f99d2b4 100644 --- a/src/preparser.cc +++ b/src/preparser.cc @@ -1007,12 +1007,7 @@ PreParser::Expression PreParser::ParseLeftHandSideExpression(bool* ok) { // 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::ParseLeftHandSideExpression(bool* ok) { } -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 @@ PreParser::Expression PreParser::ParseMemberWithNewPrefixesExpression( } 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; } diff --git a/src/preparser.h b/src/preparser.h index cbc92da851..234dde9bd7 100644 --- a/src/preparser.h +++ b/src/preparser.h @@ -854,9 +854,8 @@ class PreParser : public ParserBase { 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); diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc index 5de0ebae2a..6cb8d8df9a 100644 --- a/test/cctest/test-parsing.cc +++ b/test/cctest/test-parsing.cc @@ -2063,3 +2063,60 @@ TEST(Intrinsics) { // 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); +}