Bug 1164741 - Readd parsing support for |for (var ...1 = ...2 in ...3)|, but completely ignore the |= ...2| assignment when ascribing semantics to it. r=jorendorff, r=efaust, a=sledru
authorJeff Walden <jwalden@mit.edu>
Wed, 13 May 2015 21:13:02 -0700
changeset 274712 b852ee9e576f0c10e5c450907c0ea45e4bbcb9ea
parent 274711 5f8d1e7899aad0db0c5633e8e18e7501b4244f43
child 274713 f74f78b5025511295894cb4ba7a61c23d2d19c35
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff, efaust, sledru
bugs1164741
milestone40.0a2
Bug 1164741 - Readd parsing support for |for (var ...1 = ...2 in ...3)|, but completely ignore the |= ...2| assignment when ascribing semantics to it. r=jorendorff, r=efaust, a=sledru
js/src/frontend/Parser.cpp
js/src/tests/ecma_6/Statements/browser.js
js/src/tests/ecma_6/Statements/for-in-with-destructuring-assignments.js
js/src/tests/ecma_6/Statements/for-of-var-with-initializer.js
js/src/tests/ecma_6/Statements/shell.js
js/src/tests/ecma_6/extensions/for-in-with-assignments.js
js/src/tests/js1_8_5/reflect-parse/for-loop-destructuring.js
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -3886,16 +3886,29 @@ Parser<ParseHandler>::variables(ParseNod
                 }
 
                 MUST_MATCH_TOKEN(TOK_ASSIGN, JSMSG_BAD_DESTRUCT_DECL);
 
                 Node init = assignExpr();
                 if (!init)
                     return null();
 
+                // Ban the nonsensical |for (var V = E1 in E2);| where V is a
+                // destructuring pattern.  See bug 1164741 for background.
+                if (pc->parsingForInit && kind == PNK_VAR) {
+                    TokenKind afterInit;
+                    if (!tokenStream.peekToken(&afterInit))
+                        return null();
+                    if (afterInit == TOK_IN) {
+                        report(ParseError, false, init, JSMSG_INVALID_FOR_INOF_DECL_WITH_INIT,
+                               "in");
+                        return null();
+                    }
+                }
+
                 if (!bindBeforeInitializer && !checkDestructuring(&data, pn2))
                     return null();
 
                 pn2 = handler.newBinary(PNK_ASSIGN, pn2, init);
                 if (!pn2)
                     return null();
                 handler.addList(pn, pn2);
                 break;
