Implement handling of arrow functions in the parser

test/cctest/test-parsing.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 176 matching lines @@
   const char* data() const { return data_; }
   size_t length() const { return length_; }
 
  private:
   const char* data_;
   size_t length_;
 };
 
 
 TEST(UsingCachedData) {
+  i::FLAG_harmony_arrow_functions = true;
   v8::Isolate* isolate = CcTest::isolate();
   v8::HandleScope handles(isolate);
   v8::Local<v8::Context> context = v8::Context::New(isolate);
   v8::Context::Scope context_scope(context);
   int marker;
   CcTest::i_isolate()->stack_guard()->SetStackLimit(
       reinterpret_cast<uintptr_t>(&marker) - 128 * 1024);
 
   // Source containing functions that might be lazily compiled  and all types
   // of symbols (string, propertyName, regexp).
@@ skipping 28 matching lines @@
 }
 
 
 TEST(PreparseFunctionDataIsUsed) {
   // This tests that we actually do use the function data generated by the
   // preparser.
 
   // Make preparsing work for short scripts.
   i::FLAG_min_preparse_length = 0;
 
+  i::FLAG_harmony_arrow_functions = true;
   v8::Isolate* isolate = CcTest::isolate();
   v8::HandleScope handles(isolate);
   v8::Local<v8::Context> context = v8::Context::New(isolate);
   v8::Context::Scope context_scope(context);
   int marker;
   CcTest::i_isolate()->stack_guard()->SetStackLimit(
       reinterpret_cast<uintptr_t>(&marker) - 128 * 1024);
 
-  const char* good_code =
-      "function this_is_lazy() { var a; } function foo() { return 25; } foo();";
+  const char* good_code[] = {
+    "function this_is_lazy() { var a; } function foo() { return 25; } foo();",
+    NULL

[marja, 2014/06/17 11:47:38]

What, why? What are the changes in this test?

+  };
 
   // Insert a syntax error inside the lazy function.
-  const char* bad_code =
-      "function this_is_lazy() { if (   } function foo() { return 25; } foo();";
+  const char* bad_code[] = {
+    "function this_is_lazy() { if (   } function foo() { return 25; } foo();",
+    NULL
+  };
 
-  v8::ScriptCompiler::Source good_source(v8_str(good_code));
-  v8::ScriptCompiler::Compile(isolate, &good_source,
-                              v8::ScriptCompiler::kProduceDataToCache);
+  for (int i = 0; good_code[i]; i++) {

[marja, 2014/06/17 11:47:38]

Looping over an array that has only 1 non-trivial element?

+    v8::ScriptCompiler::Source good_source(v8_str(good_code[i]));
+    v8::ScriptCompiler::Compile(isolate, &good_source,
+                                v8::ScriptCompiler::kProduceDataToCache);
 
-  const v8::ScriptCompiler::CachedData* cached_data =
-      good_source.GetCachedData();
-  CHECK(cached_data->data != NULL);
-  CHECK_GT(cached_data->length, 0);
+    const v8::ScriptCompiler::CachedData* cached_data =
+        good_source.GetCachedData();
+    CHECK(cached_data->data != NULL);
+    CHECK_GT(cached_data->length, 0);
 
-  // Now compile the erroneous code with the good preparse data. If the preparse
-  // data is used, the lazy function is skipped and it should compile fine.
-  v8::ScriptCompiler::Source bad_source(
-      v8_str(bad_code), new v8::ScriptCompiler::CachedData(
-                            cached_data->data, cached_data->length));
-  v8::Local<v8::Value> result =
-      v8::ScriptCompiler::Compile(isolate, &bad_source)->Run();
-  CHECK(result->IsInt32());
-  CHECK_EQ(25, result->Int32Value());
+    // Now compile the erroneous code with the good preparse data. If the
+    // preparse data is used, the lazy function is skipped and it should
+    // compile fine.
+    v8::ScriptCompiler::Source bad_source(
+        v8_str(bad_code[i]), new v8::ScriptCompiler::CachedData(
+                              cached_data->data, cached_data->length));
+    v8::Local<v8::Value> result =
+        v8::ScriptCompiler::Compile(isolate, &bad_source)->Run();
+    CHECK(result->IsInt32());
+    CHECK_EQ(25, result->Int32Value());
+  }
 }
 
 
 TEST(StandAlonePreParser) {
   v8::V8::Initialize();
 
   int marker;
   CcTest::i_isolate()->stack_guard()->SetStackLimit(
       reinterpret_cast<uintptr_t>(&marker) - 128 * 1024);
 
   const char* programs[] = {
       "{label: 42}",
       "var x = 42;",
       "function foo(x, y) { return x + y; }",
       "%ArgleBargle(glop);",
       "var x = new new Function('this.x = 42');",
+      "var f = (x, y) => x + y;",
       NULL
   };
 
   uintptr_t stack_limit = CcTest::i_isolate()->stack_guard()->real_climit();
   for (int i = 0; programs[i]; i++) {
     const char* program = programs[i];
     i::Utf8ToUtf16CharacterStream stream(
         reinterpret_cast<const i::byte*>(program),
         static_cast<unsigned>(strlen(program)));
     i::CompleteParserRecorder log;
     i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
     scanner.Initialize(&stream);
 
     i::PreParser preparser(&scanner, &log, stack_limit);
     preparser.set_allow_lazy(true);
     preparser.set_allow_natives_syntax(true);
+    preparser.set_allow_arrow_functions(true);
     i::PreParser::PreParseResult result = preparser.PreParseProgram();
     CHECK_EQ(i::PreParser::kPreParseSuccess, result);
     i::ScriptData data(log.ExtractData());
     CHECK(!data.has_error());
   }
 }
 
 
 TEST(StandAlonePreParserNoNatives) {
   v8::V8::Initialize();
@@ skipping 640 matching lines @@
       "  }", "\n"
       "  more;", i::BLOCK_SCOPE, i::STRICT },
     { "  start;\n"
       "  function fun", "(a,b) { infunction; }", " more;",
       i::FUNCTION_SCOPE, i::SLOPPY },
     { "  start;\n"
       "  function fun", "(a,b) {\n"
       "    infunction;\n"
       "  }", "\n"
       "  more;", i::FUNCTION_SCOPE, i::SLOPPY },
