Parse arrow functions at proper precedence level

BUG=v8:4211
LOG=Y
R=rossberg@chromium.org

Review URL: https://codereview.chromium.org/1315823002

Cr-Commit-Position: refs/heads/master@{#30373}

diff --git a/src/ast-expression-visitor.cc b/src/ast-expression-visitor.cc
index a643b83d46bb455ec29074385400488ed9396c17..5a0912576155dd64cdde5556574a8281a1889a7a 100644 (file)
--- a/src/ast-expression-visitor.cc
+++ b/src/ast-expression-visitor.cc
@@ -337,6 +337,9 @@ void AstExpressionVisitor::VisitClassLiteral(ClassLiteral* expr) {}
 void AstExpressionVisitor::VisitSpread(Spread* expr) {}
 
 
+void AstExpressionVisitor::VisitEmptyParentheses(EmptyParentheses* expr) {}
+
+
 void AstExpressionVisitor::VisitSuperPropertyReference(
     SuperPropertyReference* expr) {}
 

diff --git a/src/ast-literal-reindexer.cc b/src/ast-literal-reindexer.cc
index 860a3961f05b66a292ea1dac421ad261fad63549..d9a6ed1c41d141ce3c92d50c957120bf16ad82e0 100644 (file)
--- 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 dc0528caa0cff687feec0ddb359a1704e7171a53..7338cc6724219678852d5a90c40155b5a62e53ad 100644 (file)
--- 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 1366041387215bc4c018c4b1650d39ff5f6549c4..6e9390f6e3f62ebfe9d168b25993ffb0fef3a47c 100644 (file)
--- 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 e6acf4d157e35986d402e5f37bca597594946ae6..c31bdb120b924cb85cc85420c0ad76bc27f93efc 100644 (file)
--- 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 4d1d41e12229320a2998bf3ba6f60da2e94a7e31..b73cbb35f108ff49bb9e0b7d25464a2088d63ee6 100644 (file)
--- 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 8b71eb7f9d08ad398695d9f8c74893a887faa320..1017b3185ba11845436f9213b97b98e770544cbc 100644 (file)
--- a/src/full-codegen/full-codegen.cc
+++ b/src/full-codegen/full-codegen.cc
@@ -1399,6 +1399,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 65906573d8ad0505de1c1ce8de30bf5682f12518..3787fdfac648be85fc3cbc70c65ed12d58ca957c 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -11533,6 +11533,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 9cce681ad4ac79119ec701907df4813d156b3d60..50f27bdd227d0a01b22c56e42553cf11b2346a16 100644 (file)
--- a/src/interpreter/bytecode-generator.cc
+++ b/src/interpreter/bytecode-generator.cc
@@ -332,7 +332,12 @@ void BytecodeGenerator::VisitCompareOperation(CompareOperation* node) {
 }
 
 
-void BytecodeGenerator::VisitSpread(Spread* node) { UNIMPLEMENTED(); }
+void BytecodeGenerator::VisitSpread(Spread* node) { UNREACHABLE(); }
+
+
+void BytecodeGenerator::VisitEmptyParentheses(EmptyParentheses* node) {
+  UNREACHABLE();
+}
 
 
 void BytecodeGenerator::VisitThisFunction(ThisFunction* node) {

diff --git a/src/messages.h b/src/messages.h
index c8b9497fb939070724a5bf868cd69d8ee7733f9d..d293b74182f46b56178d5ad366c052112355aaa3 100644 (file)
--- 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 f93f428cfce6e98d6ae9c64fa3bc2af2db840371..5faca42f3b4e8c774ce9f4a3b39238e4e8bc757f 100644 (file)
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -3969,6 +3969,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 f3ecf7f1542dd658f3f2bbfc789f7d19381b4efc..489530d47750bafe3a520d6faf37ab494163e2ea 100644 (file)
--- a/src/pattern-rewriter.cc
+++ b/src/pattern-rewriter.cc
@@ -364,7 +364,12 @@ void Parser::PatternRewriter::VisitAssignment(Assignment* node) {
 
 
 void Parser::PatternRewriter::VisitSpread(Spread* node) {
-  // TODO(dslomov): implement.
+  UNREACHABLE();
+}
+
+
+void Parser::PatternRewriter::VisitEmptyParentheses(EmptyParentheses* node) {
+  UNREACHABLE();
 }
 
 

diff --git a/src/preparser.h b/src/preparser.h
index f8f20e530aa18d2f7e4c8c1d892470d33dc662c6..9ecf77a9ee5c30a10580ec935ed0b0dc276a590f 100644 (file)
--- 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<Traits>::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<Traits>::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 e71489fa7f2c401111468f7de94d55bf24075a4a..bf28cc6378749a94217a0949bd04da43a01fc11b 100644 (file)
--- 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("<this-function>");
 }
@@ -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 22e4b219d9d05ffdacfba20d3278f8b852326465..1cb7577775622c59880638d2d2bf5067cf5c6d7f 100644 (file)
--- a/src/typing.cc
+++ b/src/typing.cc
@@ -742,6 +742,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 f042a20fad718057598ef7647090341ada73b5e8..bad6157a0a1343bdbba61440d90bc5d1f1de6300 100644 (file)
--- 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 (file)
index 0000000..8affc73
--- /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);