Parse arrow functions at proper precedence level

src/preparser.h

Index: src/preparser.h
diff --git a/src/preparser.h b/src/preparser.h
index f8f20e530aa18d2f7e4c8c1d892470d33dc662c6..9ecf77a9ee5c30a10580ec935ed0b0dc276a590f 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<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)) {

[rossberg, 2015/08/25 12:55:56]

I wonder, does rest actually need special treatment here? Can't it simply be
parsed as spread in the else case below, and later be rewritten?

[wingo, 2015/08/25 13:18:18]

I think it does need special treatment.  All the other places that parse spread
(array literals, call arguments, comma expressions) parse spread specially.  We
could of course parse it anywhere as a prefix unary op, marking the production
invalid as an expression, but that is a riskier change if we are considering
uplifting this to M46.  WDYT?

[rossberg, 2015/08/25 13:26:42]

SGTM. Let's leave that as a clean-up for a separate CL.

+        // (...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.