-    { "  (function fun", "(a,b) { infunction; }", ")();",
+    // TODO(aperez): Change to use i::ARROW_SCOPE when implemented
+    { "  start;\n", "(a,b) => a + b", "; more;",
+      i::FUNCTION_SCOPE, i::SLOPPY },
+    { "  start;\n", "(a,b) => { return a+b; }", "\nmore;",
+      i::FUNCTION_SCOPE, i::SLOPPY },
+    { "  start;\n"
+      "  (function fun", "(a,b) { infunction; }", ")();",
       i::FUNCTION_SCOPE, i::SLOPPY },
     { "  for ", "(let x = 1 ; x < 10; ++ x) { block; }", " more;",
       i::BLOCK_SCOPE, i::STRICT },
     { "  for ", "(let x = 1 ; x < 10; ++ x) { block; }", "; more;",
       i::BLOCK_SCOPE, i::STRICT },
     { "  for ", "(let x = 1 ; x < 10; ++ x) {\n"
       "    block;\n"
       "  }", "\n"
       "  more;", i::BLOCK_SCOPE, i::STRICT },
     { "  for ", "(let x = 1 ; x < 10; ++ x) statement;", " more;",
@@ skipping 134 matching lines @@
 
     // Parse program source.
     i::Handle<i::String> source = factory->NewStringFromUtf8(
         i::CStrVector(program.start())).ToHandleChecked();
     CHECK_EQ(source->length(), kProgramSize);
     i::Handle<i::Script> script = factory->NewScript(source);
     i::CompilationInfoWithZone info(script);
     i::Parser parser(&info);
     parser.set_allow_lazy(true);
     parser.set_allow_harmony_scoping(true);
+    parser.set_allow_arrow_functions(true);
     info.MarkAsGlobal();
     info.SetStrictMode(source_data[i].strict_mode);
     parser.Parse();
     CHECK(info.function() != NULL);
 
     // Check scope types and positions.
     i::Scope* scope = info.function()->scope();
     CHECK(scope->is_global_scope());
     CHECK_EQ(scope->start_position(), 0);
     CHECK_EQ(scope->end_position(), kProgramSize);
@@ skipping 57 matching lines @@
 void SetParserFlags(i::ParserBase<Traits>* parser,
                     i::EnumSet<ParserFlag> flags) {
   parser->set_allow_lazy(flags.Contains(kAllowLazy));
   parser->set_allow_natives_syntax(flags.Contains(kAllowNativesSyntax));
   parser->set_allow_harmony_scoping(flags.Contains(kAllowHarmonyScoping));
   parser->set_allow_modules(flags.Contains(kAllowModules));
   parser->set_allow_generators(flags.Contains(kAllowGenerators));
   parser->set_allow_for_of(flags.Contains(kAllowForOf));
   parser->set_allow_harmony_numeric_literals(
       flags.Contains(kAllowHarmonyNumericLiterals));
+  parser->set_allow_arrow_functions(i::FLAG_harmony_arrow_functions);

[marja, 2014/06/17 11:47:38]

We'd probably like to run each existing ParserSyncTest both with and without
arrow functions, so you should handle it like the flags above (add a flag for it
into "flags" here).

But I realize it's actually not as easy as it should be, because for some tests
we want to pin some flags (so that other flags vary but those are always true).

I can try to add the machinery for that (an "alwaysTrueFlags" array which
overrides the varying flags) and see how it looks.

 }
 
 
 void TestParserSyncWithFlags(i::Handle<i::String> source,
                              i::EnumSet<ParserFlag> flags,
                              ParserSyncTestResult result) {
   i::Isolate* isolate = CcTest::i_isolate();
   i::Factory* factory = isolate->factory();
 
   uintptr_t stack_limit = isolate->stack_guard()->real_climit();
@@ skipping 307 matching lines @@
     "var foo, eval;",
     "var foo, arguments;",
     "try { } catch (eval) { }",
     "try { } catch (arguments) { }",
     "function eval() { }",
     "function arguments() { }",
     "function foo(eval) { }",
     "function foo(arguments) { }",
     "function foo(bar, eval) { }",
     "function foo(bar, arguments) { }",
+    "(eval) => { }",
+    "(arguments) => { }",
+    "(foo, eval) => { }",
+    "(foo, arguments) => { }",
     "eval = 1;",
     "arguments = 1;",
     "var foo = eval = 1;",
     "var foo = arguments = 1;",
     "++eval;",
     "++arguments;",
     "eval++;",
     "arguments++;",
     NULL
   };
@@ skipping 472 matching lines @@
           program, test_cases[i].functions,
           data->function_count());
       CHECK(false);
     }
     delete data;
   }
 }
 
 
 TEST(FunctionDeclaresItselfStrict) {
+  i::FLAG_harmony_arrow_functions = true;
   // Tests that we produce the right kinds of errors when a function declares
   // itself strict (we cannot produce there errors as soon as we see the
   // offending identifiers, because we don't know at that point whether the
   // function is strict or not).
   const char* context_data[][2] = {
     {"function eval() {", "}"},
     {"function arguments() {", "}"},
     {"function yield() {", "}"},
     {"function interface() {", "}"},
     {"function foo(eval) {", "}"},
     {"function foo(arguments) {", "}"},
     {"function foo(yield) {", "}"},
     {"function foo(interface) {", "}"},
     {"function foo(bar, eval) {", "}"},
     {"function foo(bar, arguments) {", "}"},
     {"function foo(bar, yield) {", "}"},
     {"function foo(bar, interface) {", "}"},
     {"function foo(bar, bar) {", "}"},
+    {"eval => {", "}"},

[marja, 2014/06/17 11:47:38]

You could have a separate test case for these, and remove the i::FLAG above (so
that the main test case would be both with and without the arrow function flag).

+    {"arguments => {", "}"},
+    {"yield => {", "}"},
+    {"interface => {", "}"},
+    {"(eval) => {", "}"},
+    {"(arguments) => {", "}"},
+    {"(yield) => {", "}"},
+    {"(interface) => {", "}"},
+    {"(eval, bar) => {", "}"},
+    {"(bar, eval) => {", "}"},
+    {"(bar, arguments) => {", "}"},
+    {"(bar, yield) => {", "}"},
+    {"(bar, interface) => {", "}"},
+    {"(bar, bar) => {", "}"},
     { NULL, NULL }
   };
 
   const char* strict_statement_data[] = {
     "\"use strict\";",
     NULL
   };
 
   const char* non_strict_statement_data[] = {
     ";",
@@ skipping 494 matching lines @@
   v8::Local<v8::String> source =
       v8::String::NewFromTwoByte(isolate, two_byte_source);
   v8::Local<v8::Value> result = CompileRun(source);
   CHECK(result->IsString());
   v8::Local<v8::String> expected_name =
       v8::String::NewFromTwoByte(isolate, two_byte_name);
   CHECK(result->Equals(expected_name));
   i::DeleteArray(two_byte_source);
   i::DeleteArray(two_byte_name);
 }
+
+
+TEST(ErrorsArrowFunctions) {
+  // Tests that parser and preparser generate the same kind of errors
+  // on invalid arrow function syntax.
+  i::FLAG_harmony_arrow_functions = true;
+
+  const char* context_data[][2] = {
+    {"", ";"},
+    {"v = ", ";"},
+    {"bar ? (", ") : baz;"},
+    {"bar ? baz : (", ");"},
+    {"bar[", "];"},
+    {"bar, ", ";"},
+    {"", ", bar;"},
+    { NULL, NULL }
+  };
+
+  const char* statement_data[] = {
+    "=> 0",
+    "=>",
+    "() =>",
+    "=> {}",
+    ") => {}",
+    ", => {}",
+    "(,) => {}",
+    "return => {}",
+    "(a, if) => {}",
+    "(a * b) => {}",
+    "() => {'value': 42}",
+
+    // Check that the early return introduced in ParsePrimaryExpression
+    // does not accept stray closing parentheses.
+    ")",
+    ") => 0",
+    "foo[()]",
+    "()",
+
+    // Parameter lists with extra parens should be recognized as errors.
+    "(()) => 0",
+    "((x)) => 0",
+    "((x, y)) => 0",
+    "(x, (y)) => 0",
+    "((x, y, z)) => 0",
+    "(x, (y, z)) => 0",
+    "((x, y), z) => 0",
+

[marja, 2014/06/17 11:47:38]

Do any of the cases bump into the "is not a variable proxy" in
ParserTraits::ParameterListFromExpression()? I guess not? That would happen when
we expect a parameter name, but get some other kind of expression which is not a
parameter name, such as "13" or "foo ? bar : baz" or "foo + bar". Could you add
test cases for those?

+    NULL
+  };
+
+  RunParserSyncTest(context_data, statement_data, kError);
+}
+
+
+TEST(NoErrorsArrowFunctions) {
+  // Tests that parser and preparser accept valid arrow functions syntax.
+  i::FLAG_harmony_arrow_functions = true;
+
+  const char* context_data[][2] = {
+    {"", ";"},
+    {"bar ? (", ") : baz;"},
+    {"bar ? baz : (", ");"},
+    {"bar, ", ";"},
+    {"", ", bar;"},
+    { NULL, NULL }
+  };
+
+  const char* statement_data[] = {
+    "() => {}",
+    "() => { return 42 }",
+    "x => { return x; }",
+    "(x) => { return x; }",
+    "(x, y) => { return x + y; }",
+    "(x, y, z) => { return x + y + z; }",
+    "(x, y) => { x.a = y; }",
+    "() => 42",
+    "x => x",
+    "x => x * x",
+    "(x) => x",
+    "(x) => x * x",
+    "(x, y) => x + y",
+    "(x, y, z) => x, y, z",
+    "(x, y) => x.a = y",
+    "() => ({'value': 42})",
+    "x => y => x + y",
+    "(x, y) => (u, v) => x*u + y*v",
+    "(x, y) => z => z * (x + y)",
+    "x => (y, z) => z * (x + y)",
+    NULL
+  };
+
+  RunParserSyncTest(context_data, statement_data, kSuccess);
+}