Implement handling of arrow functions in the parser

src/parser.cc

Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index c535704f02461b667d289c68caf7da3b113abfc6..f995bdda512245c8ae2178a05a4fbafdb53cd02a 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -757,8 +757,9 @@ Handle<String> ParserTraits::NextLiteralString(Scanner* scanner,
 
 Expression* ParserTraits::ThisExpression(
     Scope* scope,
-    AstNodeFactory<AstConstructionVisitor>* factory) {
-  return factory->NewVariableProxy(scope->receiver());
+    AstNodeFactory<AstConstructionVisitor>* factory,
+    int pos) {
+  return factory->NewVariableProxy(scope->receiver(), pos);
 }
 
 
@@ -859,6 +860,7 @@ Parser::Parser(CompilationInfo* info)
   set_allow_lazy(false);  // Must be explicitly enabled.
   set_allow_generators(FLAG_harmony_generators);
   set_allow_for_of(FLAG_harmony_iteration);
+  set_allow_arrow_functions(FLAG_harmony_arrow_functions);
   set_allow_harmony_numeric_literals(FLAG_harmony_numeric_literals);
 }
 
@@ -994,6 +996,7 @@ FunctionLiteral* Parser::DoParseProgram(CompilationInfo* info,
           FunctionLiteral::kGlobalOrEval,
           FunctionLiteral::kNotParenthesized,
           FunctionLiteral::kNotGenerator,
+          FunctionLiteral::kNotArrow,
           0);
       result->set_ast_properties(factory()->visitor()->ast_properties());
       result->set_slot_processor(factory()->visitor()->slot_processor());
@@ -1079,14 +1082,22 @@ FunctionLiteral* Parser::ParseLazy(Utf16CharacterStream* source) {
               ? FunctionLiteral::ANONYMOUS_EXPRESSION
               : FunctionLiteral::NAMED_EXPRESSION)
         : FunctionLiteral::DECLARATION;
+    bool is_generator = shared_info->is_generator();
     bool ok = true;
-    result = ParseFunctionLiteral(name,
-                                  Scanner::Location::invalid(),
-                                  false,  // Strict mode name already checked.
-                                  shared_info->is_generator(),
-                                  RelocInfo::kNoPosition,
-                                  function_type,
-                                  &ok);
+
+    if (shared_info->is_arrow()) {
+      ASSERT(!is_generator);
+      result = reinterpret_cast<FunctionLiteral*>(
+          ParseArrowFunctionLiteral(shared_info->start_position(), NULL, &ok));
+    } else {
+      result = ParseFunctionLiteral(name,
+                                    Scanner::Location::invalid(),
+                                    false,  // Strict mode name already checked.
+                                    is_generator,
+                                    RelocInfo::kNoPosition,
+                                    function_type,
+                                    &ok);
+    }
     // Make sure the results agree.
     ASSERT(ok == (result != NULL));
   }
@@ -3188,6 +3199,59 @@ Handle<FixedArray> CompileTimeValue::GetElements(Handle<FixedArray> value) {
 }
 
 
+Vector<VariableProxy*> ParserTraits::ParameterListFromExpression(

[marja, 2014/04/24 08:08:06]

It looks like this doesn't check that the expression is parenthesized, so this
would accept foo, bar => { baz }. You should add tests for such cases in the
style of test-parsing.cc.

[aperez, 2014/04/24 12:38:37]

The example you mention would be a comma-expression with two items, and it
is a valid expression, equivalent to:

  foo, (bar  => { baz })
  (foo, bar => { baz })
  (foo, (bar => { baz }))

So the case you mention is valid syntax, and it is already handled by the
grammar because comma has more precedence than expressions, and an arrow
function is an expression (the whole arrrow function, including the
parameter list).

There is no need to check for those cases here, because the input up
to the comma has already been parsed as a comma-expression when the
ParseArrowFunction() function is called.

[marja, 2014/04/29 09:38:44]

Hmm, okay. It would be good to add test cases to make sure these expressions are
interpreted correctly.

+    Expression* expression, bool* ok) {
+
+  // Parsing the parameter list of an arrow function does not have parameters
+  // (as in: () => ...) returns NULL, so expression being NULL is not an error,
+  // but an valid empty parameter list.
+  if (expression == NULL)
+    return Vector<VariableProxy*>::empty();
+
+  const char* error_token = Token::String(Token::THIS);
+  int error_pos = RelocInfo::kNoPosition;
+
+  Collector<VariableProxy*> collector;
+  while (expression->IsBinaryOperation()) {
+    BinaryOperation* binop = expression->AsBinaryOperation();
+
+    if (binop->op() != Token::COMMA) {
+      error_token = Token::String(expression->AsBinaryOperation()->op());
+      goto invalid_token;

[marja, 2014/04/24 08:08:06]

Afaics, it would be possible to write this in a goto-less way; just set some
error condition and break from the while loop, after the loop, check the error
conditions.

[aperez, 2014/04/24 12:38:37]

Sure thing, I will do that. The function will not be as terse, though.

+    }
+
+    expression = binop->right();
+    if (!expression->IsVariableProxy())
+      goto not_variable_proxy;
+    if (expression->AsVariableProxy()->is_this())
+      goto invalid_token;
+
+    collector.Add(expression->AsVariableProxy());
+    expression = binop->left();
+  }
+
+  if (!expression->IsVariableProxy())
+    goto not_variable_proxy;
+  if (expression->AsVariableProxy()->is_this())
+    goto invalid_token;
+
+  collector.Add(expression->AsVariableProxy());
+  return collector.ToVector();
+
+ not_variable_proxy:
+  error_token = "";
+  // fall-through
+ invalid_token:
+  error_pos = expression->position();
+  ParserTraits::ReportMessageAt(
+      Scanner::Location(error_pos, error_pos + strlen(error_token)),
+      "unexpected_token",
+      Vector<const char*>(&error_token, *error_token ? 1 : 0));
+  *ok = false;
+  return Vector<VariableProxy*>::empty();
+}
+
+
 FunctionLiteral* Parser::ParseFunctionLiteral(
     Handle<String> function_name,
     Scanner::Location function_name_location,
@@ -3453,6 +3517,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
                                     FunctionLiteral::kIsFunction,
                                     parenthesized,
                                     generator,
+                                    FunctionLiteral::kNotArrow,
                                     pos);
   function_literal->set_function_token_position(function_token_pos);
   function_literal->set_ast_properties(&ast_properties);
@@ -3620,6 +3685,7 @@ PreParser::PreParseResult Parser::ParseLazyFunctionBodyWithPreParser(
     reusable_preparser_->set_allow_lazy(true);
     reusable_preparser_->set_allow_generators(allow_generators());
     reusable_preparser_->set_allow_for_of(allow_for_of());
+    reusable_preparser_->set_allow_arrow_functions(allow_arrow_functions());
     reusable_preparser_->set_allow_harmony_numeric_literals(
         allow_harmony_numeric_literals());
   }