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
authorJeff Walden <jwalden@mit.edu>
Wed, 13 May 2015 21:13:02 -0700
changeset 244707 9841c5d229b8e7b439df88071a65b28caaea8cce
parent 244706 c6fe9c11095a26eeb894209944c7747690533b25
child 244708 e6826d9be450d58145fedea05423c3616ea523e9
push id28786
push usercbook@mozilla.com
push dateWed, 20 May 2015 13:54:15 +0000
treeherdermozilla-central@8d8df22fe72d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff, efaust
bugs1164741
milestone41.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
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
@@ -3939,22 +3939,36 @@ Parser<ParseHandler>::variables(YieldHan
                 }
 
                 MUST_MATCH_TOKEN(TOK_ASSIGN, JSMSG_BAD_DESTRUCT_DECL);
 
                 Node init = assignExpr(InAllowed, yieldHandling);
                 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;
             }
 
             if (tt != TOK_NAME) {
                 if (tt == TOK_YIELD) {
                     if (!checkYieldNameValidity())
                         return null();
@@ -3992,21 +4006,44 @@ Parser<ParseHandler>::variables(YieldHan
                 bool bindBeforeInitializer = kind != PNK_LET && kind != PNK_CONST;
                 if (bindBeforeInitializer && !data.binder(&data, name, this))
                     return null();
 
                 Node init = assignExpr(InAllowed, yieldHandling);
                 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();
@@ -4880,18 +4917,24 @@ Parser<FullParseHandler>::forStatement(Y
          * 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);