Bug 1147371: Implement IteratorClose for for-of

Issue #74

js/src/builtin/TypedArray.js

@@ -1428,7 +1428,7 @@ function TypedArrayStaticFrom(source, mapfn = undefined, thisArg = undefined) {
             // 22.2.2.1.1 IterableToList, step 4.a.
             var next = callContentFunction(iterator.next, iterator);
             if (!IsObject(next))
-                ThrowTypeError(JSMSG_NEXT_RETURNED_PRIMITIVE);
+                ThrowTypeError(JSMSG_ITER_METHOD_RETURNED_PRIMITIVE, "next");
 
             // 22.2.2.1.1 IterableToList, step 4.b.
             if (next.done)
@@ -1555,7 +1555,7 @@ function IterableToList(items, method) {
         // Step 4.a.
         var next = callContentFunction(iterator.next, iterator);
         if (!IsObject(next))
-            ThrowTypeError(JSMSG_NEXT_RETURNED_PRIMITIVE);
+            ThrowTypeError(JSMSG_ITER_METHOD_RETURNED_PRIMITIVE, "next");
 
         // Step 4.b.
         if (next.done)

js/src/frontend/BytecodeEmitter.cpp

@@ -58,6 +58,7 @@ using mozilla::Some;
 class BreakableControl;
 class LabelControl;
 class LoopControl;
+class ForOfLoopControl;
 class TryFinallyControl;
 
 static bool
@@ -150,6 +151,13 @@ BytecodeEmitter::NestableControl::is<LoopControl>() const
     return StatementKindIsLoop(kind_);
 }
 
+template <>
+bool
+BytecodeEmitter::NestableControl::is<ForOfLoopControl>() const
+{
+    return kind_ == StatementKind::ForOfLoop;
+}
+
 template <>
 bool
 BytecodeEmitter::NestableControl::is<TryFinallyControl>() const
@@ -270,6 +278,64 @@ class LoopControl : public BreakableControl
     }
 };
 
