Bug 1077949 - Fix TDZ checks when closing over non-dominating lexical declarations in switches. (r=Waldo)
authorShu-yu Guo <shu@rfrn.org>
Wed, 15 Oct 2014 18:06:50 -0700
changeset 210665 0b865ab14ff00cb11bf45e02b51b663c450629e7
parent 210664 748ffeff132c1c5c7bcea9e44a74d4ba0bf08374
child 210666 23009633a7dc2023a9d13d26596fda4dbc8897e5
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersWaldo
bugs1077949
milestone36.0a1
Bug 1077949 - Fix TDZ checks when closing over non-dominating lexical declarations in switches. (r=Waldo)
js/src/frontend/Parser.cpp
js/src/jit-test/tests/basic/letTDZSwitchClosure.js
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -1249,16 +1249,27 @@ ConvertDefinitionToNamedLambdaUse(TokenS
      * setter can either ignore the set (in non-strict mode) or
      * produce an error (in strict mode).
      */
     if (dn->isClosed() || dn->isAssigned())
         funbox->setNeedsDeclEnvObject();
     return true;
 }
 
+static bool
+IsNonDominatingInScopedSwitch(ParseContext<FullParseHandler> *pc, HandleAtom name,
+                              Definition *dn)
+{
+    MOZ_ASSERT(dn->isLet());
+    StmtInfoPC *stmt = LexicalLookup(pc, name, nullptr, nullptr);
+    if (stmt && stmt->type == STMT_SWITCH)
+        return dn->pn_cookie.slot() < stmt->firstDominatingLexicalInCase;
+    return false;
+}
+
 /*
  * Beware: this function is called for functions nested in other functions or
  * global scripts but not for functions compiled through the Function
  * constructor or JSAPI. To always execute code when a function has finished
  * parsing, use Parser::functionBody.
  */
 template <>
 bool
@@ -1333,27 +1344,35 @@ Parser<FullParseHandler>::leaveFunction(
              * (see LegacyCompExprTransplanter::transplant, the PN_CODE/PN_NAME
              * case), and nowhere else, currently.
              */
             if (dn != outer_dn) {
                 if (ParseNode *pnu = dn->dn_uses) {
                     // In ES6, lexical bindings cannot be accessed until
                     // initialized. If we are parsing a function with a
                     // hoisted body-level use, all free variables that get
-                    // linked to an outer 'let' binding need to be marked as
+                    // linked to an outer lexical binding need to be marked as
                     // needing dead zone checks. e.g.,
                     //
                     // function outer() {
                     //   inner();
                     //   function inner() { use(x); }
                     //   let x;
                     // }
                     //
                     // The use of 'x' inside 'inner' needs to be marked.
-                    if (bodyLevelHoistedUse && outer_dn->isLet()) {
+                    //
+                    // Similarly, if we are closing over a lexical binding
+                    // from another case in a switch, those uses also need to
+                    // be marked as needing dead zone checks.
+                    RootedAtom name(context, atom);
+                    if (outer_dn->isLet() &&
+                        (bodyLevelHoistedUse ||
+                         IsNonDominatingInScopedSwitch(outerpc, name, outer_dn)))
+                    {
                         while (true) {
                             pnu->pn_dflags |= PND_LET;
                             if (!pnu->pn_link)
                                 break;
                             pnu = pnu->pn_link;
                         }
                         pnu = dn->dn_uses;
                     }
@@ -1867,16 +1886,23 @@ Parser<ParseHandler>::addFreeVariablesFr
         // In ES6, lexical bindings are unaccessible before initialization. If
         // the inner function closes over a placeholder definition, we need to
         // mark the variable as maybe needing a dead zone check when we emit
         // bytecode.
         //
         // Note that body-level function declaration statements are always
         // hoisted to the top, so all accesses to free let variables need the
         // dead zone check.
+        //
+        // Subtlety: we don't need to check for closing over a non-dominating
+        // lexical binding in a switch, as lexical declarations currently
+        // disable syntax parsing. So a non-dominating but textually preceding
+        // lexical declaration would have aborted syntax parsing, and a
+        // textually following declaration would return true for
+        // handler.isPlaceholderDefinition(dn) below.
         if (handler.isPlaceholderDefinition(dn) || bodyLevelHoistedUse)
             freeVariables[i].setIsHoistedUse();
 
         /* Mark the outer dn as escaping. */
         handler.setFlag(handler.getDefinitionNode(dn), PND_CLOSED);
     }
 
     PropagateTransitiveParseFlags(lazy, pc->sc);
@@ -3137,17 +3163,17 @@ Parser<ParseHandler>::noteNameUse(Handle
 
     handler.linkUseToDef(pn, dn);
 
     if (stmt) {
         if (stmt->type == STMT_WITH) {
             handler.setFlag(pn, PND_DEOPTIMIZED);
         } else if (stmt->type == STMT_SWITCH && stmt->isBlockScope) {
             // See comments above StmtInfoPC and switchStatement for how
-            // firstDominatingLetInCase is computed.
+            // firstDominatingLexicalInCase is computed.
             MOZ_ASSERT(stmt->firstDominatingLexicalInCase <= stmt->staticBlock().numVariables());
             handler.markMaybeUninitializedLexicalUseInSwitch(pn, dn,
                                                              stmt->firstDominatingLexicalInCase);
         }
     }
 
     return true;
 }
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/letTDZSwitchClosure.js
@@ -0,0 +1,60 @@
+function assertThrowsReferenceError(f) {
+  var err;
+  try {
+    f();
+  } catch (e) {
+    err = e;
+  }
+  assertEq(err instanceof ReferenceError, true);
+}
+
+function f() {
+    switch (0) {
+        case 1:
+            let x
+        case function() {
+            print(x)
+        }():
+    }
+}
+assertThrowsReferenceError(f);
+
+function g() {
+  switch (0) {
+    case 1:
+      let x;
+    case 0:
+      var inner = function () {
+        print(x);
+      }
+      inner();
+      break;
+  }
+}
+assertThrowsReferenceError(g);
+
+function h() {
+  switch (0) {
+    case 0:
+      var inner = function () {
+        print(x);
+      }
+      inner();
+    case 1:
+      let x;
+  }
+}
+assertThrowsReferenceError(h);
+
+// Tests that a dominating lexical doesn't throw.
+function F() {
+  switch (0) {
+    case 0:
+      let x = 42;
+      var inner = function () {
+        assertEq(x, 42);
+      }
+      inner();
+  }
+}
+F();