From 9271b0ccf9ddb217deb1f0b9ef9b59b64dc40214 Mon Sep 17 00:00:00 2001 From: wingo Date: Fri, 21 Aug 2015 04:33:28 -0700 Subject: [PATCH] Parse arrow functions at proper precedence level BUG=v8:4211 LOG=Y R=rossberg@chromium.org Review URL: https://codereview.chromium.org/1286383005 Cr-Commit-Position: refs/heads/master@{#30298} --- src/ast-literal-reindexer.cc | 3 ++ src/ast-numbering.cc | 5 ++ src/ast.h | 18 ++++++- src/compiler/ast-graph-builder.cc | 6 +++ src/compiler/ast-loop-assignment-analyzer.cc | 3 ++ src/full-codegen/full-codegen.cc | 5 ++ src/hydrogen.cc | 5 ++ src/interpreter/bytecode-generator.cc | 5 ++ src/messages.h | 2 - src/parser.cc | 2 + src/pattern-rewriter.cc | 5 ++ src/preparser.h | 55 ++++++++++---------- src/prettyprinter.cc | 15 ++++++ src/typing.cc | 5 ++ test/message/arrow-missing.out | 6 +-- test/mjsunit/harmony/regress/regress-4211.js | 12 +++++ 16 files changed, 118 insertions(+), 34 deletions(-) create mode 100644 test/mjsunit/harmony/regress/regress-4211.js diff --git a/src/ast-literal-reindexer.cc b/src/ast-literal-reindexer.cc index 860a3961f..d9a6ed1c4 100644 --- a/src/ast-literal-reindexer.cc +++ b/src/ast-literal-reindexer.cc @@ -174,6 +174,9 @@ void AstLiteralReindexer::VisitSpread(Spread* node) { } +void AstLiteralReindexer::VisitEmptyParentheses(EmptyParentheses* node) {} + + void AstLiteralReindexer::VisitForInStatement(ForInStatement* node) { Visit(node->each()); Visit(node->enumerable()); diff --git a/src/ast-numbering.cc b/src/ast-numbering.cc index dc0528caa..7338cc672 100644 --- a/src/ast-numbering.cc +++ b/src/ast-numbering.cc @@ -371,6 +371,11 @@ void AstNumberingVisitor::VisitSpread(Spread* node) { } +void AstNumberingVisitor::VisitEmptyParentheses(EmptyParentheses* node) { + UNREACHABLE(); +} + + void AstNumberingVisitor::VisitForInStatement(ForInStatement* node) { IncrementNodeCount(); DisableSelfOptimization(); diff --git a/src/ast.h b/src/ast.h index 136604138..6e9390f6e 100644 --- a/src/ast.h +++ b/src/ast.h @@ -88,7 +88,8 @@ namespace internal { V(ThisFunction) \ V(SuperPropertyReference) \ V(SuperCallReference) \ - V(CaseClause) + V(CaseClause) \ + V(EmptyParentheses) #define AST_NODE_LIST(V) \ DECLARATION_NODE_LIST(V) \ @@ -2848,6 +2849,17 @@ class SuperCallReference final : public Expression { }; +// This class is produced when parsing the () in arrow functions without any +// arguments and is not actually a valid expression. +class EmptyParentheses final : public Expression { + public: + DECLARE_NODE_TYPE(EmptyParentheses) + + private: + EmptyParentheses(Zone* zone, int pos) : Expression(zone, pos) {} +}; + + #undef DECLARE_NODE_TYPE @@ -3633,6 +3645,10 @@ class AstNodeFactory final BASE_EMBEDDED { this_function_var, pos); } + EmptyParentheses* NewEmptyParentheses(int pos) { + return new (zone_) EmptyParentheses(zone_, pos); + } + private: Zone* zone_; AstValueFactory* ast_value_factory_; diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc index 11dc456e3..044b904bd 100644 --- a/src/compiler/ast-graph-builder.cc +++ b/src/compiler/ast-graph-builder.cc @@ -2867,6 +2867,12 @@ void AstGraphBuilder::VisitSpread(Spread* expr) { } +void AstGraphBuilder::VisitEmptyParentheses(EmptyParentheses* expr) { + // Handled entirely by the parser itself. + UNREACHABLE(); +} + + void AstGraphBuilder::VisitThisFunction(ThisFunction* expr) { Node* value = GetFunctionClosure(); ast_context()->ProduceValue(value); diff --git a/src/compiler/ast-loop-assignment-analyzer.cc b/src/compiler/ast-loop-assignment-analyzer.cc index 4d1d41e12..b73cbb35f 100644 --- a/src/compiler/ast-loop-assignment-analyzer.cc +++ b/src/compiler/ast-loop-assignment-analyzer.cc @@ -193,6 +193,9 @@ void ALAA::VisitCompareOperation(CompareOperation* e) { void ALAA::VisitSpread(Spread* e) { Visit(e->expression()); } +void ALAA::VisitEmptyParentheses(EmptyParentheses* e) { UNREACHABLE(); } + + void ALAA::VisitCaseClause(CaseClause* cc) { if (!cc->is_default()) Visit(cc->label()); VisitStatements(cc->statements()); diff --git a/src/full-codegen/full-codegen.cc b/src/full-codegen/full-codegen.cc index 51fa09e56..5b8d9c559 100644 --- a/src/full-codegen/full-codegen.cc +++ b/src/full-codegen/full-codegen.cc @@ -1400,6 +1400,11 @@ void FullCodeGenerator::ExitTryBlock(int handler_index) { void FullCodeGenerator::VisitSpread(Spread* expr) { UNREACHABLE(); } +void FullCodeGenerator::VisitEmptyParentheses(EmptyParentheses* expr) { + UNREACHABLE(); +} + + FullCodeGenerator::NestedStatement* FullCodeGenerator::TryFinally::Exit( int* stack_depth, int* context_length) { // The macros used here must preserve the result register. diff --git a/src/hydrogen.cc b/src/hydrogen.cc index a51cfc0fe..317164283 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -11525,6 +11525,11 @@ void HOptimizedGraphBuilder::HandleLiteralCompareNil(CompareOperation* expr, void HOptimizedGraphBuilder::VisitSpread(Spread* expr) { UNREACHABLE(); } +void HOptimizedGraphBuilder::VisitEmptyParentheses(EmptyParentheses* expr) { + UNREACHABLE(); +} + + HInstruction* HOptimizedGraphBuilder::BuildThisFunction() { // If we share optimized code between different closures, the // this-function is not a constant, except inside an inlined body. diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc index 9cce681ad..4fa1d8218 100644 --- a/src/interpreter/bytecode-generator.cc +++ b/src/interpreter/bytecode-generator.cc @@ -335,6 +335,11 @@ void BytecodeGenerator::VisitCompareOperation(CompareOperation* node) { void BytecodeGenerator::VisitSpread(Spread* node) { UNIMPLEMENTED(); } +void BytecodeGenerator::VisitEmptyParentheses(EmptyParentheses* node) { + UNIMPLEMENTED(); +} + + void BytecodeGenerator::VisitThisFunction(ThisFunction* node) { UNIMPLEMENTED(); } diff --git a/src/messages.h b/src/messages.h index 14f1dcf3e..f00ff8bc8 100644 --- a/src/messages.h +++ b/src/messages.h @@ -308,8 +308,6 @@ class CallSite { T(MalformedArrowFunParamList, "Malformed arrow function parameter list") \ T(MalformedRegExp, "Invalid regular expression: /%/: %") \ T(MalformedRegExpFlags, "Invalid regular expression flags") \ - T(MissingArrow, \ - "Expected () to start arrow function, but got '%' instead of '=>'") \ T(ModuleExportUndefined, "Export '%' is not defined in module") \ T(MultipleDefaultsInSwitch, \ "More than one default clause in switch statement") \ diff --git a/src/parser.cc b/src/parser.cc index 96b5d9466..72967d6df 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -3934,6 +3934,8 @@ void ParserTraits::ParseArrowFunctionFormalParameterList( ParserFormalParameters* parameters, Expression* expr, const Scanner::Location& params_loc, Scanner::Location* duplicate_loc, bool* ok) { + if (expr->IsEmptyParentheses()) return; + ParseArrowFunctionFormalParameters(parameters, expr, params_loc, duplicate_loc, ok); if (!*ok) return; diff --git a/src/pattern-rewriter.cc b/src/pattern-rewriter.cc index f3ecf7f15..431b2acf8 100644 --- a/src/pattern-rewriter.cc +++ b/src/pattern-rewriter.cc @@ -368,6 +368,11 @@ void Parser::PatternRewriter::VisitSpread(Spread* node) { } +void Parser::PatternRewriter::VisitEmptyParentheses(EmptyParentheses* node) { + UNREACHABLE(); +} + + // =============== UNREACHABLE ============================= void Parser::PatternRewriter::Visit(AstNode* node) { UNREACHABLE(); } diff --git a/src/preparser.h b/src/preparser.h index f8f20e530..9ecf77a9e 100644 --- a/src/preparser.h +++ b/src/preparser.h @@ -1308,6 +1308,10 @@ class PreParserFactory { return PreParserExpression::Spread(expression); } + PreParserExpression NewEmptyParentheses(int pos) { + return PreParserExpression::Default(); + } + // Return the object itself as AstVisitor and implement the needed // dummy method right in this class. PreParserFactory* visitor() { return this; } @@ -2262,38 +2266,35 @@ ParserBase::ParsePrimaryExpression(ExpressionClassifier* classifier, } BindingPatternUnexpectedToken(classifier); Consume(Token::LPAREN); - if (allow_harmony_arrow_functions() && Check(Token::RPAREN)) { - // As a primary expression, the only thing that can follow "()" is "=>". + if (Check(Token::RPAREN)) { + // ()=>x. The continuation that looks for the => is in + // ParseAssignmentExpression. + classifier->RecordExpressionError(scanner()->location(), + MessageTemplate::kUnexpectedToken, + Token::String(Token::RPAREN)); classifier->RecordBindingPatternError(scanner()->location(), MessageTemplate::kUnexpectedToken, Token::String(Token::RPAREN)); - // Give a good error to the user who might have typed e.g. "return();". - if (peek() != Token::ARROW) { - ReportUnexpectedTokenAt(scanner_->peek_location(), peek(), - MessageTemplate::kMissingArrow); + result = factory()->NewEmptyParentheses(beg_pos); + } else if (allow_harmony_rest_parameters() && Check(Token::ELLIPSIS)) { + // (...x)=>x. The continuation that looks for the => is in + // ParseAssignmentExpression. + int ellipsis_pos = scanner()->location().beg_pos; + classifier->RecordExpressionError(scanner()->location(), + MessageTemplate::kUnexpectedToken, + Token::String(Token::ELLIPSIS)); + Scanner::Location expr_loc = scanner()->peek_location(); + Token::Value tok = peek(); + result = this->ParseAssignmentExpression(true, classifier, CHECK_OK); + // Patterns are not allowed as rest parameters. There is no way we can + // succeed so go ahead and use the convenient ReportUnexpectedToken + // interface. + if (!Traits::IsIdentifier(result)) { + ReportUnexpectedTokenAt(expr_loc, tok); *ok = false; return this->EmptyExpression(); } - Scope* scope = - this->NewScope(scope_, ARROW_SCOPE, FunctionKind::kArrowFunction); - FormalParametersT parameters(scope); - scope->set_start_position(beg_pos); - ExpressionClassifier args_classifier; - result = this->ParseArrowFunctionLiteral(parameters, args_classifier, - CHECK_OK); - } else if (allow_harmony_arrow_functions() && - allow_harmony_rest_parameters() && Check(Token::ELLIPSIS)) { - // (...x) => y - Scope* scope = - this->NewScope(scope_, ARROW_SCOPE, FunctionKind::kArrowFunction); - FormalParametersT formals(scope); - scope->set_start_position(beg_pos); - ExpressionClassifier formals_classifier; - formals.has_rest = true; - this->ParseFormalParameter(&formals, &formals_classifier, CHECK_OK); - Traits::DeclareFormalParameter( - formals.scope, formals.at(0), formals.is_simple, - &formals_classifier); + result = factory()->NewSpread(result, ellipsis_pos); if (peek() == Token::COMMA) { ReportMessageAt(scanner()->peek_location(), MessageTemplate::kParamAfterRest); @@ -2301,8 +2302,6 @@ ParserBase::ParsePrimaryExpression(ExpressionClassifier* classifier, return this->EmptyExpression(); } Expect(Token::RPAREN, CHECK_OK); - result = this->ParseArrowFunctionLiteral(formals, formals_classifier, - CHECK_OK); } else { // Heuristically try to detect immediately called functions before // seeing the call parentheses. diff --git a/src/prettyprinter.cc b/src/prettyprinter.cc index e71489fa7..89af59377 100644 --- a/src/prettyprinter.cc +++ b/src/prettyprinter.cc @@ -360,6 +360,11 @@ void CallPrinter::VisitSpread(Spread* node) { } +void CallPrinter::VisitEmptyParentheses(EmptyParentheses* node) { + UNREACHABLE(); +} + + void CallPrinter::VisitThisFunction(ThisFunction* node) {} @@ -845,6 +850,11 @@ void PrettyPrinter::VisitSpread(Spread* node) { } +void PrettyPrinter::VisitEmptyParentheses(EmptyParentheses* node) { + Print(""); +} + + void PrettyPrinter::VisitThisFunction(ThisFunction* node) { Print(""); } @@ -1565,6 +1575,11 @@ void AstPrinter::VisitSpread(Spread* node) { } +void AstPrinter::VisitEmptyParentheses(EmptyParentheses* node) { + IndentedScope indent(this, "()"); +} + + void AstPrinter::VisitThisFunction(ThisFunction* node) { IndentedScope indent(this, "THIS-FUNCTION"); } diff --git a/src/typing.cc b/src/typing.cc index 204ace6c9..e1991f25c 100644 --- a/src/typing.cc +++ b/src/typing.cc @@ -753,6 +753,11 @@ void AstTyper::VisitCompareOperation(CompareOperation* expr) { void AstTyper::VisitSpread(Spread* expr) { RECURSE(Visit(expr->expression())); } +void AstTyper::VisitEmptyParentheses(EmptyParentheses* expr) { + UNREACHABLE(); +} + + void AstTyper::VisitThisFunction(ThisFunction* expr) { } diff --git a/test/message/arrow-missing.out b/test/message/arrow-missing.out index f042a20fa..bad6157a0 100644 --- a/test/message/arrow-missing.out +++ b/test/message/arrow-missing.out @@ -1,4 +1,4 @@ -*%(basename)s:7: SyntaxError: Expected () to start arrow function, but got ';' instead of '=>' +*%(basename)s:7: SyntaxError: Unexpected token ) function foo() { return(); } - ^ -SyntaxError: Expected () to start arrow function, but got ';' instead of '=>' + ^ +SyntaxError: Unexpected token ) diff --git a/test/mjsunit/harmony/regress/regress-4211.js b/test/mjsunit/harmony/regress/regress-4211.js new file mode 100644 index 000000000..8affc7344 --- /dev/null +++ b/test/mjsunit/harmony/regress/regress-4211.js @@ -0,0 +1,12 @@ +// Copyright 2015 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +// +// Flags: --harmony-arrow-functions --harmony-rest-parameters + +assertThrows("()=>{}()", SyntaxError); +assertThrows("x=>{}()", SyntaxError); +assertThrows("(...x)=>{}()", SyntaxError); +assertThrows("(x)=>{}()", SyntaxError); +assertThrows("(x,y)=>{}()", SyntaxError); +assertThrows("(x,y,...z)=>{}()", SyntaxError); -- 2.34.1