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}

diff --git a/src/ast-literal-reindexer.cc b/src/ast-literal-reindexer.cc
index 860a396..d9a6ed1 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 dc0528c..7338cc6 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 1366041..6e9390f 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 11dc456..044b904 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 4d1d41e..b73cbb3 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 51fa09e..5b8d9c5 100644 (file)
--- 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 a51cfc0..3171642 100644 (file)
--- 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 9cce681..4fa1d82 100644 (file)
--- 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 14f1dcf..f00ff8b 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 96b5d94..72967d6 100644 (file)
--- 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 f3ecf7f..431b2ac 100644 (file)
--- 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 f8f20e5..9ecf77a 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 e71489f..89af593 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("<empty-parentheses>");
+}
+
+
 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 204ace6..e1991f2 100644 (file)
--- 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 f042a20..bad6157 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);