@@ -3939,21 +3952,44 @@ Parser<ParseHandler>::variables(ParseNod
                 bool bindBeforeInitializer = kind != PNK_LET && kind != PNK_CONST;
                 if (bindBeforeInitializer && !data.binder(&data, name, this))
                     return null();
 
                 Node init = assignExpr();
                 if (!init)
                     return null();
 
-                if (!bindBeforeInitializer && !data.binder(&data, name, this))
-                    return null();
-
-                if (!handler.finishInitializerAssignment(pn2, init, data.op))
-                    return null();
+                // Ignore an initializer if we have a for-in loop declaring a
+                // |var| with an initializer: |for (var v = ... in ...);|.
+                // Warn that this syntax is invalid so that developers looking
+                // in the console know to fix this.  ES<6 permitted the
+                // initializer while ES6 doesn't; ignoring it seems the best
+                // way to incrementally move to ES6 semantics.
+                bool performAssignment = true;
+                if (pc->parsingForInit && kind == PNK_VAR) {
+                    TokenKind afterInit;
+                    if (!tokenStream.peekToken(&afterInit))
+                        return null();
+                    if (afterInit == TOK_IN) {
+                        performAssignment = false;
+                        if (!report(ParseWarning, pc->sc->strict(), init,
+                                    JSMSG_INVALID_FOR_INOF_DECL_WITH_INIT, "in"))
+                        {
+                            return null();
+                        }
+                    }
+                }
+
+                if (performAssignment) {
+                    if (!bindBeforeInitializer && !data.binder(&data, name, this))
+                        return null();
+
+                    if (!handler.finishInitializerAssignment(pn2, init, data.op))
+                        return null();
+                }
             } else {
                 if (data.isConst && !pc->parsingForInit) {
                     report(ParseError, false, null(), JSMSG_BAD_CONST_DECL);
                     return null();
                 }
 
                 if (!data.binder(&data, name, this))
                     return null();
@@ -4824,18 +4860,24 @@ Parser<FullParseHandler>::forStatement()
          * After the following if-else, pn2 will point to the name or
          * destructuring pattern on in's left. pn1 will point to the decl, if
          * any, else nullptr. Note that the "declaration with initializer" case
          * rewrites the loop-head, moving the decl and setting pn1 to nullptr.
          */
         if (isForDecl) {
             pn2 = pn1->pn_head;
             if ((pn2->isKind(PNK_NAME) && pn2->maybeExpr()) || pn2->isKind(PNK_ASSIGN)) {
-                // We have a bizarre |for (var/const/let x = ... in/of ...)|
-                // loop erroneously permitted by ES1-5 but removed in ES6.
+                MOZ_ASSERT(!(headKind == PNK_FORIN && pn1->isKind(PNK_VAR)),
+                           "Parser::variables should have ignored the "
+                           "initializer in the ES5-sanctioned, ES6-prohibited "
+                           "|for (var ... = ... in ...)| syntax");
+
+                // Otherwise, this bizarre |for (const/let x = ... in/of ...)|
+                // loop isn't valid ES6 and has never been permitted in
+                // SpiderMonkey.
                 report(ParseError, false, pn2, JSMSG_INVALID_FOR_INOF_DECL_WITH_INIT,
                        headKind == PNK_FOROF ? "of" : "in");
                 return null();
             }
         } else {
             /* Not a declaration. */
             MOZ_ASSERT(!blockObj);
             pn2 = pn1;
new file mode 100644
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Statements/for-in-with-destructuring-assignments.js
@@ -0,0 +1,130 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var gTestfile = "for-in-with-destructuring-assignments.js";
+var BUGNUMBER = 1164741;
+var summary = "|for (var <pat> = ... in ...)| is invalid syntax";
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+// This is a total grab-bag of junk originally in tests changed when this
+// syntax was removed.  Avert your eyes!
+
+assertThrowsInstanceOf(() => eval(`
+   for (var [x] = x>>x in [[]<[]])
+   {
+     [];
+   }`),
+ SyntaxError);
+
+/******************************************************************************/
+
+assertThrowsInstanceOf(function() {
+  // Abandon all hope, ye who try to read this.
+  eval(`
+    (function () {
+      for
+      (var [x] = function(){}
+       in
+       (function m(a) {
+         if (a < 1) {
+           x;
+           return;
+         }
+         return m(a - 1) + m(a - 2);
+       })(7)(eval(""))
+      )
+      {
+        [];
+      }
+    })
+  `)();
+}, SyntaxError);
+
+/******************************************************************************/
+
+assertThrowsInstanceOf(() => eval(`
+  for (var [e] = [] in (eval("for (b = 0; b < 6; ++b) gc()"))) {}
+`), SyntaxError);
+
+/******************************************************************************/
+
+assertThrowsInstanceOf(() => eval("for (var [ v , c ] = 0 in undefined) { }"),
+                       SyntaxError);
+
+/******************************************************************************/
+
+assertThrowsInstanceOf(() => eval("var b = e; for (var [e] = b in w) c"),
+                       SyntaxError);
+
+/******************************************************************************/
+
+assertThrowsInstanceOf(() => eval("for (var {a: []} = 2 in []) { }"),
+                       SyntaxError);
+
+/******************************************************************************/
+
+assertThrowsInstanceOf(() => eval(`try
+  {
+    for (var [,{y}] = 1 in []) {}
+  }
+  catch(ex)
+  {
+  }`),
+SyntaxError);
+
+/******************************************************************************/
+
+assertThrowsInstanceOf(() => eval("for (var [x] = [] in null);"),
+                       SyntaxError);
+
+/******************************************************************************/
+
+assertThrowsInstanceOf(() => eval("for (var [x] = x in y) var x;"),
+                       SyntaxError);
+
+/******************************************************************************/
+
+assertThrowsInstanceOf(() => eval(`
+  for (var [arguments] = ({ get y(){} }) in y ) (x);
+`),
+SyntaxError);
+
+/******************************************************************************/
+
+if (typeof evalcx == 'function') {
+  var src = 'try {\n' +
+  '    for (var [e] = /x/ in d) {\n' +
+  '        (function () {});\n' +
+  '    }\n' +
+  '} catch (e) {}\n' +
+  'try {\n' +
+  '    let(x = Object.freeze(this, /x/))\n' +
+  '    e = {}.toString\n' +
+  '    function y() {}\n' +
+  '} catch (e) {}';
+
+  try
+  {
+    evalcx(src);
+    throw new Error("didn't throw");
+  }
+  catch (e)
+  {
+    assertEq(e.name === "SyntaxError", true,
+             "expected invalid syntax, got " + e);
+  }
+}
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("Tests complete");
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Statements/for-of-var-with-initializer.js
@@ -0,0 +1,32 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var gTestfile = "for-of-var-with-initializer.js";
+var BUGNUMBER = 1164741;
+var summary = "Don't assert parsing |for (var x = 3 of 42);|";
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+try
+{
+  Function("for (var x = 3 of 42);");
+  throw new Error("didn't throw");
+}
+catch (e)
+{
+  assertEq(e instanceof SyntaxError, true,
+           "expected syntax error, got: " + e);
+}
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("Tests complete");
new file mode 100644
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/extensions/for-in-with-assignments.js
@@ -0,0 +1,74 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var gTestfile = "for-in-with-assignments.js";
+var BUGNUMBER = 1164741;
+var summary =
+  "Parse |for (var ... = ... in ...)| but execute it as if the assignment " +
+  "weren't there";
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+// This is a total grab-bag of junk originally in tests changed when this
+// syntax was removed.  Leaving it all in one file will make it easier to
+// eventually remove.  Avert your eyes!
+
+if (typeof Reflect !== "undefined")
+  Reflect.parse("for (var x = 3 in []) { }");
+
+/******************************************************************************/
+
+function testQ() {
+  try {
+    for (var i = 0 in this) throw p;
+  } catch (e) {
+    if (i !== 94)
+      return "what";
+  }
+}
+testQ();
+
+/******************************************************************************/
+
+function f3(i,o){for(var x=i in o)parseInt(o[x]); return x}
+function f4(i,o){with(this)for(var x=i in o)parseInt(o[x]); return x}
+
+assertEq(f3(42, []), undefined);
+assertEq(f3(42, ['first']), "0");
+assertEq(f4(42, []), undefined);
+assertEq(f4(42, ['first']), "0");
+
+/******************************************************************************/
+
+function SetLangHead(l){
+  with(p){
+    for(var i=0 in x)
+      if(getElementById("TxtH"+i)!=undefined)
+        parseInt('huh');
+  }
+}
+x=[0,1,2,3];
+p={getElementById: function (id){parseInt(uneval(this), id); return undefined;}};
+SetLangHead(1);
+
+/******************************************************************************/
+
+(function(){for(var x = arguments in []){} function x(){}})();
+
+/******************************************************************************/
+
+with (0)
+  for (var b = 0 in 0);  // don't assert in parser
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("Tests complete");
--- a/js/src/tests/js1_8_5/reflect-parse/for-loop-destructuring.js
+++ b/js/src/tests/js1_8_5/reflect-parse/for-loop-destructuring.js
@@ -31,11 +31,21 @@ assertError("for (const {a:x,b:y,c:z} in
 assertError("for (const [x,y,z] in foo);", SyntaxError);
 assertError("for (const x of foo);", SyntaxError);
 assertError("for (const {a:x,b:y,c:z} of foo);", SyntaxError);
 assertError("for (const [x,y,z] of foo);", SyntaxError);
 assertError("for each (const x in foo);", SyntaxError);
 assertError("for each (const {a:x,b:y,c:z} in foo);", SyntaxError);
 assertError("for each (const [x,y,z] in foo);", SyntaxError);
 
+assertError("for (x = 22 in foo);", SyntaxError);-
+assertError("for ({a:x,b:y,c:z} = 22 in foo);", SyntaxError);
+assertError("for ([x,y,z] = 22 in foo);", SyntaxError);
+assertError("for (const x = 22 in foo);", SyntaxError);
+assertError("for (const {a:x,b:y,c:z} = 22 in foo);", SyntaxError);
+assertError("for (const [x,y,z] = 22 in foo);", SyntaxError);
+assertError("for each (const x = 22 in foo);", SyntaxError);
+assertError("for each (const {a:x,b:y,c:z} = 22 in foo);", SyntaxError);
+assertError("for each (const [x,y,z] = 22 in foo);", SyntaxError);
+
 }
 
 runtest(test);