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 187122 879e31a2f667470cacf32ba0cdc2a3b5fa72dd9e
parent 187121 18183f6ef0d89a43ec0c7f69a638cf845cc30495
child 187123 343aac1bb02ead4549e25569dcb2d5623550a13a
push id3503
push userraliiev@mozilla.com
push dateMon, 28 Apr 2014 18:51:11 +0000
treeherdermozilla-beta@c95ac01e332e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswingo
bugs975162
milestone30.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 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.
      */