[parser] Disallow Expression in for..of statements

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 4891 matching lines @@
   const char* data[] = {
       "for(const x = 1; ; ) {}",
       "for(const x = 1, y = 2;;){}",
       "for(const x in [1,2,3]) {}",
       "for(const x of [1,2,3]) {}",
       NULL};
   RunParserSyncTest(context_data, data, kSuccess, nullptr, 0, nullptr, 0);
 }
 
 
+TEST(StatementParsingInForIn) {
+  const char* context_data[][2] = {{"'use strict';", ""},

[adamk, 2016/01/20 00:35:57]

Can you add non-strict cases too? Seems like the mode shouldn't matter.

[mike3, 2016/01/20 17:55:40]

I've omitted strict mode because I was following the pattern set by
ConstParsingForInError. I didn't realize this until attempting to enable
non-strict mode versions that V8 requires strict mode for block-scoped
declarations.

If you prefer, I could split these tests in two: one set that has `for(x of y)`
and `for(var x of y)` (which V8 will parse outside of strict mode) and a
separate test for `for(let x of y)` and `for(const x of y)`. Let me know!

[adamk, 2016/01/20 19:31:17]

Outside strict mode, v8 will parse lexical declarations if the proper flags are
set: kAllowHarmonySloppy, kAllowHarmonySloppyLet, and kNoLegacyConst. See the
"LetSloppyOnly" test for an example that passes those flags to
RunParserSyncTest.

[mike3, 2016/01/20 19:54:08]

Thanks for the pointer! (It's been a while since I wrote C++; I really missed
the memory management puns.)

+                                   {"function foo(){ 'use strict';", "}"},
+                                   {NULL, NULL}};
+
+  const char* data[] = {"for(x in {}, {}) {}", "for(var x in {}, {}) {}",
+                        "for(let x in {}, {}) {}", "for(const x in {}, {}) {}",
+                        NULL};
+  RunParserSyncTest(context_data, data, kSuccess, nullptr, 0, nullptr, 0);

[adamk, 2016/01/20 00:35:57]

You can leave off the last 4 arguments here (they default to null and 0). Same
goes for all the calls below

[mike3, 2016/01/20 17:55:40]

Acknowledged.

+}
+
+
 TEST(ConstParsingInForInError) {
   const char* context_data[][2] = {{"'use strict';", ""},
                                    {"function foo(){ 'use strict';", "}"},
                                    {NULL, NULL}};
 
   const char* data[] = {
       "for(const x,y = 1; ; ) {}",
       "for(const x = 4 in [1,2,3]) {}",
       "for(const x = 4, y in [1,2,3]) {}",
       "for(const x = 4 of [1,2,3]) {}",
@@ skipping 153 matching lines @@
   const char* data[] = {
       "for (var of [1, 2, 3]) {}",
       "for (const of [1, 2, 3]) {}",
       NULL};
   static const ParserFlag always_flags[] = {kAllowHarmonySloppy};
   RunParserSyncTest(context_data, data, kError, nullptr, 0, always_flags,
                     arraysize(always_flags));
 }
 
 
+TEST(ForOfInOperator) {
+  const char* context_data[][2] = {{"'use strict';", ""},
+                                   {"function foo(){ 'use strict';", "}"},
+                                   {NULL, NULL}};
+
+  const char* data[] = {
+      "for(x of 'foo' in {}) {}", "for(var x of 'foo' in {}) {}",
+      "for(let x of 'foo' in {}) {}", "for(const x of 'foo' in {}) {}", NULL};
+  RunParserSyncTest(context_data, data, kSuccess, nullptr, 0, nullptr, 0);
+}
+
+
+TEST(ForOfYieldIdentifier) {
+  const char* context_data[][2] = {{NULL, NULL}};

[adamk, 2016/01/20 00:35:57]

This won't actually run any tests, since you haven't provided any context_data.
See the outer for loop in RunParserSyncTest for how this works.

[mike3, 2016/01/20 17:55:40]

Acknowledged.

+
+  const char* data[] = {"for(x of yield) {}", "for(var x of yield) {}",

[adamk, 2016/01/20 00:38:09]

Also curious why you added tests for yield, that seems unrelated to your patch.

[mike3, 2016/01/20 17:55:40]

I was thinking it would be good to be thorough, since the expression I modified
has a [?Yield] production parameter, and there are current no tests to assert
that is observed. You're right that my change doesn't implement this behavior,
though, so I'm happy to remove these tests if you'd prefer.

[adamk, 2016/01/20 19:31:17]

It's fine to leave them in, thanks for the explanation of why they got added
here (we can always use more coverage of yield parsing).

+                        "for(let x of yield) {}", "for(const x of yield) {}",
+                        NULL};
+  RunParserSyncTest(context_data, data, kSuccess, nullptr, 0, nullptr, 0);
+}
+
+
+TEST(ForOfYieldExpressionError) {

[adamk, 2016/01/20 00:35:57]

This name seems wrong.

[mike3, 2016/01/20 17:55:40]

Acknowledged.

+  const char* context_data[][2] = {{"'use strict';", ""},

[adamk, 2016/01/20 00:35:57]

Again, non-strict versions would be good too, same below.

+                                   {"function foo(){ 'use strict';", "}"},
+                                   {NULL, NULL}};
+
+  const char* data[] = {"(function* g() { for(x of [], []) {} }",
+                        "(function* g() { for(var x of [], []) {} }",
+                        "(function* g() { for(let x of [], []) {} }",
+                        "(function* g() { for(const x of [], []) {} }", NULL};
+  RunParserSyncTest(context_data, data, kError, nullptr, 0, nullptr, 0);
+}
+
+
+TEST(ForOfExpressionError) {
+  const char* context_data[][2] = {{"'use strict';", ""},
+                                   {"function foo(){ 'use strict';", "}"},
+                                   {NULL, NULL}};
+
+  const char* data[] = {
+      "for(x of [], []) {}", "for(var x of [], []) {}",
+      "for(let x of [], []) {}", "for(const x of [], []) {}",
+
+      // AssignmentExpression should be validated statically:
+      "for(x of { y = 23 }) {}", "for(var x of { y = 23 }) {}",
+      "for(let x of { y = 23 }) {}", "for(const x of { y = 23 }) {}", NULL};
+  RunParserSyncTest(context_data, data, kError, nullptr, 0, nullptr, 0);
+}
+
+
 TEST(InvalidUnicodeEscapes) {
   const char* context_data[][2] = {{"", ""},
                                    {"'use strict';", ""},
                                    {NULL, NULL}};
   const char* data[] = {
     "var foob\\u123r = 0;",
     "var \\u123roo = 0;",
     "\"foob\\u123rr\"",
     // No escapes allowed in regexp flags
     "/regex/\\u0069g",
@@ skipping 2743 matching lines @@
 }
 
 
 TEST(MiscSyntaxErrors) {
   const char* context_data[][2] = {
       {"'use strict'", ""}, {"", ""}, {NULL, NULL}};
   const char* error_data[] = {"for (();;) {}", NULL};
 
   RunParserSyncTest(context_data, error_data, kError, NULL, 0, NULL, 0);
 }