Bug 1237449 - Finish cleaning up emitForOf's code now that it's not also implementing the spread operation. r=efaust

--HG--
extra : rebase_source : f18142304263d22c0486ae53d521af2e5d18afe6

js/src/frontend/BytecodeEmitter.cpp

@@ -5598,44 +5598,31 @@ BytecodeEmitter::emitSpread(bool allowSelfHosted)
 }
 
 bool
-BytecodeEmitter::emitForOf(StmtType type, ParseNode* pn)
+BytecodeEmitter::emitForOf(ParseNode* pn)
 {
-    MOZ_ASSERT(type == StmtType::FOR_OF_LOOP || type == StmtType::SPREAD);
-#ifdef DEBUG
-    if (type == StmtType::FOR_OF_LOOP) {
-        MOZ_ASSERT(pn);
-        MOZ_ASSERT(pn->pn_left->isKind(PNK_FOROF));
-    } else {
-        MOZ_ASSERT(!pn);
-    }
-#endif
+    MOZ_ASSERT(pn->pn_left->isKind(PNK_FOROF));
 
     ptrdiff_t top = offset();
-    ParseNode* forHead = pn ? pn->pn_left : nullptr;
-    ParseNode* forHeadExpr = forHead ? forHead->pn_kid3 : nullptr;
-    ParseNode* forBody = pn ? pn->pn_right : nullptr;
+    ParseNode* forHead = pn->pn_left;
 
-    ParseNode* loopDecl = forHead ? forHead->pn_kid1 : nullptr;
-    if (loopDecl && !emitForInOrOfVariables(loopDecl))
-        return false;
-
-    if (type == StmtType::FOR_OF_LOOP) {
-        // For-of loops run with two values on the stack: the iterator and the
-        // current result object.
-
-        // Compile the object expression to the right of 'of'.
-        if (!emitTree(forHeadExpr))
-            return false;
-        if (!emitIterator())
-            return false;
-
-        // Push a dummy result so that we properly enter iteration midstream.
-        if (!emit1(JSOP_UNDEFINED))                // ITER RESULT
+    if (ParseNode* loopDecl = forHead->pn_kid1) {
+        if (!emitForInOrOfVariables(loopDecl))
             return false;
     }
 
+    // Compile the expression to the right of 'of'.
+    ParseNode* forHeadExpr = forHead->pn_kid3;
+    if (!emitTree(forHeadExpr))                           // ITERABLE
+        return false;
+    if (!emitIterator())                                  // ITER
+        return false;
+
+    // Push a dummy result so that we properly enter iteration midstream.
+    if (!emit1(JSOP_UNDEFINED))                           // ITER RESULT
+        return false;
+
     LoopStmtInfo stmtInfo(cx);
-    pushLoopStatement(&stmtInfo, type, top);
+    pushLoopStatement(&stmtInfo, StmtType::FOR_OF_LOOP, top);
 
     // Jump down to the loop condition to minimize overhead assuming at least
     // one iteration, as the other loop forms do.  Annotate so IonMonkey can
@@ -5649,33 +5636,34 @@ BytecodeEmitter::emitForOf(StmtType type, ParseNode* pn)
 
     top = offset();
     stmtInfo.setTop(top);
-    if (!emitLoopHead(nullptr))
+    if (!emitLoopHead(nullptr))                           // ITER RESULT
         return false;
 
-    if (type == StmtType::SPREAD)
-        this->stackDepth++;
-
+    ptrdiff_t beq;
+    {
 #ifdef DEBUG
-    int loopDepth = this->stackDepth;
+        auto loopDepth = this->stackDepth;
 #endif
 
-    // Emit code to assign result.value to the iteration variable.
-    if (type == StmtType::FOR_OF_LOOP) {
+        // Emit code to assign result.value to the iteration variable.
         if (!emit1(JSOP_DUP))                             // ITER RESULT RESULT
             return false;
-    }
-    if (!emitAtomOp(cx->names().value, JSOP_GETPROP))     // ... RESULT VALUE
-        return false;
-    if (type == StmtType::FOR_OF_LOOP) {
-        if (!emitAssignment(forHead->pn_kid2, JSOP_NOP, nullptr)) // ITER RESULT VALUE
+        if (!emitAtomOp(cx->names().value, JSOP_GETPROP)) // ITER RESULT VALUE
+            return false;
+
+        ParseNode* forTarget = forHead->pn_kid2;
+        if (!emitAssignment(forTarget, JSOP_NOP, nullptr))// ITER RESULT VALUE
             return false;
         if (!emit1(JSOP_POP))                             // ITER RESULT
             return false;
 
         // The stack should be balanced around the assignment opcode sequence.
-        MOZ_ASSERT(this->stackDepth == loopDepth);
+        MOZ_ASSERT(this->stackDepth == loopDepth,
+                   "the stack must be balanced around the assignment "
+                   "operation");
 
         // Emit code for the loop body.
+        ParseNode* forBody = pn->pn_right;
         if (!emitTree(forBody))
             return false;
 
@@ -5684,41 +5672,29 @@ BytecodeEmitter::emitForOf(StmtType type, ParseNode* pn)
         do {
             stmt->update = offset();
         } while ((stmt = stmt->enclosing) != nullptr && stmt->type == StmtType::LABEL);
-    } else {
-        if (!emit1(JSOP_INITELEM_INC))                    // ITER ARR (I+1)
+
+        // COME FROM the beginning of the loop to here.
+        setJumpOffsetAt(jmp);
+        if (!emitLoopEntry(forHeadExpr))
             return false;
 
-        MOZ_ASSERT(this->stackDepth == loopDepth - 1);
-
-        // StmtType::SPREAD never contain continue, so do not set "update" offset.
-    }
-
-    // COME FROM the beginning of the loop to here.
-    setJumpOffsetAt(jmp);
-    if (!emitLoopEntry(forHeadExpr))
-        return false;
-
-    if (type == StmtType::FOR_OF_LOOP) {
         if (!emit1(JSOP_POP))                             // ITER
             return false;
         if (!emit1(JSOP_DUP))                             // ITER ITER
             return false;
-    } else {
-        if (!emitDupAt(2))                                // ITER ARR I ITER
+
+        if (!emitIteratorNext(forHead))                   // ITER RESULT
             return false;
+        if (!emit1(JSOP_DUP))                             // ITER RESULT RESULT
+            return false;
+        if (!emitAtomOp(cx->names().done, JSOP_GETPROP))  // ITER RESULT DONE?
+            return false;
+
+        if (!emitJump(JSOP_IFEQ, top - offset(), &beq))   // ITER RESULT
+            return false;
+
+        MOZ_ASSERT(this->stackDepth == loopDepth);
     }
-    if (!emitIteratorNext(forHead))                       // ... RESULT
-        return false;
-    if (!emit1(JSOP_DUP))                                 // ... RESULT RESULT
-        return false;
-    if (!emitAtomOp(cx->names().done, JSOP_GETPROP))      // ... RESULT DONE?
-        return false;
-
-    ptrdiff_t beq;
-    if (!emitJump(JSOP_IFEQ, top - offset(), &beq))       // ... RESULT
-        return false;
-
-    MOZ_ASSERT(this->stackDepth == loopDepth);
 
     // Let Ion know where the closing jump of this loop is.
     if (!setSrcNoteOffset(noteIndex, 0, beq - jmp))
@@ -5731,13 +5707,7 @@ BytecodeEmitter::emitForOf(StmtType type, ParseNode* pn)
     if (!tryNoteList.append(JSTRY_FOR_OF, stackDepth, top, offset()))
         return false;
 
-    if (type == StmtType::SPREAD) {
-        if (!emit2(JSOP_PICK, 3))      // ARR I RESULT ITER
-            return false;
-    }
-
-    // Pop the result and the iter.
-    return emitUint16Operand(JSOP_POPN, 2);
+    return emitUint16Operand(JSOP_POPN, 2);               //
 }
 
 bool
@@ -5751,7 +5721,7 @@ BytecodeEmitter::emitForIn(ParseNode* pn)
     if (loopDecl && !emitForInOrOfVariables(loopDecl))
         return false;
 
-    /* Compile the object expression to the right of 'in'. */
+    // Evaluate the expression to the right of 'in'.
     if (!emitTree(forHead->pn_kid3))
         return false;
 
@@ -6038,7 +6008,7 @@ BytecodeEmitter::emitFor(ParseNode* pn)
         return emitForIn(pn);
 
     MOZ_ASSERT(pn->pn_left->isKind(PNK_FOROF));
-    return emitForOf(StmtType::FOR_OF_LOOP, pn);
+    return emitForOf(pn);
 }
 
 bool
@@ -6097,7 +6067,7 @@ BytecodeEmitter::emitComprehensionForOf(ParseNode* pn)
     // For-of loops run with two values on the stack: the iterator and the
     // current result object.
 
-    // Compile the object expression to the right of 'of'.
+    // Evaluate the expression to the right of 'of'.
     if (!emitTree(forHeadExpr))                // EXPR
         return false;
     if (!emitIterator())                       // ITER
@@ -6219,7 +6189,7 @@ BytecodeEmitter::emitComprehensionForIn(ParseNode* pn)
     if (loopDecl && !emitComprehensionForInOrOfVariables(loopDecl, &letBlockScope))
         return false;
 
-    /* Compile the object expression to the right of 'in'. */
+    // Evaluate the expression to the right of 'in'.
     if (!emitTree(forHead->pn_kid3))
         return false;
 

js/src/frontend/BytecodeEmitter.h

@@ -638,6 +638,8 @@ struct BytecodeEmitter
     bool pushInitialConstants(JSOp op, unsigned n);
     bool initializeBlockScopedLocalsFromStack(Handle<StaticBlockObject*> blockObj);
 
+    // Emit bytecode for the spread operator.
+    //
     // emitSpread expects the current index (I) of the array, the array itself
     // and the iterator to be on the stack in that order (iterator on the bottom).
     // It will pop the iterator and I, then iterate over the iterator by calling
@@ -646,15 +648,9 @@ struct BytecodeEmitter
     // iteration count). The stack after iteration will look like |ARRAY INDEX|.
     bool emitSpread(bool allowSelfHosted = false);
 
-    // If type is StmtType::FOR_OF_LOOP, emit bytecode for a for-of loop.
-    // pn should be PNK_FOR, and pn->pn_left should be PNK_FOROF.
-    //
-    // If type is StmtType::SPREAD, emit bytecode for spread operator.
-    // pn should be nullptr.
-    //
-    // Please refer the comment above emitSpread for additional information about
-    // stack convention.
-    bool emitForOf(StmtType type, ParseNode* pn);
+    // Emit bytecode for a for-of loop.  pn should be PNK_FOR, and pn->pn_left
+    // should be PNK_FOROF.
+    bool emitForOf(ParseNode* pn);
 
     bool emitClass(ParseNode* pn);
     bool emitSuperPropLHS(ParseNode* superBase, bool isCall = false);