From: marja@chromium.org Date: Mon, 17 Feb 2014 15:40:51 +0000 (+0000) Subject: (Pre)Parser: Simplify NewExpression handling (fixed). X-Git-Tag: upstream/4.7.83~10676 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=73c4a61848f190c2025b7c7a2537e52bab02bab0;p=platform%2Fupstream%2Fv8.git (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. R=ulan@chromium.org BUG=v8:3126 LOG=N Review URL: https://codereview.chromium.org/168583008 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19417 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/parser.cc b/src/parser.cc index 3ef2204..4090597 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,54 +3318,54 @@ 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()); - - Expression* result; - if (peek() == Token::NEW) { - result = ParseNewPrefix(stack, CHECK_OK); - } else { - result = ParseMemberWithNewPrefixesExpression(stack, CHECK_OK); - } - - if (!stack->is_empty()) { - int last = stack->pop(); - result = factory()->NewCallNew( - result, new(zone()) ZoneList(0, zone()), last); - } - return result; -} + // 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* Parser::ParseNewExpression(bool* ok) { - PositionStack stack(ok); - return ParseNewPrefix(&stack, ok); + if (peek() == Token::NEW) { + Consume(Token::NEW); + int new_pos = position(); + Expression* result = ParseMemberWithNewPrefixesExpression(CHECK_OK); + if (peek() == Token::LPAREN) { + // NewExpression with arguments. + ZoneList* 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(0, zone()), new_pos); + } + // 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 @@ Expression* Parser::ParseMemberWithNewPrefixesExpression(PositionStack* stack, 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 @@ Expression* Parser::ParseMemberWithNewPrefixesExpression(PositionStack* stack, Consume(Token::PERIOD); int pos = position(); Handle 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* args = ParseArguments(CHECK_OK); - int pos = stack->pop(); - result = factory()->NewCallNew(result, args, pos); - break; - } default: - return result; + return expression; } } + ASSERT(false); + return NULL; } diff --git a/src/parser.h b/src/parser.h index b21c275..85a219e 100644 --- a/src/parser.h +++ b/src/parser.h @@ -638,11 +638,10 @@ 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* ParseMemberExpressionContinuation(Expression* expression, + bool* ok); Expression* ParseArrayLiteral(bool* ok); Expression* ParseObjectLiteral(bool* ok); diff --git a/src/preparser.cc b/src/preparser.cc index 6759550..fc4481f 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,39 +1047,38 @@ 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); + // 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 @@ PreParser::Expression PreParser::ParseMemberWithNewPrefixesExpression( } 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(); } diff --git a/src/preparser.h b/src/preparser.h index cbc92da..5cb0a9d 100644 --- a/src/preparser.h +++ b/src/preparser.h @@ -854,9 +854,10 @@ 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 ParseMemberExpressionContinuation(PreParserExpression expression, + 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 5de0eba..30e97aa 100644 --- a/test/cctest/test-parsing.cc +++ b/test/cctest/test-parsing.cc @@ -2063,3 +2063,61 @@ 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;", + "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); +}