Bug 1306701 - Part 1: Evaluate var-initializer expression in for-in loop per Annex B.3.6 (ES2017). r=Waldo
authorAndré Bargull <andre.bargull@gmail.com>
Mon, 10 Oct 2016 10:29:32 -0700
changeset 360359 86e17a8b40d0daaa968613960860b62690d31790
parent 360358 d4c1417d3155cc3bc69ae984698470ea61af78b9
child 360360 61da7056917a6361f5499f5431b83421979c5233
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-beta@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs1306701
milestone52.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 1306701 - Part 1: Evaluate var-initializer expression in for-in loop per Annex B.3.6 (ES2017). r=Waldo
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/Parser.cpp
js/src/tests/ecma_2017/Statements/for-in-with-assignment-semantics.js
js/src/tests/ecma_2017/Statements/for-in-with-assignment-syntax.js
js/src/tests/ecma_2017/Statements/for-in-with-assignments.js
js/src/tests/ecma_6/extensions/for-in-with-assignments.js
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -6168,16 +6168,43 @@ BytecodeEmitter::emitForIn(ParseNode* fo
     MOZ_ASSERT(forInLoop->isKind(PNK_FOR));
     MOZ_ASSERT(forInLoop->isArity(PN_BINARY));
     MOZ_ASSERT(forInLoop->isOp(JSOP_ITER));
 
     ParseNode* forInHead = forInLoop->pn_left;
     MOZ_ASSERT(forInHead->isKind(PNK_FORIN));
     MOZ_ASSERT(forInHead->isArity(PN_TERNARY));
 
+    // Annex B: Evaluate the var-initializer expression if present.
+    // |for (var i = initializer in expr) { ... }|
+    ParseNode* forInTarget = forInHead->pn_kid1;
+    if (parser->handler.isDeclarationList(forInTarget)) {
+        ParseNode* decl = parser->handler.singleBindingFromDeclaration(forInTarget);
+        if (decl->isKind(PNK_NAME)) {
+            if (ParseNode* initializer = decl->expr()) {
+                MOZ_ASSERT(forInTarget->isKind(PNK_VAR),
+                           "for-in initializers are only permitted for |var| declarations");
+
+                if (!updateSourceCoordNotes(decl->pn_pos.begin))
+                    return false;
+
+                auto emitRhs = [initializer](BytecodeEmitter* bce, const NameLocation&, bool) {
+                    return bce->emitTree(initializer);
+                };
+
+                if (!emitInitializeName(decl, emitRhs))
+                    return false;
+
+                // Pop the initializer.
+                if (!emit1(JSOP_POP))
+                    return false;
+            }
+        }
+    }
+
     // Evaluate the expression being iterated.
     ParseNode* expr = forInHead->pn_kid3;
     if (!emitTree(expr))                                  // EXPR
         return false;
 
     // Convert the value to the appropriate sort of iterator object for the
     // loop variant (for-in, for-each-in, or destructuring for-in).
     unsigned iflags = forInLoop->pn_iflags;
@@ -6210,17 +6237,16 @@ BytecodeEmitter::emitForIn(ParseNode* fo
     // If the loop had an escaping lexical declaration, replace the current
     // environment with an dead zoned one to implement TDZ semantics.
     if (headLexicalEmitterScope) {
         // The environment chain only includes an environment for the for-in
         // loop head *if* a scope binding is captured, thereby requiring
         // recreation each iteration. If a lexical scope exists for the head,
         // it must be the innermost one. If that scope has closed-over
         // bindings inducing an environment, recreate the current environment.
-        DebugOnly<ParseNode*> forInTarget = forInHead->pn_kid1;
         MOZ_ASSERT(forInTarget->isKind(PNK_LET) || forInTarget->isKind(PNK_CONST));
         MOZ_ASSERT(headLexicalEmitterScope == innermostEmitterScope);
         MOZ_ASSERT(headLexicalEmitterScope->scope(this)->kind() == ScopeKind::Lexical);
 
         if (headLexicalEmitterScope->hasEnvironment()) {
             if (!emit1(JSOP_RECREATELEXICALENV))          // ITER ITERVAL
                 return false;
         }
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -4182,20 +4182,21 @@ Parser<ParseHandler>::initializerInNameD
                 *forHeadKind = PNK_FORIN;
                 if (!report(ParseWarning, pc->sc()->strict(), initializer,
                             JSMSG_INVALID_FOR_IN_DECL_WITH_INIT))
                 {
                     return false;
                 }
 
                 *forInOrOfExpression = expressionAfterForInOrOf(PNK_FORIN, yieldHandling);
-                return *forInOrOfExpression != null();
+                if (!*forInOrOfExpression)
+                    return false;
+            } else {
+                *forHeadKind = PNK_FORHEAD;
             }
-
-            *forHeadKind = PNK_FORHEAD;
         } else {
             MOZ_ASSERT(*forHeadKind == PNK_FORHEAD);
 
             // In the very rare case of Parser::assignExpr consuming an
             // ArrowFunction with block body, when full-parsing with the arrow
             // function being a skipped lazy inner function, we don't have
             // lookahead for the next token.  Do a one-off peek here to be
             // consistent with what Parser::matchForInOrOf does in the other
@@ -4203,20 +4204,23 @@ Parser<ParseHandler>::initializerInNameD
             //
             // If you think this all sounds pretty code-smelly, you're almost
             // certainly correct.
             TokenKind ignored;
             if (!tokenStream.peekToken(&ignored))
                 return false;
         }
 
-        // Per Parser::forHeadStart, the semicolon in |for (;| is ultimately
-        // gotten as Operand.  But initializer expressions terminate with the
-        // absence of an operator gotten as None, so we need an exception.
-        tokenStream.addModifierException(TokenStream::OperandIsNone);
+        if (*forHeadKind == PNK_FORHEAD) {
+            // Per Parser::forHeadStart, the semicolon in |for (;| is
+            // ultimately gotten as Operand.  But initializer expressions
+            // terminate with the absence of an operator gotten as None,
+            // so we need an exception.
+            tokenStream.addModifierException(TokenStream::OperandIsNone);
+        }
     }
 
     return handler.finishInitializerAssignment(binding, initializer);
 }
 
 template <typename ParseHandler>
 typename ParseHandler::Node
 Parser<ParseHandler>::declarationName(Node decl, DeclarationKind declKind, TokenKind tt,
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_2017/Statements/for-in-with-assignment-semantics.js
@@ -0,0 +1,46 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+const unreachable = () => { throw "unreachable"; };
+
+// var-initializer expression is executed before for-in expression.
+var log = "";
+for (var x = (log += "head") in (log += "|expr", null)) unreachable();
+assertEq(log, "head|expr");
+
+log = "";
+for (var x = (log += "head") in (log += "|expr", {})) unreachable();
+assertEq(log, "head|expr");
+
+
+// for-in expression isn't executed when var-initializer throws exception.
+function ExpectedError() {}
+assertThrowsInstanceOf(() => {
+    var throwErr = () => { throw new ExpectedError(); };
+    for (var x = throwErr() in unreachable()) unreachable();
+}, ExpectedError);
+
+
+// Ensure environment operations are performed correctly.
+var scope = new Proxy({x: 0}, new Proxy({}, {
+    get(t, pk, r) {
+        log += pk + "|";
+    }
+}));
+
+log = "";
+with (scope) {
+    for (var x = 0 in {}) ;
+}
+assertEq(log, "has|get|set|getOwnPropertyDescriptor|defineProperty|");
+
+log = "";
+with (scope) {
+    for (var x = 0 in {p: 0}) ;
+}
+assertEq(log, "has|get|set|getOwnPropertyDescriptor|defineProperty|".repeat(2));
+
+
+if (typeof reportCompare === "function")
+    reportCompare(true, true);
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_2017/Statements/for-in-with-assignment-syntax.js
@@ -0,0 +1,66 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+const validSyntax = [
+    "var x",
+];
+
+const destructuring = [
+    "[]",
+    "[,]",
+    "[a]",
+    "[a = 0]",
+    "[...a]",
+    "[...[]]",
+    "[...[a]]",
+    "{}",
+    "{p: x}",
+    "{p: x = 0}",
+    "{x}",
+    "{x = 0}",
+];
+
+const invalidSyntax = [
+    ...destructuring.map(binding => `var ${binding}`),
+    "let x",
+    ...destructuring.map(binding => `let ${binding}`),
+    "const x",
+    ...destructuring.map(binding => `const ${binding}`),
+    "x",
+    ...destructuring.map(binding => `${binding}`),
+    "o.p",
+    "o[0]",
+    "f()",
+];
+
+for (let valid of validSyntax) {
+    eval(`for (${valid} = 0 in {});`)
+}
+
+for (let invalid of invalidSyntax) {
+    assertThrowsInstanceOf(() => eval(`for (${invalid} = 0 in {});`), SyntaxError);
+}
+
+// Invalid syntax, needs method context to parse.
+assertThrowsInstanceOf(() => eval(`({ m() { for (super.p = 0 in {}); } })`), SyntaxError);
+assertThrowsInstanceOf(() => eval(`({ m() { for (super[0] = 0 in {}); } })`), SyntaxError);
+
+// Throws ReferenceError instead of SyntaxError, because we intermingle parsing
+// and early error checking.
+assertThrowsInstanceOf(() => eval(`for (0 = 0 in {});`), ReferenceError);
+assertThrowsInstanceOf(() => eval(`for (i++ = 0 in {});`), ReferenceError);
+assertThrowsInstanceOf(() => eval(`for (new F() = 0 in {});`), ReferenceError);
+assertThrowsInstanceOf(() => eval(`function f() { for (new.target = 0 in {}); }`), ReferenceError);
+assertThrowsInstanceOf(() => eval(`class C extends D { constructor() { for (super() = 0 in {}); } }`), ReferenceError);
+
+// Same as above, only this time don't make it look like we actually parse a for-in statement.
+assertThrowsInstanceOf(() => eval(`for (0 = 0 #####`), ReferenceError);
+assertThrowsInstanceOf(() => eval(`for (i++ = 0 #####`), ReferenceError);
+assertThrowsInstanceOf(() => eval(`for (new F() = 0 #####`), ReferenceError);
+assertThrowsInstanceOf(() => eval(`function f() { for (new.target = 0 #####`), ReferenceError);
+assertThrowsInstanceOf(() => eval(`class C extends D { constructor() { for (super() = 0 #####`), ReferenceError);
+
+
+if (typeof reportCompare === "function")
+    reportCompare(true, true);
rename from js/src/tests/ecma_6/extensions/for-in-with-assignments.js
rename to js/src/tests/ecma_2017/Statements/for-in-with-assignments.js
--- a/js/src/tests/ecma_6/extensions/for-in-with-assignments.js
+++ b/js/src/tests/ecma_2017/Statements/for-in-with-assignments.js
@@ -1,28 +1,27 @@
 /*
  * 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";
+var summary = "Parse |for (var ... = ... in ...)|."
 
 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!
+// TC39 has revived this syntax for ES2017 - "What is dead may never die."
 
 if (typeof Reflect !== "undefined" && Reflect.parse)
   Reflect.parse("for (var x = 3 in []) { }");
 
 /******************************************************************************/
 
 function testQ() {
   try {
@@ -34,19 +33,19 @@ function testQ() {
 }
 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, []), 42);
 assertEq(f3(42, ['first']), "0");
-assertEq(f4(42, []), undefined);
+assertEq(f4(42, []), 42);
 assertEq(f4(42, ['first']), "0");
 
 /******************************************************************************/
 
 function SetLangHead(l){
   with(p){
     for(var i=0 in x)
       if(getElementById("TxtH"+i)!=undefined)
@@ -67,16 +66,17 @@ with (0)
   for (var b = 0 in 0);  // don't assert in parser
 
 /******************************************************************************/
 
 function* g1() {
   for (var x = yield in {}) ;
 }
 var it = g1();
+assertEq(it.next().done, false);
 assertEq(it.next().done, true);
 
 /******************************************************************************/
 
 if (typeof reportCompare === "function")
   reportCompare(true, true);
 
 print("Tests complete");