Bug 975162 - Remove empty destructuring let-block variable goofiness (r=wingo)
authorLuke Wagner <luke@mozilla.com>
Fri, 21 Feb 2014 10:23:40 -0600
changeset 170261 879e31a2f667470cacf32ba0cdc2a3b5fa72dd9e
parent 170260 18183f6ef0d89a43ec0c7f69a638cf845cc30495
child 170262 343aac1bb02ead4549e25569dcb2d5623550a13a
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewerswingo
bugs975162
milestone30.0a1
Bug 975162 - Remove empty destructuring let-block variable goofiness (r=wingo)
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/Parser.cpp
js/src/vm/ScopeObject.cpp
js/src/vm/ScopeObject.h
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -686,33 +686,29 @@ AllLocalsAliased(StaticBlockObject &obj)
             return false;
     return true;
 }
 #endif
 
 static bool
 ComputeAliasedSlots(ExclusiveContext *cx, BytecodeEmitter *bce, Handle<StaticBlockObject *> blockObj)
 {
+    if (blockObj->slotCount() == 0)
+        return true;
+
     uint32_t depthPlusFixed = blockObj->stackDepth();
     if (!AdjustBlockSlot(cx, bce, &depthPlusFixed))
         return false;
 
     for (unsigned i = 0; i < blockObj->slotCount(); i++) {
-        Definition *dn = blockObj->maybeDefinitionParseNode(i);
-
-        /* Beware the empty destructuring dummy. */
-        if (!dn) {
-            blockObj->setAliased(i, bce->sc->allLocalsAliased());
-            continue;
-        }
+        Definition *dn = blockObj->definitionParseNode(i);
 
         JS_ASSERT(dn->isDefn());
         if (!dn->pn_cookie.set(bce->parser->tokenStream, dn->pn_cookie.level(),
-                               dn->frameSlot() + depthPlusFixed))
-        {
+                               dn->frameSlot() + depthPlusFixed)) {
             return false;
         }
 
 #ifdef DEBUG
         for (ParseNode *pnu = dn->dn_uses; pnu; pnu = pnu->pn_link) {
             JS_ASSERT(pnu->pn_lexdef == dn);
             JS_ASSERT(!(pnu->pn_dflags & PND_BOUND));
             JS_ASSERT(pnu->pn_cookie.isFree());
@@ -3411,17 +3407,16 @@ EmitVariables(ExclusiveContext *cx, Byte
              * here and initialize the name.
              */
             if (pn2->pn_left->isKind(PNK_NAME)) {
                 pn3 = pn2->pn_right;
                 pn2 = pn2->pn_left;
                 goto do_name;
             }
 
-            ptrdiff_t stackDepthBefore = bce->stackDepth;
             JSOp op = JSOP_POP;
             if (pn->pn_count == 1) {
                 /*
                  * If this is the only destructuring assignment in the list,
                  * try to optimize to a group assignment.  If we're in a let
                  * head, pass JSOP_POP rather than the pseudo-prolog JSOP_NOP
                  * in pn->pn_op, to suppress a second (and misplaced) 'let'.
                  */
@@ -3442,24 +3437,16 @@ EmitVariables(ExclusiveContext *cx, Byte
                     return false;
 
                 if (!EmitTree(cx, bce, pn2->pn_right))
                     return false;
 
                 if (!EmitDestructuringOps(cx, bce, pn3, isLet))
                     return false;
             }
-            ptrdiff_t stackDepthAfter = bce->stackDepth;
-
-            /* Give let ([] = x) a slot (see CheckDestructuring). */
-            JS_ASSERT(stackDepthBefore <= stackDepthAfter);
-            if (isLet && stackDepthBefore == stackDepthAfter) {
-                if (Emit1(cx, bce, JSOP_UNDEFINED) < 0)
-                    return false;
-            }
 
             /* If we are not initializing, nothing to pop. */
             if (emitOption != InitializeVars) {
                 if (next)
                     continue;
                 break;
             }
             goto emit_note_pop;
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -3061,17 +3061,16 @@ Parser<FullParseHandler>::checkDestructu
 
     if (left->isKind(PNK_ARRAYCOMP)) {
         report(ParseError, false, left, JSMSG_ARRAY_COMP_LEFTSIDE);
         return false;
     }
 
     Rooted<StaticBlockObject *> blockObj(context);
     blockObj = data && data->binder == bindLet ? data->let.blockObj.get() : nullptr;
-    uint32_t blockCountBefore = blockObj ? blockObj->slotCount() : 0;
 
     if (left->isKind(PNK_ARRAY)) {
         for (ParseNode *pn = left->pn_head; pn; pn = pn->pn_next) {
             if (!pn->isKind(PNK_ELISION)) {
                 if (pn->isKind(PNK_ARRAY) || pn->isKind(PNK_OBJECT)) {
                     ok = checkDestructuring(data, pn, false);
                 } else {
                     if (data) {
@@ -3116,48 +3115,16 @@ Parser<FullParseHandler>::checkDestructu
                 }
                 ok = checkAndMarkAsAssignmentLhs(expr, KeyedDestructuringAssignment);
             }
             if (!ok)
                 return false;
         }
     }
 
-    /*
-     * The catch/finally handler implementation in the interpreter assumes
-     * that any operation that introduces a new scope (like a "let" or "with"
-     * block) increases the stack depth. This way, it is possible to restore
-     * the scope chain based on stack depth of the handler alone. "let" with
-     * an empty destructuring pattern like in
-     *
-     *   let [] = 1;
-     *
-     * would violate this assumption as the there would be no let locals to
-     * store on the stack.
-     *
-     * Furthermore, the decompiler needs an abstract stack location to store
-     * the decompilation of each let block/expr initializer. E.g., given:
-     *
-     *   let (x = 1, [[]] = b, y = 3, {a:[]} = c) { ... }
-     *
-     * four slots are needed.
-     *
-     * To satisfy both constraints, we push a dummy slot (and add a
-     * corresponding dummy property to the block object) for each initializer
-     * that doesn't introduce at least one binding.
-     */
-    if (toplevel && blockObj && blockCountBefore == blockObj->slotCount()) {
-        bool redeclared;
-        RootedId id(context, INT_TO_JSID(blockCountBefore));
-        if (!StaticBlockObject::addVar(context, blockObj, id, blockCountBefore, &redeclared))
-            return false;
-        JS_ASSERT(!redeclared);
-        JS_ASSERT(blockObj->slotCount() == blockCountBefore + 1);
-    }
-
     return true;
 }
 
 template <>
 bool
 Parser<SyntaxParseHandler>::checkDestructuring(BindData<SyntaxParseHandler> *data,
                                                Node left, bool toplevel)
 {
--- a/js/src/vm/ScopeObject.cpp
+++ b/js/src/vm/ScopeObject.cpp
@@ -715,17 +715,17 @@ StaticBlockObject::create(ExclusiveConte
 
     return &obj->as<StaticBlockObject>();
 }
 
 /* static */ Shape *
 StaticBlockObject::addVar(ExclusiveContext *cx, Handle<StaticBlockObject*> block, HandleId id,
                           unsigned index, bool *redeclared)
 {
-    JS_ASSERT(JSID_IS_ATOM(id) || (JSID_IS_INT(id) && JSID_TO_INT(id) == (int)index));
+    JS_ASSERT(JSID_IS_ATOM(id));
     JS_ASSERT(index < VAR_INDEX_LIMIT);
 
     *redeclared = false;
 
     /* Inline JSObject::addProperty in order to trap the redefinition case. */
     Shape **spp;
     if (Shape::search(cx, block->lastProperty(), id, &spp, true)) {
         *redeclared = true;
--- a/js/src/vm/ScopeObject.h
+++ b/js/src/vm/ScopeObject.h
@@ -485,20 +485,19 @@ class StaticBlockObject : public BlockOb
      * Frontend compilation temporarily uses the object's slots to link
      * a let var to its associated Definition parse node.
      */
     void setDefinitionParseNode(unsigned i, frontend::Definition *def) {
         JS_ASSERT(slotValue(i).isUndefined());
         setSlotValue(i, PrivateValue(def));
     }
 
-    frontend::Definition *maybeDefinitionParseNode(unsigned i) {
+    frontend::Definition *definitionParseNode(unsigned i) {
         Value v = slotValue(i);
-        return v.isUndefined() ? nullptr
-                               : reinterpret_cast<frontend::Definition *>(v.toPrivate());
+        return reinterpret_cast<frontend::Definition *>(v.toPrivate());
     }
 
     /*
      * While ScopeCoordinate can generally reference up to 2^24 slots, block objects have an
      * additional limitation that all slot indices must be storable as uint16_t short-ids in the
      * associated Shape. If we could remove the block dependencies on shape->shortid, we could
      * remove INDEX_LIMIT.
      */