+class ForOfLoopControl : public LoopControl
+{
+    // The stack depth of the iterator.
+    int32_t iterDepth_;
+
+    // for-of loops, when throwing from non-iterator code (i.e. from the body
+    // or from evaluating the LHS of the loop condition), need to call
+    // IteratorClose. If IteratorClose itself throws, we must not re-call
+    // IteratorClose. Since non-local jumps like break and return call
+    // IteratorClose, whenever a non-local jump is emitted, we must terminate
+    // the current JSTRY_ITERCLOSE note to skip the non-local jump code, then
+    // start a new one.
+    //
+    // Visually,
+    //
+    //   for (x of y) {
+    //     ...             instantiate ForOfLoopControl
+    //     ...         +   <-- iterCloseTryStart_ points to right before
+    //     ...                 assignment to loop variable
+    //     ...         ^
+    //     ...         |
+    //     if (...)    v
+    //                 +   call finishIterCloseTryNote before |break|
+    //                     above range is noted with JSTRY_ITERCLOSE
+    //
+    //       break;    <-- break and IteratorClose are not inside
+    //                     JSTRY_ITERCLOSE note
+    //
+    //                     call startNewIterCloseTryNote after |break|
+    //                 +   <-- next iterCloseTryStart_ points here
+    //     ...         |
+    //     ...         ~
+    //   }
+    ptrdiff_t iterCloseTryStart_;
+
+  public:
+    ForOfLoopControl(BytecodeEmitter* bce, int32_t iterDepth)
+      : LoopControl(bce, StatementKind::ForOfLoop),
+        iterDepth_(iterDepth),
+        iterCloseTryStart_(-1)
+    {
+        MOZ_ASSERT(bce->stackDepth >= iterDepth);
+    }
+
+    MOZ_MUST_USE bool finishIterCloseTryNote(BytecodeEmitter* bce) {
+        ptrdiff_t end = bce->offset();
+        MOZ_ASSERT(end >= iterCloseTryStart_);
+        if (end != iterCloseTryStart_)
+            return bce->tryNoteList.append(JSTRY_ITERCLOSE, iterDepth_, iterCloseTryStart_, end);
+        return true;
+    }
+
+    void startNewIterCloseTryNote(BytecodeEmitter* bce) {
+        MOZ_ASSERT(bce->offset() > iterCloseTryStart_);
+        iterCloseTryStart_ = bce->offset();
+    }
+};
+
 class TryFinallyControl : public BytecodeEmitter::NestableControl
 {
     bool emittingSubroutine_;
@@ -1497,6 +1563,156 @@ BytecodeEmitter::TDZCheckCache::noteTDZCheck(BytecodeEmitter* bce, JSAtom* name,
     return true;
 }
 
+class MOZ_STACK_CLASS IfThenElseEmitter
+{
+    BytecodeEmitter* bce_;
+    JumpList jumpAroundThen_;
+    JumpList jumpsAroundElse_;
+    unsigned noteIndex_;
+    int32_t thenDepth_;
+#ifdef DEBUG
+    int32_t pushed_;
+    bool calculatedPushed_;
+#endif
+    enum State {
+        Start,
+        If,
+        Cond,
+        IfElse,
+        Else,
+        End
+    };
+    State state_;
+
+  public:
+    explicit IfThenElseEmitter(BytecodeEmitter* bce)
+      : bce_(bce),
+        noteIndex_(-1),
+        thenDepth_(0),
+#ifdef DEBUG
+        pushed_(0),
+        calculatedPushed_(false),
+#endif
+        state_(Start)
+    {}
+
+    ~IfThenElseEmitter()
+    {}
+
+  private:
+    bool emitIf(State nextState) {
+        MOZ_ASSERT(state_ == Start || state_ == Else);
+        MOZ_ASSERT(nextState == If || nextState == IfElse || nextState == Cond);
+
+        // Clear jumpAroundThen_ offset that points previous JSOP_IFEQ.
+        if (state_ == Else)
+            jumpAroundThen_ = JumpList();
+
+        // Emit an annotated branch-if-false around the then part.
+        SrcNoteType type = nextState == If ? SRC_IF : nextState == IfElse ? SRC_IF_ELSE : SRC_COND;
+        if (!bce_->newSrcNote(type, &noteIndex_))
+            return false;
+        if (!bce_->emitJump(JSOP_IFEQ, &jumpAroundThen_))
+            return false;
+
+        // To restore stack depth in else part, save depth of the then part.
+#ifdef DEBUG
+        // If DEBUG, this is also necessary to calculate |pushed_|.
+        thenDepth_ = bce_->stackDepth;
+#else
+        if (nextState == IfElse || nextState == Cond)
+            thenDepth_ = bce_->stackDepth;
+#endif
+        state_ = nextState;
+        return true;
+    }
+
+  public:
+    bool emitIf() {
+        return emitIf(If);
+    }
+
+    bool emitCond() {
+        return emitIf(Cond);
+    }
+
+    bool emitIfElse() {
+        return emitIf(IfElse);
+    }
+
+    bool emitElse() {
+        MOZ_ASSERT(state_ == IfElse || state_ == Cond);
+
+        calculateOrCheckPushed();
+
+        // Emit a jump from the end of our then part around the else part. The
+        // patchJumpsToTarget call at the bottom of this function will fix up
+        // the offset with jumpsAroundElse value.
+        if (!bce_->emitJump(JSOP_GOTO, &jumpsAroundElse_))
+            return false;
+
+        // Ensure the branch-if-false comes here, then emit the else.
+        if (!bce_->emitJumpTargetAndPatch(jumpAroundThen_))
+            return false;
+
+        // Annotate SRC_IF_ELSE or SRC_COND with the offset from branch to
+        // jump, for IonMonkey's benefit.  We can't just "back up" from the pc
+        // of the else clause, because we don't know whether an extended
+        // jump was required to leap from the end of the then clause over
+        // the else clause.
+        if (!bce_->setSrcNoteOffset(noteIndex_, 0,
+                                    jumpsAroundElse_.offset - jumpAroundThen_.offset))
+        {
+            return false;
+        }
+
+        // Restore stack depth of the then part.
+        bce_->stackDepth = thenDepth_;
+        state_ = Else;
+        return true;
+    }
+
+    bool emitEnd() {
+        MOZ_ASSERT(state_ == If || state_ == Else);
+
+        calculateOrCheckPushed();
+
+        if (state_ == If) {
+            // No else part, fixup the branch-if-false to come here.
+            if (!bce_->emitJumpTargetAndPatch(jumpAroundThen_))
+                return false;
+        }
+
+        // Patch all the jumps around else parts.
+        if (!bce_->emitJumpTargetAndPatch(jumpsAroundElse_))
+            return false;
+
+        state_ = End;
+        return true;
+    }
+
+    void calculateOrCheckPushed() {
+#ifdef DEBUG
+        if (!calculatedPushed_) {
+            pushed_ = bce_->stackDepth - thenDepth_;
+            calculatedPushed_ = true;
+        } else {
+            MOZ_ASSERT(pushed_ == bce_->stackDepth - thenDepth_);
+        }
+#endif
+    }
+
+#ifdef DEBUG
+    int32_t pushed() const {
+        return pushed_;
+    }
+
+    int32_t popped() const {
+        return -pushed_;
+    }
+#endif
+};
+
 BytecodeEmitter::BytecodeEmitter(BytecodeEmitter* parent,
                                  Parser<FullParseHandler>* parser, SharedContext* sc,
                                  HandleScript script, Handle<LazyScript*> lazyScript,
@@ -2013,22 +2229,43 @@ BytecodeEmitter::flushPops(int* npops)
 
 namespace {
 
-class NonLocalExitControl {
+class NonLocalExitControl
+{
+  public:
+    enum Kind
+    {
+        // IteratorClose is handled especially inside the exception unwinder.
+        Throw,
+
+        // A 'continue' statement does not call IteratorClose for the loop it
+        // is continuing, i.e. excluding the target loop.
+        Continue,
+
+        // A 'break' or 'return' statement does call IteratorClose for the
+        // loop it is breaking out of or returning from, i.e. including the
+        // target loop.
+        Break,
+        Return
+    };
+
+  private:
     BytecodeEmitter* bce_;
     const uint32_t savedScopeNoteIndex_;
     const int savedDepth_;
     uint32_t openScopeNoteIndex_;
+    Kind kind_;
 
     NonLocalExitControl(const NonLocalExitControl&) = delete;
 
     MOZ_MUST_USE bool leaveScope(BytecodeEmitter::EmitterScope* scope);
 
   public:
-    explicit NonLocalExitControl(BytecodeEmitter* bce)
+    NonLocalExitControl(BytecodeEmitter* bce, Kind kind)
       : bce_(bce),
         savedScopeNoteIndex_(bce->scopeNoteList.length()),
         savedDepth_(bce->stackDepth),
-        openScopeNoteIndex_(bce->innermostEmitterScope->noteIndex())
+        openScopeNoteIndex_(bce->innermostEmitterScope->noteIndex()),
+        kind_(kind)
     { }
 
     ~NonLocalExitControl() {
@@ -2075,6 +2312,14 @@ NonLocalExitControl::prepareForNonLocalJump(BytecodeEmitter::NestableControl* ta
 
     EmitterScope* es = bce_->innermostEmitterScope;
     int npops = 0;
+    bool hasForOfLoopsWithIteratorClose = false;
+
+    // IteratorClose is handled specially in the exception unwinder. For
+    // 'continue', 'break', and 'return' statements, emit IteratorClose
+    // bytecode inline. 'continue' statements do not call IteratorClose for
+    // the loop they are continuing.
+    bool emitIteratorClose = kind_ == Continue || kind_ == Break || kind_ == Return;
+    bool emitIteratorCloseAtTarget = emitIteratorClose && kind_ != Continue;
 
     auto flushPops = [&npops](BytecodeEmitter* bce) {
         if (npops && !bce->flushPops(&npops))
@@ -2114,15 +2359,29 @@ NonLocalExitControl::prepareForNonLocalJump(BytecodeEmitter::NestableControl* ta
           }
 
           case StatementKind::ForOfLoop:
-            npops += 2;
+            // The iterator and the current value are on the stack.
+            //
+            if (emitIteratorClose) {
+                hasForOfLoopsWithIteratorClose = true;
+                if (!control->as<ForOfLoopControl>().finishIterCloseTryNote(bce_))
+                    return false;
+                if (!bce_->emit1(JSOP_POP))               // ... ITER
+                    return false;
+                if (!bce_->emitIteratorClose())           // ...
+                    return false;
+            } else {
+                if (!bce_->emit1(JSOP_POP))               // ... ITER
+                    return false;
+                if (!bce_->emit1(JSOP_POP))               // ...
+                    return false;
+            }
             break;
 
           case StatementKind::ForInLoop:
-            /* The iterator and the current value are on the stack. */
+            // The iterator and the current value are on the stack.
-            npops += 1;
+            if (!bce_->emit1(JSOP_POP))                   // ... ITER
-            if (!flushPops(bce_))
                 return false;
-            if (!bce_->emit1(JSOP_ENDITER))
+            if (!bce_->emit1(JSOP_ENDITER))               // ...
                 return false;
             break;
 
@@ -2131,13 +2390,45 @@ NonLocalExitControl::prepareForNonLocalJump(BytecodeEmitter::NestableControl* ta
         }
     }
 
+    if (target && target->is<ForOfLoopControl>() && emitIteratorCloseAtTarget) {
+        hasForOfLoopsWithIteratorClose = true;
+        if (!target->as<ForOfLoopControl>().finishIterCloseTryNote(bce_))
+            return false;
+
+        // The iterator and the current value are on the stack. At the level
+        // of the target block, there's bytecode after the loop that will pop
+        // the iterator and the value, so duplicate the iterator and call
+        // IteratorClose.
+        if (!bce_->emitDupAt(1))                          // ... ITER RESULT ITER
+            return false;
+        if (!bce_->emitIteratorClose())                   // ... ITER RESULT
+            return false;
+    }
+
     EmitterScope* targetEmitterScope = target ? target->emitterScope() : bce_->varEmitterScope;
     for (; es != targetEmitterScope; es = es->enclosingInFrame()) {
         if (!leaveScope(es))
             return false;
     }
 
-    return flushPops(bce_);
+    if (!flushPops(bce_))
+        return false;
+
+    // See comment in ForOfLoopControl.
+    if (hasForOfLoopsWithIteratorClose) {
+        for (NestableControl* control = bce_->innermostNestableControl;
+             control != target;
+             control = control->enclosing())
+        {
+            if (control->is<ForOfLoopControl>())
+                control->as<ForOfLoopControl>().startNewIterCloseTryNote(bce_);
+        }
+
+        if (target && target->is<ForOfLoopControl>() && emitIteratorCloseAtTarget)
+            target->as<ForOfLoopControl>().startNewIterCloseTryNote(bce_);
+    }
+
+    return true;
 }
 
 }  // anonymous namespace
@@ -2145,7 +2436,9 @@ NonLocalExitControl::prepareForNonLocalJump(BytecodeEmitter::NestableControl* ta
 bool
 BytecodeEmitter::emitGoto(NestableControl* target, JumpList* jumplist, SrcNoteType noteType)
 {
-    NonLocalExitControl nle(this);
+    NonLocalExitControl nle(this, noteType == SRC_CONTINUE
+                                  ? NonLocalExitControl::Continue
+                                  : NonLocalExitControl::Break);
 
     if (!nle.prepareForNonLocalJump(target))
         return false;
@@ -4529,6 +4822,144 @@ BytecodeEmitter::emitIteratorNext(ParseNode* pn, bool allowSelfHosted)
     return true;
 }
 
+bool
+BytecodeEmitter::emitIteratorClose(Maybe<JumpTarget> yieldStarTryStart, bool allowSelfHosted)
+{
+    MOZ_ASSERT(allowSelfHosted || emitterMode != BytecodeEmitter::SelfHosting,
+               ".close() on iterators is prohibited in self-hosted code because it "
+               "can run user-modifiable iteration code");
+
+    // Generate inline logic corresponding to IteratorClose (ES 7.4.6).
+    //
+    // Callers need to ensure that the iterator object is at the top of the
+    // stack.
+
+    if (!emit1(JSOP_DUP))                                 // ... ITER ITER
+        return false;
+
+    // Step 3.
+    //
+    // Get the "return" method.
+    if (!emitAtomOp(cx->names().return_, JSOP_CALLPROP))  // ... ITER RET
+        return false;
+
+    // Step 4.
+    //
+    // Do nothing if "return" is null or undefined.
+    IfThenElseEmitter ifReturnMethodIsDefined(this);
+    if (!emit1(JSOP_DUP))                                 // ... ITER RET RET
+        return false;
+    if (!emit1(JSOP_UNDEFINED))                           // ... ITER RET RET UNDEFINED
+        return false;
+    if (!emit1(JSOP_NE))                                  // ... ITER RET ?NEQL
+        return false;
+    if (!ifReturnMethodIsDefined.emitIfElse())
+        return false;
+
+    // Steps 5, 8.
+    //
+    // Call "return" if it is not undefined or null, and check that it returns
+    // an Object.
+    if (!emit1(JSOP_SWAP))                                // ... RET ITER
+        return false;
+
+    // ES 14.4.13, yield * AssignmentExpression, step 5.c
+    //
+    // When emitting iterator.return() for yield* forced return, we need to
+    // pass the argument passed to Generator.prototype.return to the return
+    // method.
+    if (yieldStarTryStart) {
+        IfThenElseEmitter ifGeneratorClosing(this);
+        if (!emitDupAt(2))                                // ... FTYPE FVALUE RET ITER FVALUE
+            return false;
+        if (!emit1(JSOP_ISGENCLOSING))                    // ... FTYPE FVALUE RET ITER FVALUE CLOSING
+            return false;
+        if (!emit1(JSOP_SWAP))                            // ... FTYPE FVALUE RET ITER CLOSING FVALUE
+            return false;
+        if (!emit1(JSOP_POP))                             // ... FTYPE FVALUE RET ITER CLOSING
+            return false;
+        if (!ifGeneratorClosing.emitIfElse())             // ... FTYPE FVALUE RET ITER
+            return false;
+
+        if (!emit1(JSOP_GETRVAL))                         // ... FTYPE FVALUE RET ITER RVAL
+            return false;
+        if (!emitAtomOp(cx->names().value, JSOP_GETPROP)) // ... FTYPE FVALUE RET ITER VALUE
+            return false;
+        if (!emitCall(JSOP_CALL, 1))                      // ... FTYPE FVALUE RESULT
+            return false;
+        checkTypeSet(JSOP_CALL);
+        if (!emitCheckIsObj(CheckIsObjectKind::IteratorReturn)) // ... FTYPE FVALUE RESULT
+            return false;
+
+        IfThenElseEmitter ifReturnDone(this);
+        if (!emit1(JSOP_DUP))                             // ITER OLDRESULT FTYPE FVALUE RESULT RESULT
+            return false;
+        if (!emitAtomOp(cx->names().done, JSOP_GETPROP))  // ITER OLDRESULT FTYPE FVALUE RESULT DONE
+            return false;
+        if (!ifReturnDone.emitIfElse())                   // ITER OLDRESULT FTYPE FVALUE RESULT
+            return false;
+        if (!emit1(JSOP_DUP))                             // ITER OLDRESULT FTYPE FVALUE RESULT RESULT
+            return false;
+        if (!emit1(JSOP_SETRVAL))                         // ITER OLDRESULT FTYPE FVALUE RESULT
+            return false;
+        if (!ifReturnDone.emitElse())                     // ITER OLDRESULT FTYPE FVALUE RESULT
+            return false;
+        int32_t savedDepth = this->stackDepth;
+        if (!emit2(JSOP_UNPICK, 3))                       // ITER RESULT OLDRESULT FTYPE FVALUE
+            return false;
+        if (!emitUint16Operand(JSOP_POPN, 3))             // ITER RESULT
+            return false;
+        JumpList beq;
+        JumpTarget breakTarget{ -1 };
+        if (!emitBackwardJump(JSOP_GOTO, *yieldStarTryStart, &beq, &breakTarget)) // ITER RESULT
+            return false;
+        this->stackDepth = savedDepth;
+        if (!ifReturnDone.emitEnd())
+            return false;
+
+        if (!ifGeneratorClosing.emitElse())               // ... FTYPE FVALUE RET ITER
+            return false;
+        if (!emitCall(JSOP_CALL, 0))                      // ... FTYPE FVALUE RESULT
+            return false;
+        checkTypeSet(JSOP_CALL);
+        if (!emitCheckIsObj(CheckIsObjectKind::IteratorReturn)) // ... FTYPE FVALUE RESULT
+            return false;
+
+        if (!ifGeneratorClosing.emitEnd())
+            return false;
+    } else {
+        if (!emitCall(JSOP_CALL, 0))                      // ... RESULT
+            return false;
+        checkTypeSet(JSOP_CALL);
+        if (!emitCheckIsObj(CheckIsObjectKind::IteratorReturn)) // ... RESULT
+            return false;
+    }
+
+    if (!ifReturnMethodIsDefined.emitElse())
+        return false;
+    if (!emit1(JSOP_POP))                                 // ... ITER
+        return false;
+    if (!ifReturnMethodIsDefined.emitEnd())
+        return false;
+
+    return emit1(JSOP_POP);                               // ...
+}
+
+template <typename InnerEmitter>
+bool
+BytecodeEmitter::wrapWithIteratorCloseTryNote(int32_t iterDepth, InnerEmitter emitter)
+{
+    MOZ_ASSERT(this->stackDepth >= iterDepth);
+
+    ptrdiff_t start = offset();
+    if (!emitter(this))
+        return false;
+    ptrdiff_t end = offset();
+    if (start != end)
+        return tryNoteList.append(JSTRY_ITERCLOSE, iterDepth, start, end);
+    return true;
+}
+
 bool
 BytecodeEmitter::emitDefault(ParseNode* defaultExpr, ParseNode* pattern)
 {
@@ -4617,156 +5048,6 @@ BytecodeEmitter::emitInitializerInBranch(ParseNode* initializer, ParseNode* patt
     return emitInitializer(initializer, pattern);
 }
 
-class MOZ_STACK_CLASS IfThenElseEmitter
-{
-    BytecodeEmitter* bce_;
-    JumpList jumpAroundThen_;
-    JumpList jumpsAroundElse_;
-    unsigned noteIndex_;
-    int32_t thenDepth_;
-#ifdef DEBUG
-    int32_t pushed_;
-    bool calculatedPushed_;
-#endif
-    enum State {
-        Start,
-        If,
-        Cond,
-        IfElse,
-        Else,
-        End
-    };
-    State state_;
-
-  public:
-    explicit IfThenElseEmitter(BytecodeEmitter* bce)
-      : bce_(bce),
-        noteIndex_(-1),
-        thenDepth_(0),
-#ifdef DEBUG
-        pushed_(0),
-        calculatedPushed_(false),
-#endif
-        state_(Start)
-    {}
-
-    ~IfThenElseEmitter()
-    {}
-
-  private:
-    bool emitIf(State nextState) {
-        MOZ_ASSERT(state_ == Start || state_ == Else);
-        MOZ_ASSERT(nextState == If || nextState == IfElse || nextState == Cond);
-
-        // Clear jumpAroundThen_ offset that points previous JSOP_IFEQ.
-        if (state_ == Else)
-            jumpAroundThen_ = JumpList();
-
-        // Emit an annotated branch-if-false around the then part.
-        SrcNoteType type = nextState == If ? SRC_IF : nextState == IfElse ? SRC_IF_ELSE : SRC_COND;
-        if (!bce_->newSrcNote(type, &noteIndex_))
-            return false;
-        if (!bce_->emitJump(JSOP_IFEQ, &jumpAroundThen_))
-            return false;
-
-        // To restore stack depth in else part, save depth of the then part.
-#ifdef DEBUG
-        // If DEBUG, this is also necessary to calculate |pushed_|.
-        thenDepth_ = bce_->stackDepth;
-#else
-        if (nextState == IfElse || nextState == Cond)
-            thenDepth_ = bce_->stackDepth;
-#endif
-        state_ = nextState;
-        return true;
-    }
-
-  public:
-    bool emitIf() {
-        return emitIf(If);
-    }
-
-    bool emitCond() {
-        return emitIf(Cond);
-    }
-
-    bool emitIfElse() {
-        return emitIf(IfElse);
-    }
-
-    bool emitElse() {
-        MOZ_ASSERT(state_ == IfElse || state_ == Cond);
-
-        calculateOrCheckPushed();
-
-        // Emit a jump from the end of our then part around the else part. The
-        // patchJumpsToTarget call at the bottom of this function will fix up
-        // the offset with jumpsAroundElse value.
-        if (!bce_->emitJump(JSOP_GOTO, &jumpsAroundElse_))
-            return false;
-
-        // Ensure the branch-if-false comes here, then emit the else.
-        if (!bce_->emitJumpTargetAndPatch(jumpAroundThen_))
-            return false;
-
-        // Annotate SRC_IF_ELSE or SRC_COND with the offset from branch to
-        // jump, for IonMonkey's benefit.  We can't just "back up" from the pc
-        // of the else clause, because we don't know whether an extended
-        // jump was required to leap from the end of the then clause over
-        // the else clause.
-        if (!bce_->setSrcNoteOffset(noteIndex_, 0,
-                                    jumpsAroundElse_.offset - jumpAroundThen_.offset))
-        {
-            return false;
-        }
-
-        // Restore stack depth of the then part.
-        bce_->stackDepth = thenDepth_;
-        state_ = Else;
-        return true;
-    }
-
-    bool emitEnd() {
-        MOZ_ASSERT(state_ == If || state_ == Else);
-
-        calculateOrCheckPushed();
-
-        if (state_ == If) {
-            // No else part, fixup the branch-if-false to come here.
-            if (!bce_->emitJumpTargetAndPatch(jumpAroundThen_))
-                return false;
-        }
-
-        // Patch all the jumps around else parts.
-        if (!bce_->emitJumpTargetAndPatch(jumpsAroundElse_))
-            return false;
-
-        state_ = End;
-        return true;
-    }
-
-    void calculateOrCheckPushed() {
-#ifdef DEBUG
-        if (!calculatedPushed_) {
-            pushed_ = bce_->stackDepth - thenDepth_;
-            calculatedPushed_ = true;
-        } else {
-            MOZ_ASSERT(pushed_ == bce_->stackDepth - thenDepth_);
-        }
-#endif
-    }
-
-#ifdef DEBUG
-    int32_t pushed() const {
-        return pushed_;
-    }
-
-    int32_t popped() const {
-        return -pushed_;
-    }
-#endif
-};
-
 bool
 BytecodeEmitter::emitDestructuringOpsArray(ParseNode* pattern, DestructuringFlavor flav)
 {
@@ -5710,7 +5991,7 @@ BytecodeEmitter::emitCatch(ParseNode* pn)
             return false;
 
         {
-            NonLocalExitControl nle(this);
+            NonLocalExitControl nle(this, NonLocalExitControl::Throw);
 
             // Move exception back to cx->exception to prepare for
             // the next catch.
@@ -6303,12 +6584,14 @@ BytecodeEmitter::emitForOf(ParseNode* forOfLoop, EmitterScope* headLexicalEmitte
     if (!emitIterator())                                  // ITER
         return false;
 
+    int32_t iterDepth = stackDepth;
+
     // For-of loops have both the iterator and the value on the stack. Push
     // undefined to balance the stack.
     if (!emit1(JSOP_UNDEFINED))                           // ITER RESULT
         return false;
 
-    LoopControl loopInfo(this, StatementKind::ForOfLoop);
+    ForOfLoopControl loopInfo(this, iterDepth);
 
     // Annotate so IonMonkey can find the loop-closing jump.
     unsigned noteIndex;
@@ -6354,18 +6637,23 @@ BytecodeEmitter::emitForOf(ParseNode* forOfLoop, EmitterScope* headLexicalEmitte
 #endif
 
         // Emit code to assign result.value to the iteration variable.
+        //
+        // Note that ES 13.7.5.13, step 5.c says getting result.value does not
+        // call IteratorClose, so start JSTRY_ITERCLOSE after the GETPROP.
         if (!emit1(JSOP_DUP))                             // ITER RESULT RESULT
             return false;
         if (!emitAtomOp(cx->names().value, JSOP_GETPROP)) // ITER RESULT VALUE
             return false;
 
+        loopInfo.startNewIterCloseTryNote(this);
+
         if (!emitInitializeForInOrOfTarget(forOfHead))    // ITER RESULT VALUE
             return false;
 
         if (!emit1(JSOP_POP))                             // ITER RESULT
             return false;
 
-        MOZ_ASSERT(this->stackDepth == loopDepth,
+        MOZ_ASSERT(stackDepth == loopDepth,
                    "the stack must be balanced around the initializing "
                    "operation");
 
@@ -6374,6 +6662,9 @@ BytecodeEmitter::emitForOf(ParseNode* forOfLoop, EmitterScope* headLexicalEmitte
         if (!emitTree(forBody))                           // ITER RESULT
             return false;
 
+        if (!loopInfo.finishIterCloseTryNote(this))
+            return false;
+
         // Set offset for continues.
         loopInfo.continueTarget = { offset() };
 
@@ -7609,7 +7900,7 @@ BytecodeEmitter::emitReturn(ParseNode* pn)
             return false;
     }
 
-    NonLocalExitControl nle(this);
+    NonLocalExitControl nle(this, NonLocalExitControl::Return);
 
     if (!nle.prepareForNonLocalJumpToOutermost())
         return false;

js/src/frontend/BytecodeEmitter.h

@@ -679,6 +679,12 @@ struct MOZ_STACK_CLASS BytecodeEmitter
     // Pops iterator from the top of the stack. Pushes the result of |.next()|
     // onto the stack.
     MOZ_MUST_USE bool emitIteratorNext(ParseNode* pn, bool allowSelfHosted = false);
+    MOZ_MUST_USE bool emitIteratorClose(
+        mozilla::Maybe<JumpTarget> yieldStarTryStart = mozilla::Nothing(),
+        bool allowSelfHosted = false);
+
+    template <typename InnerEmitter>
+    MOZ_MUST_USE bool wrapWithIteratorCloseTryNote(int32_t iterDepth, InnerEmitter emitter);
 
     // Check if the value on top of the stack is "undefined". If so, replace
     // that value on the stack with the value defined by |defaultExpr|.

js/src/jit/JitFrames.cpp

@@ -328,11 +328,15 @@ NumArgAndLocalSlots(const InlineFrameIterator& frame)
 }
 
 static void
-CloseLiveIteratorIon(JSContext* cx, const InlineFrameIterator& frame, uint32_t stackSlot)
+CloseLiveIteratorIon(JSContext* cx, const InlineFrameIterator& frame, JSTryNote* tn)
 {
+    MOZ_ASSERT(tn->kind == JSTRY_FOR_IN || tn->kind == JSTRY_ITERCLOSE);
+    MOZ_ASSERT(tn->stackDepth > 0);
+
     SnapshotIterator si = frame.snapshotIterator();
 
     // Skip stack slots until we reach the iterator object.
+    uint32_t stackSlot = tn->stackDepth;
     uint32_t skipSlots = NumArgAndLocalSlots(frame) + stackSlot - 1;
 
     for (unsigned i = 0; i < skipSlots; i++)
@@ -341,10 +345,14 @@ CloseLiveIteratorIon(JSContext* cx, const InlineFrameIterator& frame, uint32_t s
     Value v = si.read();
     RootedObject obj(cx, &v.toObject());
 
-    if (cx->isExceptionPending())
+    if (cx->isExceptionPending()) {
-        UnwindIteratorForException(cx, obj);
+        if (tn->kind == JSTRY_FOR_IN)
-    else
+            UnwindIteratorForException(cx, obj);
+        else
+            IteratorCloseForException(cx, obj);
+    } else {
         UnwindIteratorForUncatchableException(cx, obj);
+    }
 }
 
 class IonFrameStackDepthOp
@@ -417,12 +425,13 @@ HandleExceptionIon(JSContext* cx, const InlineFrameIterator& frame, ResumeFromEx
         JSTryNote* tn = *tni;
 
         switch (tn->kind) {
-          case JSTRY_FOR_IN: {
+          case JSTRY_FOR_IN:
-            MOZ_ASSERT(JSOp(*(script->main() + tn->start + tn->length)) == JSOP_ENDITER);
+          case JSTRY_ITERCLOSE: {
+            MOZ_ASSERT_IF(tn->kind == JSTRY_FOR_IN,
+                          JSOp(*(script->main() + tn->start + tn->length)) == JSOP_ENDITER);
             MOZ_ASSERT(tn->stackDepth > 0);
 
-            uint32_t localSlot = tn->stackDepth;
+            CloseLiveIteratorIon(cx, frame, tn);
-            CloseLiveIteratorIon(cx, frame, localSlot);
             break;
           }
 
@@ -598,17 +607,23 @@ ProcessTryNotesBaseline(JSContext* cx, const JitFrameIterator& frame, Environmen
             return true;
           }
 
-          case JSTRY_FOR_IN: {
+          case JSTRY_FOR_IN:
+          case JSTRY_ITERCLOSE: {
             uint8_t* framePointer;
             uint8_t* stackPointer;
             BaselineFrameAndStackPointersFromTryNote(tn, frame, &framePointer, &stackPointer);
-            Value iterValue(*(Value*) stackPointer);
+            Value iterValue(*(reinterpret_cast<Value*>(stackPointer)));
             RootedObject iterObject(cx, &iterValue.toObject());
-            if (!UnwindIteratorForException(cx, iterObject)) {
+            bool ok;
+            if (tn->kind == JSTRY_FOR_IN)
+                ok = UnwindIteratorForException(cx, iterObject);
+            else
+                ok = IteratorCloseForException(cx, iterObject);
+            if (!ok) {
                 // See comment in the JSTRY_FOR_IN case in Interpreter.cpp's
                 // ProcessTryNotes.
                 SettleOnTryNote(cx, tn, frame, ei, rfe, pc);
-                MOZ_ASSERT(**pc == JSOP_ENDITER);
+                MOZ_ASSERT_IF(tn->kind == JSTRY_FOR_IN, **pc == JSOP_ENDITER);
                 return false;
             }
             break;

js/src/js.msg

@@ -96,7 +96,7 @@ MSG_DEF(JSMSG_NOT_ITERABLE,            1, JSEXN_TYPEERR, "{0} is not iterable")
 MSG_DEF(JSMSG_NOT_ITERATOR,            1, JSEXN_TYPEERR, "{0} is not iterator")
 MSG_DEF(JSMSG_ALREADY_HAS_PRAGMA,      2, JSEXN_WARN, "{0} is being assigned a {1}, but already has one")
 MSG_DEF(JSMSG_GET_ITER_RETURNED_PRIMITIVE, 0, JSEXN_TYPEERR, "[Symbol.iterator]() returned a non-object value")
-MSG_DEF(JSMSG_NEXT_RETURNED_PRIMITIVE, 0, JSEXN_TYPEERR, "iterator.next() returned a non-object value")
+MSG_DEF(JSMSG_ITER_METHOD_RETURNED_PRIMITIVE, 1, JSEXN_TYPEERR, "iterator.{0}() returned a non-object value")
 MSG_DEF(JSMSG_CANT_SET_PROTO,          0, JSEXN_TYPEERR, "can't set prototype of this object")
 MSG_DEF(JSMSG_CANT_SET_PROTO_OF,       1, JSEXN_TYPEERR, "can't set prototype of {0}")
 MSG_DEF(JSMSG_CANT_SET_PROTO_CYCLE,    0, JSEXN_TYPEERR, "can't set prototype: it would cause a prototype chain cycle")

js/src/jsiter.cpp

@@ -12,6 +12,7 @@
 #include "mozilla/Maybe.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/PodOperations.h"
+#include "mozilla/Unused.h"
 
 #include "jsarray.h"
 #include "jsatom.h"
@@ -1229,6 +1230,7 @@ js::CloseIterator(JSContext* cx, HandleObject obj)
         }
         return LegacyGeneratorObject::close(cx, obj);
     }
+
     return true;
 }
 
@@ -1246,6 +1248,56 @@ js::UnwindIteratorForException(JSContext* cx, HandleObject obj)
     return true;
 }
 
+bool
+js::IteratorCloseForException(JSContext* cx, HandleObject obj)
+{
+    MOZ_ASSERT(cx->isExceptionPending());
+
+    bool isClosingGenerator = cx->isClosingGenerator();
+    JS::AutoSaveExceptionState savedExc(cx);
+
+    // Implements IteratorClose (ES 7.4.6) for exception unwinding. See
+    // also the bytecode generated by BytecodeEmitter::emitIteratorClose.
+
+    // Step 3.
+    //
+    // Get the "return" method.
+    RootedValue returnMethod(cx);
+    if (!GetProperty(cx, obj, obj, cx->names().return_, &returnMethod))
+        return false;
+
+    // Step 4.
+    //
+    // Do nothing if "return" is null or undefined. Throw a TypeError if the
+    // method is not IsCallable.
+    if (returnMethod.isNullOrUndefined())
+        return true;
+    if (!IsCallable(returnMethod))
+        return ReportIsNotFunction(cx, returnMethod);
+
+    // Step 5, 6, 8.
+    //
+    // Call "return" if it is not null or undefined.
+    RootedValue rval(cx);
+    bool ok = Call(cx, returnMethod, obj, &rval);
+    if (isClosingGenerator) {
+        // Closing an iterator is implemented as an exception, but in spec
+        // terms it is a Completion value with [[Type]] return. In this case
+        // we *do* care if the call threw and if it returned an object.
+        if (!ok)
+            return false;
+        if (!rval.isObject())
+            return ThrowCheckIsObject(cx, CheckIsObjectKind::IteratorReturn);
+    } else {
+        // We don't care if the call threw or that it returned an Object, as
+        // Step 6 says if IteratorClose is being called during a throw, the
+        // original throw has primacy.
+        savedExc.restore();
+    }
+
+    return true;
+}
+
 void
 js::UnwindIteratorForUncatchableException(JSContext* cx, JSObject* obj)
 {

js/src/jsiter.h

@@ -189,6 +189,9 @@ CloseIterator(JSContext* cx, HandleObject iterObj);
 bool
 UnwindIteratorForException(JSContext* cx, HandleObject obj);
 
+bool
+IteratorCloseForException(JSContext* cx, HandleObject obj);
+
 void
 UnwindIteratorForUncatchableException(JSContext* cx, JSObject* obj);
 

js/src/jsscript.h

@@ -84,7 +84,8 @@ enum JSTryNoteKind {
     JSTRY_FINALLY,
     JSTRY_FOR_IN,
     JSTRY_FOR_OF,
-    JSTRY_LOOP
+    JSTRY_LOOP,
+    JSTRY_ITERCLOSE
 };
 
 /*

js/src/shell/js.cpp

@@ -2589,7 +2589,8 @@ JS_STATIC_ASSERT(JSTRY_CATCH == 0);
 JS_STATIC_ASSERT(JSTRY_FINALLY == 1);
 JS_STATIC_ASSERT(JSTRY_FOR_IN == 2);
 
-static const char* const TryNoteNames[] = { "catch", "finally", "for-in", "for-of", "loop" };
+static const char* const TryNoteNames[] = { "catch", "finally", "for-in", "for-of", "loop",
+                                            "iterclose" };
 
 static MOZ_MUST_USE bool
 TryNotes(JSContext* cx, HandleScript script, Sprinter* sp)
@@ -2597,15 +2598,15 @@ TryNotes(JSContext* cx, HandleScript script, Sprinter* sp)
     if (!script->hasTrynotes())
         return true;
 
-    if (sp->put("\nException table:\nkind      stack    start      end\n") < 0)
+    if (sp->put("\nException table:\nkind        stack    start      end\n") < 0)
         return false;
 
     JSTryNote* tn = script->trynotes()->vector;
     JSTryNote* tnlimit = tn + script->trynotes()->length;
     do {
         MOZ_ASSERT(tn->kind < ArrayLength(TryNoteNames));
-        uint8_t startOff = script->pcToOffset(script->main()) + tn->start;
+        uint32_t startOff = script->pcToOffset(script->main()) + tn->start;
-        if (!sp->jsprintf(" %-7s %6u %8u %8u\n",
+        if (!sp->jsprintf(" %-9s %6u %8u %8u\n",
                           TryNoteNames[tn->kind], tn->stackDepth,
                           startOff, startOff + tn->length))
         {

js/src/tests/ecma_6/Statements/for-of-iterator-close.js

@@ -0,0 +1,102 @@
+// Tests that IteratorReturn is called when a for-of loop has an abrupt
+// completion value during non-iterator code.
+
+function test() {
+    var returnCalled = 0;
+    var returnCalledExpected = 0;
+    var iterable = {};
+    iterable[Symbol.iterator] = makeIterator({
+        ret: function() {
+            returnCalled++;
+            return {};
+        }
+    });
+
+    // break calls iter.return
+    for (var x of iterable)
+        break;
+    assertEq(returnCalled, ++returnCalledExpected);
+
+    // throw in body calls iter.return
+    assertThrowsValue(function() {
+        for (var x of iterable)
+            throw "in body";
+    }, "in body");
+    assertEq(returnCalled, ++returnCalledExpected);
+
+    // throw in lhs ref calls iter.return
+    function throwlhs() {
+        throw "in lhs";
+    }
+    assertThrowsValue(function() {
+        for ((throwlhs()) of iterable)
+            continue;
+    }, "in lhs");
+    assertEq(returnCalled, ++returnCalledExpected);
+
+    // throw in iter.return doesn't re-call iter.return
+    iterable[Symbol.iterator] = makeIterator({
+        ret: function() {
+            returnCalled++;
+            throw "in iter.return";
+        }
+    });
+    assertThrowsValue(function() {
+        for (var x of iterable)
+            break;
+    }, "in iter.return");
+    assertEq(returnCalled, ++returnCalledExpected);
+
+    // throw in iter.next doesn't call IteratorClose
+    iterable[Symbol.iterator] = makeIterator({
+        next: function() {
+            throw "in next";
+        }
+    });
+    assertThrowsValue(function() {
+        for (var x of iterable)
+            break;
+    }, "in next");
+    assertEq(returnCalled, returnCalledExpected);
+
+    // "return" must return an Object.
+    iterable[Symbol.iterator] = makeIterator({
+        ret: function() {
+            returnCalled++;
+            return 42;
+        }
+    });
+    assertThrowsInstanceOf(function() {
+        for (var x of iterable)
+            break;
+    }, TypeError);
+    assertEq(returnCalled, ++returnCalledExpected);
+
+    // continue doesn't call iter.return for the loop it's continuing
+    var i = 0;
+    iterable[Symbol.iterator] = makeIterator({
+        next: function() {
+            return { done: i++ > 5 };
+        },
+        ret: function() {
+            returnCalled++;
+            return {};
+        }
+    });
+    for (var x of iterable)
+        continue;
+    assertEq(returnCalled, returnCalledExpected);
+
+    // continue does call iter.return for loops it skips
+    i = 0;
+    L: do {
+        for (var x of iterable)
+            continue L;
+    } while (false);
+    assertEq(returnCalled, ++returnCalledExpected);
+}
+
+test();
+
+if (typeof reportCompare === "function")
+    reportCompare(0, 0);

js/src/tests/ecma_6/shell.js

@@ -3,21 +3,39 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 (function(global) {
-  /** Yield every permutation of the elements in some array. */
+    /** Yield every permutation of the elements in some array. */
-  global.Permutations = function* Permutations(items) {
+    global.Permutations = function* Permutations(items) {
-      if (items.length == 0) {
+        if (items.length == 0) {
-          yield [];
+            yield [];
-      } else {
+        } else {
-          items = items.slice(0);
+            items = items.slice(0);
-          for (let i = 0; i < items.length; i++) {
+            for (let i = 0; i < items.length; i++) {
-              let swap = items[0];
+                let swap = items[0];
-              items[0] = items[i];
+                items[0] = items[i];
-              items[i] = swap;
+                items[i] = swap;
-              for (let e of Permutations(items.slice(1, items.length)))
+                for (let e of Permutations(items.slice(1, items.length)))
-                  yield [items[0]].concat(e);
+                    yield [items[0]].concat(e);
-          }
+            }
-      }
+        }
-  };
+    };
+
+    /** Make an iterator with a return method. */
+    global.makeIterator = function makeIterator(overrides) {
+        var iterator = {
+            next: function() {
+                if (overrides && overrides.next)
+                    return overrides.next();
+                return { done: false };
+            },
+            return: function() {
+                if (overrides && overrides.ret)
+                    return overrides.ret();
+                return { done: true };
+            }
+        };
+
+        return function() { return iterator; };
+    };
 })(this);
 
 if (typeof assertThrowsInstanceOf === 'undefined') {

js/src/vm/Interpreter.cpp

@@ -1171,13 +1171,19 @@ ProcessTryNotes(JSContext* cx, EnvironmentIter& ei, InterpreterRegs& regs)
             SettleOnTryNote(cx, tn, ei, regs);
             return FinallyContinuation;
 
-          case JSTRY_FOR_IN: {
+          case JSTRY_FOR_IN:
+          case JSTRY_ITERCLOSE: {
             /* This is similar to JSOP_ENDITER in the interpreter loop. */
             DebugOnly<jsbytecode*> pc = regs.fp()->script()->main() + tn->start + tn->length;
-            MOZ_ASSERT(JSOp(*pc) == JSOP_ENDITER);
+            MOZ_ASSERT_IF(tn->kind == JSTRY_FOR_IN, JSOp(*pc) == JSOP_ENDITER);
             Value* sp = regs.spForStackDepth(tn->stackDepth);
             RootedObject obj(cx, &sp[-1].toObject());
-            if (!UnwindIteratorForException(cx, obj)) {
+            bool ok;
+            if (tn->kind == JSTRY_FOR_IN)
+                ok = UnwindIteratorForException(cx, obj);
+            else
+                ok = IteratorCloseForException(cx, obj);
+            if (!ok) {
                 // We should only settle on the note only if
                 // UnwindIteratorForException itself threw, as
                 // onExceptionUnwind should be called anew with the new
@@ -5040,7 +5046,12 @@ js::ThrowCheckIsObject(JSContext* cx, CheckIsObjectKind kind)
 {
     switch (kind) {
       case CheckIsObjectKind::IteratorNext:
-        JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_NEXT_RETURNED_PRIMITIVE);
+        JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                                  JSMSG_ITER_METHOD_RETURNED_PRIMITIVE, "next");
+        break;
+      case CheckIsObjectKind::IteratorReturn:
+        JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                                  JSMSG_ITER_METHOD_RETURNED_PRIMITIVE, "return");
         break;
       case CheckIsObjectKind::GetIterator:
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_GET_ITER_RETURNED_PRIMITIVE);

js/src/vm/Interpreter.h

@@ -562,6 +562,7 @@ ReportRuntimeRedeclaration(JSContext* cx, HandlePropertyName name, const char* r
 
 enum class CheckIsObjectKind : uint8_t {
     IteratorNext,
+    IteratorReturn,
     GetIterator
 };