Revert of Parse arrow functions at proper precedence level (patchset #2 id:60001...
authoryangguo <yangguo@chromium.org>
Mon, 24 Aug 2015 06:56:56 +0000 (23:56 -0700)
committerCommit bot <commit-bot@chromium.org>
Mon, 24 Aug 2015 06:57:12 +0000 (06:57 +0000)
Reason for revert:
Breaks layout test. Please change test expectation on blink first.

--- /mnt/data/b/build/slave/V8-Blink_Linux_64/build/layout-test-results/inspector/sources/debugger-pause/debugger-pause-in-internal-expected.txt
+++ /mnt/data/b/build/slave/V8-Blink_Linux_64/build/layout-test-results/inspector/sources/debugger-pause/debugger-pause-in-internal-actual.txt
@@ -1,4 +1,4 @@
-CONSOLE ERROR: line 9: Uncaught SyntaxError: Expected () to start arrow function, but got '}' instead of '=>'
+CONSOLE ERROR: line 9: Uncaught SyntaxError: Unexpected token )
 Tests that pause on exception in internal script does not crash.

 Script source was shown.

Original issue's description:
> Parse arrow functions at proper precedence level
>
> BUG=v8:4211
> LOG=Y
> R=rossberg@chromium.org
>
> Committed: https://crrev.com/9271b0ccf9ddb217deb1f0b9ef9b59b64dc40214
> Cr-Commit-Position: refs/heads/master@{#30298}

TBR=rossberg@chromium.org,mstarzinger@chromium.org,fennyfanny655@gmail.com,machenbach@chromium.org,wingo@igalia.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:4211

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

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

16 files changed:
src/ast-literal-reindexer.cc
src/ast-numbering.cc
src/ast.h
src/compiler/ast-graph-builder.cc
src/compiler/ast-loop-assignment-analyzer.cc
src/full-codegen/full-codegen.cc
src/hydrogen.cc
src/interpreter/bytecode-generator.cc
src/messages.h
src/parser.cc
src/pattern-rewriter.cc
src/preparser.h
src/prettyprinter.cc
src/typing.cc
test/message/arrow-missing.out
test/mjsunit/harmony/regress/regress-4211.js [deleted file]

index d9a6ed1..860a396 100644 (file)
@@ -174,9 +174,6 @@ void AstLiteralReindexer::VisitSpread(Spread* node) {
 }
 
 
-void AstLiteralReindexer::VisitEmptyParentheses(EmptyParentheses* node) {}
-
-
 void AstLiteralReindexer::VisitForInStatement(ForInStatement* node) {
   Visit(node->each());
   Visit(node->enumerable());
index 7338cc6..dc0528c 100644 (file)
@@ -371,11 +371,6 @@ void AstNumberingVisitor::VisitSpread(Spread* node) {
 }
 
 
-void AstNumberingVisitor::VisitEmptyParentheses(EmptyParentheses* node) {
-  UNREACHABLE();
-}
-
-
 void AstNumberingVisitor::VisitForInStatement(ForInStatement* node) {
   IncrementNodeCount();
   DisableSelfOptimization();
index 6e9390f..1366041 100644 (file)
--- a/src/ast.h
+++ b/src/ast.h
@@ -88,8 +88,7 @@ namespace internal {
   V(ThisFunction)               \
   V(SuperPropertyReference)     \
   V(SuperCallReference)         \
-  V(CaseClause)                 \
-  V(EmptyParentheses)
+  V(CaseClause)
 
 #define AST_NODE_LIST(V)                        \
   DECLARATION_NODE_LIST(V)                      \
@@ -2849,17 +2848,6 @@ 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
 
 
@@ -3645,10 +3633,6 @@ 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_;
index 044b904..11dc456 100644 (file)
@@ -2867,12 +2867,6 @@ 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);
index b73cbb3..4d1d41e 100644 (file)
@@ -193,9 +193,6 @@ 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());
index 1017b31..8b71eb7 100644 (file)
@@ -1399,11 +1399,6 @@ 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.
index 782baa0..44f08b9 100644 (file)
@@ -11525,11 +11525,6 @@ 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.
index 4fa1d82..9cce681 100644 (file)
@@ -335,11 +335,6 @@ void BytecodeGenerator::VisitCompareOperation(CompareOperation* node) {
 void BytecodeGenerator::VisitSpread(Spread* node) { UNIMPLEMENTED(); }
 
 
-void BytecodeGenerator::VisitEmptyParentheses(EmptyParentheses* node) {
-  UNIMPLEMENTED();
-}
-
-
 void BytecodeGenerator::VisitThisFunction(ThisFunction* node) {
   UNIMPLEMENTED();
 }
index d293b74..c8b9497 100644 (file)
@@ -308,6 +308,8 @@ 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")                        \
index 1c9ee16..d4da7ce 100644 (file)
@@ -3934,8 +3934,6 @@ 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;
index 431b2ac..f3ecf7f 100644 (file)
@@ -368,11 +368,6 @@ void Parser::PatternRewriter::VisitSpread(Spread* node) {
 }
 
 
-void Parser::PatternRewriter::VisitEmptyParentheses(EmptyParentheses* node) {
-  UNREACHABLE();
-}
-
-
 // =============== UNREACHABLE =============================
 
 void Parser::PatternRewriter::Visit(AstNode* node) { UNREACHABLE(); }
index 9ecf77a..f8f20e5 100644 (file)
@@ -1308,10 +1308,6 @@ 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; }
@@ -2266,35 +2262,38 @@ ParserBase<Traits>::ParsePrimaryExpression(ExpressionClassifier* classifier,
       }
       BindingPatternUnexpectedToken(classifier);
       Consume(Token::LPAREN);
-      if (Check(Token::RPAREN)) {
-        // ()=>x.  The continuation that looks for the => is in
-        // ParseAssignmentExpression.
-        classifier->RecordExpressionError(scanner()->location(),
-                                          MessageTemplate::kUnexpectedToken,
-                                          Token::String(Token::RPAREN));
+      if (allow_harmony_arrow_functions() && Check(Token::RPAREN)) {
+        // As a primary expression, the only thing that can follow "()" is "=>".
         classifier->RecordBindingPatternError(scanner()->location(),
                                               MessageTemplate::kUnexpectedToken,
                                               Token::String(Token::RPAREN));
-        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);
+        // 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);
           *ok = false;
           return this->EmptyExpression();
         }
-        result = factory()->NewSpread(result, ellipsis_pos);
+        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);
         if (peek() == Token::COMMA) {
           ReportMessageAt(scanner()->peek_location(),
                           MessageTemplate::kParamAfterRest);
@@ -2302,6 +2301,8 @@ 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.
index 89af593..e71489f 100644 (file)
@@ -360,11 +360,6 @@ void CallPrinter::VisitSpread(Spread* node) {
 }
 
 
-void CallPrinter::VisitEmptyParentheses(EmptyParentheses* node) {
-  UNREACHABLE();
-}
-
-
 void CallPrinter::VisitThisFunction(ThisFunction* node) {}
 
 
@@ -850,11 +845,6 @@ void PrettyPrinter::VisitSpread(Spread* node) {
 }
 
 
-void PrettyPrinter::VisitEmptyParentheses(EmptyParentheses* node) {
-  Print("<empty-parentheses>");
-}
-
-
 void PrettyPrinter::VisitThisFunction(ThisFunction* node) {
   Print("<this-function>");
 }
@@ -1575,11 +1565,6 @@ void AstPrinter::VisitSpread(Spread* node) {
 }
 
 
-void AstPrinter::VisitEmptyParentheses(EmptyParentheses* node) {
-  IndentedScope indent(this, "()");
-}
-
-
 void AstPrinter::VisitThisFunction(ThisFunction* node) {
   IndentedScope indent(this, "THIS-FUNCTION");
 }
index 1cb7577..22e4b21 100644 (file)
@@ -742,11 +742,6 @@ 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) {
 }
 
index bad6157..f042a20 100644 (file)
@@ -1,4 +1,4 @@
-*%(basename)s:7: SyntaxError: Unexpected token )
+*%(basename)s:7: SyntaxError: Expected () to start arrow function, but got ';' instead of '=>'
 function foo() { return(); }
-                        ^
-SyntaxError: Unexpected token )
+                         ^
+SyntaxError: Expected () to start arrow function, but got ';' instead of '=>'
diff --git a/test/mjsunit/harmony/regress/regress-4211.js b/test/mjsunit/harmony/regress/regress-4211.js
deleted file mode 100644 (file)
index 8affc73..0000000
+++ /dev/null
@@ -1,12 +0,0 @@
-// 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);