V4: Eliminate "done" from iterators

Instead of dragging another stack value around to mark if the iterator
was done, rather pass it an offset it should jump to if so. It can then
jump over any IteratorClose instruction while the ExceptionHandler can
still point to the IteratorClose instruction.

For this to work, we also have to refrain from checking for exceptions
as part of IteratorNext or IteratorClose. If IteratorNext generates an
exception, it also jumps to the "done" label, after which we dispatch
the exception. We don't want to jump to the exception handler for other
instructions in between as that would close the iterator. The iterator
should _not_ be closed if it has just thrown an exception, though. The
same holds for IteratorClose: If it throws an exception, we don't want
to jump back to the beginning of the loop's exception handler, since
that would produce an infinite loop. We also don't want to reset the
exception handler before IteratorClose because it needs to also be reset
if the iterator does not need to be closed.

This saves quite a few instructions and stack variables on actual
iteration.

For destructuring, we have to change the execution flow a bit. We need
to first perform the iteration for non-rest parameters, saving the
results in separate stack slots. This way we can apply our new "jump if
done" behavior if the iterator runs out or produces an exception itself.
We then save the "done" state in a separate stack slot, as before.
During the assignment of the iteration results to the actual variables,
we install an exception handler, so that we can still close the iterator
if one of the initializers throws an exception. This produces a few more
instructions than before:

1. We need to set and read the "needsClose" variable explicitly rather
   than having IteratorNext and IteratorDone do it implicitly.
2. We need an additional CheckException after the iteration.
3. We need an additional conditional Jump over the IteratorDone.

Everything considered, the savings we get for regular iteration and the
more consistent semantics of the instructions involved are well worth
the few extra instructions on destructuring, especially since everything
those extra instructions do was done implicitly by the iterator
instructions before.

For consistency, the IteratorNextForYieldStar instruction is refactored
to work the same way as IteratorNext: In case of either an exception or
"done" it jumps to an offset, and we refrain from individually
exception-checking each IteratorNextForYieldStart instruction.

Task-number: QTBUG-116725
Change-Id: I9e2ad4319495aecabafdbbd3dd0cbf3c6191f942
Reviewed-by: Olivier De Cannière <olivier.decanniere@qt.io>
Reviewed-by: Sami Shalayel <sami.shalayel@qt.io>

20 files changed, 159 insertions, 129 deletions

diff --git a/src/qml/compiler/qv4bytecodegenerator_p.h b/src/qml/compiler/qv4bytecodegenerator_p.h
index 35e539fb5e..fa14754f85 100644
--- a/src/qml/compiler/qv4bytecodegenerator_p.h
+++ b/src/qml/compiler/qv4bytecodegenerator_p.h
@@ -58,7 +58,7 @@ public:
                 link();
         }
 
-        void link() {
+        void link() const {
             Q_ASSERT(index >= 0);
             Q_ASSERT(generator->labels[index] == -1);
             generator->labels[index] = generator->instructions.size();
diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index aba071580b..1fca7bd75a 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -713,9 +713,8 @@ void Codegen::destructureElementList(const Codegen::Reference &array, PatternEle
     RegisterScope scope(this);
 
     Reference iterator = Reference::fromStackSlot(this);
-    Reference iteratorValue = Reference::fromStackSlot(this);
-    Reference iteratorDone = Reference::fromStackSlot(this);
-    Reference::storeConstOnStack(this, Encode(false), iteratorDone.stackSlot());
+    QVarLengthArray<Reference> iteratorValues;
+    Reference ignored;
 
     array.loadInAccumulator();
     Instruction::GetIterator iteratorObjInstr;
@@ -723,45 +722,76 @@ void Codegen::destructureElementList(const Codegen::Reference &array, PatternEle
     bytecodeGenerator->addInstruction(iteratorObjInstr);
     iterator.storeConsumeAccumulator();
 
+    BytecodeGenerator::Label done = bytecodeGenerator->newLabel();
+    Reference needsClose = Reference::storeConstOnStack(this, Encode(false));
+
+    for (PatternElementList *p = bindingList; p; p = p->next) {
+        PatternElement *e = p->element;
+        for (Elision *elision = p->elision; elision; elision = elision->next) {
+            iterator.loadInAccumulator();
+            Instruction::IteratorNext next;
+            if (!ignored.isValid())
+                ignored = Reference::fromStackSlot(this);
+            next.value = ignored.stackSlot();
+            bytecodeGenerator->addJumpInstruction(next).link(done);
+        }
+
+        if (!e)
+            continue;
+
+        if (e->type != PatternElement::RestElement) {
+            iterator.loadInAccumulator();
+            Instruction::IteratorNext next;
+            iteratorValues.push_back(Reference::fromStackSlot(this));
+            next.value = iteratorValues.back().stackSlot();
+            bytecodeGenerator->addJumpInstruction(next).link(done);
+        }
+    }
+
+    // If we've iterated through all the patterns without exhausing the iterator, it needs
+    // to be closed. But we don't close it here because:
+    // a, closing might throw an exception and we want to assign the values before we handle that
+    // b, there might be a rest element that could still continue iterating
+    Reference::fromConst(this, Encode(true)).storeOnStack(needsClose.stackSlot());
+
+    done.link();
+    bytecodeGenerator->checkException();
+
     {
-        auto cleanup = [this, iterator, iteratorDone]() {
+        ControlFlowUnwindCleanup flow(this, [&]() {
+            BytecodeGenerator::Label skipClose = bytecodeGenerator->newLabel();
+            needsClose.loadInAccumulator();
+            bytecodeGenerator->jumpFalse().link(skipClose);
             iterator.loadInAccumulator();
             Instruction::IteratorClose close;
-            close.done = iteratorDone.stackSlot();
             bytecodeGenerator->addInstruction(close);
-        };
-
-        ControlFlowUnwindCleanup flow(this, cleanup);
+            skipClose.link();
+        });
 
+        auto it = iteratorValues.constBegin();
         for (PatternElementList *p = bindingList; p; p = p->next) {
             PatternElement *e = p->element;
-            for (Elision *elision = p->elision; elision; elision = elision->next) {
-                iterator.loadInAccumulator();
-                Instruction::IteratorNext next;
-                next.value = iteratorValue.stackSlot();
-                next.done = iteratorDone.stackSlot();
-                bytecodeGenerator->addInstruction(next);
-            }
 
             if (!e)
                 continue;
 
-            RegisterScope scope(this);
-            iterator.loadInAccumulator();
-
             if (e->type == PatternElement::RestElement) {
-                Reference::fromConst(this, Encode(true)).storeOnStack(iteratorDone.stackSlot());
+                Q_ASSERT(it == iteratorValues.constEnd());
+
+                // The rest element is guaranteed to exhaust the iterator
+                Reference::fromConst(this, Encode(false)).storeOnStack(needsClose.stackSlot());
+
+                iterator.loadInAccumulator();
                 bytecodeGenerator->addInstruction(Instruction::DestructureRestElement());
-                initializeAndDestructureBindingElement(e, Reference::fromAccumulator(this), isDefinition);
+                initializeAndDestructureBindingElement(
+                        e, Reference::fromAccumulator(this), isDefinition);
             } else {
-                Instruction::IteratorNext next;
-                next.value = iteratorValue.stackSlot();
-                next.done = iteratorDone.stackSlot();
-                bytecodeGenerator->addInstruction(next);
-                initializeAndDestructureBindingElement(e, iteratorValue, isDefinition);
-                if (hasError())
-                    return;
+                Q_ASSERT(it != iteratorValues.constEnd());
+                initializeAndDestructureBindingElement(e, *it++, isDefinition);
             }
+
+            if (hasError())
+                return;
         }
     }
 }
@@ -1167,7 +1197,6 @@ bool Codegen::visit(ArrayPattern *ast)
             RegisterScope scope(this);
 
             Reference iterator = Reference::fromStackSlot(this);
-            Reference iteratorDone = Reference::fromConst(this, Encode(false)).storeOnStack();
             Reference lhsValue = Reference::fromStackSlot(this);
 
             // There should be a temporal block, so that variables declared in lhs shadow outside vars.
@@ -1187,24 +1216,23 @@ bool Codegen::visit(ArrayPattern *ast)
 
             BytecodeGenerator::Label in = bytecodeGenerator->newLabel();
             BytecodeGenerator::Label end = bytecodeGenerator->newLabel();
+            BytecodeGenerator::Label done = bytecodeGenerator->newLabel();
 
             {
-                auto cleanup = [this, iterator, iteratorDone]() {
+                auto cleanup = [this, iterator, done]() {
                     iterator.loadInAccumulator();
                     Instruction::IteratorClose close;
-                    close.done = iteratorDone.stackSlot();
                     bytecodeGenerator->addInstruction(close);
+                    done.link();
                 };
-                ControlFlowLoop flow(this, &end, &in, cleanup);
+                ControlFlowLoop flow(this, &end, &in, std::move(cleanup));
 
                 in.link();
                 bytecodeGenerator->addLoopStart(in);
                 iterator.loadInAccumulator();
                 Instruction::IteratorNext next;
                 next.value = lhsValue.stackSlot();
-                next.done = iteratorDone.stackSlot();
-                bytecodeGenerator->addInstruction(next);
-                bytecodeGenerator->addJumpInstruction(Instruction::JumpTrue()).link(end);
+                bytecodeGenerator->addJumpInstruction(next).link(done);
 
                 lhsValue.loadInAccumulator();
                 pushAccumulator();
@@ -3220,15 +3248,15 @@ bool Codegen::visit(YieldExpression *ast)
         Instruction::IteratorNextForYieldStar next;
         next.object = lhsValue.stackSlot();
         next.iterator = iterator.stackSlot();
-        bytecodeGenerator->addInstruction(next);
-
-        BytecodeGenerator::Jump done = bytecodeGenerator->jumpTrue();
+        BytecodeGenerator::Jump done = bytecodeGenerator->addJumpInstruction(next);
         bytecodeGenerator->jumpNotUndefined().link(loop);
+
         lhsValue.loadInAccumulator();
         emitReturn(acc);
 
 
         done.link();
+        bytecodeGenerator->checkException();
 
         lhsValue.loadInAccumulator();
         setExprResult(acc);
@@ -3572,7 +3600,6 @@ bool Codegen::visit(ForEachStatement *ast)
     TailCallBlocker blockTailCalls(this);
 
     Reference iterator = Reference::fromStackSlot(this);
-    Reference iteratorDone = Reference::fromConst(this, Encode(false)).storeOnStack();
     Reference lhsValue = Reference::fromStackSlot(this);
 
     // There should be a temporal block, so that variables declared in lhs shadow outside vars.
@@ -3593,16 +3620,20 @@ bool Codegen::visit(ForEachStatement *ast)
 
     BytecodeGenerator::Label in = bytecodeGenerator->newLabel();
     BytecodeGenerator::Label end = bytecodeGenerator->newLabel();
+    BytecodeGenerator::Label done;
 
     {
         std::function<void()> cleanup;
         if (ast->type == ForEachType::Of) {
-            cleanup = [iterator, iteratorDone, this]() {
+            done = bytecodeGenerator->newLabel();
+            cleanup = [iterator, this, done]() {
                 iterator.loadInAccumulator();
                 Instruction::IteratorClose close;
-                close.done = iteratorDone.stackSlot();
                 bytecodeGenerator->addInstruction(close);
+                done.link();
             };
+        } else {
+            done = end;
         }
         ControlFlowLoop flow(this, &end, &in, std::move(cleanup));
         bytecodeGenerator->addLoopStart(in);
@@ -3610,9 +3641,7 @@ bool Codegen::visit(ForEachStatement *ast)
         iterator.loadInAccumulator();
         Instruction::IteratorNext next;
         next.value = lhsValue.stackSlot();
-        next.done = iteratorDone.stackSlot();
-        bytecodeGenerator->addInstruction(next);
-        bytecodeGenerator->addJumpInstruction(Instruction::JumpTrue()).link(end);
+        bytecodeGenerator->addJumpInstruction(next).link(done);
 
         // each iteration gets it's own context, as per spec
         {
diff --git a/src/qml/compiler/qv4compilercontrolflow_p.h b/src/qml/compiler/qv4compilercontrolflow_p.h
index 070a1390ce..b190b77410 100644
--- a/src/qml/compiler/qv4compilercontrolflow_p.h
+++ b/src/qml/compiler/qv4compilercontrolflow_p.h
@@ -167,8 +167,8 @@ struct ControlFlowUnwindCleanup : public ControlFlowUnwind
     ~ControlFlowUnwindCleanup() {
         if (cleanup) {
             unwindLabel.link();
-            generator()->setUnwindHandler(parentUnwindHandler());
             cleanup();
+            generator()->setUnwindHandler(parentUnwindHandler());
             emitUnwindHandler();
         }
     }
diff --git a/src/qml/compiler/qv4instr_moth.cpp b/src/qml/compiler/qv4instr_moth.cpp
index 4fda5b1b19..2f6f12437d 100644
--- a/src/qml/compiler/qv4instr_moth.cpp
+++ b/src/qml/compiler/qv4instr_moth.cpp
@@ -405,15 +405,15 @@ QString dumpBytecode(
         MOTH_END_INSTR(GetIterator)
 
         MOTH_BEGIN_INSTR(IteratorNext)
-            s << dumpRegister(value, nFormals) << ", " << dumpRegister(done, nFormals);
+            s << dumpRegister(value, nFormals) << ", " << ABSOLUTE_OFFSET();
         MOTH_END_INSTR(IteratorNext)
 
         MOTH_BEGIN_INSTR(IteratorNextForYieldStar)
-            s << dumpRegister(iterator, nFormals) << ", " << dumpRegister(object, nFormals);
+            s << dumpRegister(iterator, nFormals) << ", " << dumpRegister(object, nFormals)
+              << ABSOLUTE_OFFSET();
         MOTH_END_INSTR(IteratorNextForYieldStar)
 
         MOTH_BEGIN_INSTR(IteratorClose)
-            s << dumpRegister(done, nFormals);
         MOTH_END_INSTR(IteratorClose)
 
         MOTH_BEGIN_INSTR(DestructureRestElement)
diff --git a/src/qml/compiler/qv4instr_moth_p.h b/src/qml/compiler/qv4instr_moth_p.h
index 7ebf849f3e..70e9f9e7a2 100644
--- a/src/qml/compiler/qv4instr_moth_p.h
+++ b/src/qml/compiler/qv4instr_moth_p.h
@@ -60,7 +60,7 @@ QT_BEGIN_NAMESPACE
 #define INSTR_Yield(op) INSTRUCTION(op, Yield, 0)
 #define INSTR_YieldStar(op) INSTRUCTION(op, YieldStar, 0)
 #define INSTR_Resume(op) INSTRUCTION(op, Resume, 1, offset)
-#define INSTR_IteratorNextForYieldStar(op) INSTRUCTION(op, IteratorNextForYieldStar, 2, iterator, object)
+#define INSTR_IteratorNextForYieldStar(op) INSTRUCTION(op, IteratorNextForYieldStar, 3, iterator, object, offset)
 #define INSTR_StoreProperty(op) INSTRUCTION(op, StoreProperty, 2, name, base)
 #define INSTR_SetLookup(op) INSTRUCTION(op, SetLookup, 2, index, base)
 #define INSTR_LoadSuperProperty(op) INSTRUCTION(op, LoadSuperProperty, 1, property)
@@ -94,8 +94,8 @@ QT_BEGIN_NAMESPACE
 #define INSTR_PopScriptContext(op) INSTRUCTION(op, PopScriptContext, 0)
 #define INSTR_PopContext(op) INSTRUCTION(op, PopContext, 0)
 #define INSTR_GetIterator(op) INSTRUCTION(op, GetIterator, 1, iterator)
-#define INSTR_IteratorNext(op) INSTRUCTION(op, IteratorNext, 2, value, done)
-#define INSTR_IteratorClose(op) INSTRUCTION(op, IteratorClose, 1, done)
+#define INSTR_IteratorNext(op) INSTRUCTION(op, IteratorNext, 2, value, offset)
+#define INSTR_IteratorClose(op) INSTRUCTION(op, IteratorClose, 0)
 #define INSTR_DestructureRestElement(op) INSTRUCTION(op, DestructureRestElement, 0)
 #define INSTR_DeleteProperty(op) INSTRUCTION(op, DeleteProperty, 2, base, index)
 #define INSTR_DeleteName(op) INSTRUCTION(op, DeleteName, 1, name)
diff --git a/src/qml/jit/qv4baselinejit.cpp b/src/qml/jit/qv4baselinejit.cpp
index d2895e8bf2..bd2e331cbc 100644
--- a/src/qml/jit/qv4baselinejit.cpp
+++ b/src/qml/jit/qv4baselinejit.cpp
@@ -586,7 +586,7 @@ void BaselineJIT::generate_GetIterator(int iterator)
     BASELINEJIT_GENERATE_RUNTIME_CALL(GetIterator, CallResultDestination::InAccumulator);
 }
 
-void BaselineJIT::generate_IteratorNext(int value, int done)
+void BaselineJIT::generate_IteratorNext(int value, int offset)
 {
     as->saveAccumulatorInFrame();
     as->prepareCallWithArgCount(3);
@@ -594,10 +594,10 @@ void BaselineJIT::generate_IteratorNext(int value, int done)
     as->passAccumulatorAsArg(1);
     as->passEngineAsArg(0);
     BASELINEJIT_GENERATE_RUNTIME_CALL(IteratorNext, CallResultDestination::InAccumulator);
-    as->storeReg(done);
+    labels.insert(as->jumpTrue(absoluteOffset(offset)));
 }
 
-void BaselineJIT::generate_IteratorNextForYieldStar(int iterator, int object)
+void BaselineJIT::generate_IteratorNextForYieldStar(int iterator, int object, int offset)
 {
     as->saveAccumulatorInFrame();
     as->prepareCallWithArgCount(4);
@@ -606,13 +606,13 @@ void BaselineJIT::generate_IteratorNextForYieldStar(int iterator, int object)
     as->passAccumulatorAsArg(1);
     as->passEngineAsArg(0);
     BASELINEJIT_GENERATE_RUNTIME_CALL(IteratorNextForYieldStar, CallResultDestination::InAccumulator);
+    labels.insert(as->jumpTrue(absoluteOffset(offset)));
 }
 
-void BaselineJIT::generate_IteratorClose(int done)
+void BaselineJIT::generate_IteratorClose()
 {
     as->saveAccumulatorInFrame();
-    as->prepareCallWithArgCount(3);
-    as->passJSSlotAsArg(done, 2);
+    as->prepareCallWithArgCount(2);
     as->passAccumulatorAsArg(1);
     as->passEngineAsArg(0);
     BASELINEJIT_GENERATE_RUNTIME_CALL(IteratorClose, CallResultDestination::InAccumulator);
diff --git a/src/qml/jit/qv4baselinejit_p.h b/src/qml/jit/qv4baselinejit_p.h
index c502e4d405..40138ea700 100644
--- a/src/qml/jit/qv4baselinejit_p.h
+++ b/src/qml/jit/qv4baselinejit_p.h
@@ -106,9 +106,9 @@ public:
     void generate_PopScriptContext() override;
     void generate_PopContext() override;
     void generate_GetIterator(int iterator) override;
-    void generate_IteratorNext(int value, int done) override;
-    void generate_IteratorNextForYieldStar(int iterator, int object) override;
-    void generate_IteratorClose(int done) override;
+    void generate_IteratorNext(int value, int offset) override;
+    void generate_IteratorNextForYieldStar(int iterator, int object, int offset) override;
+    void generate_IteratorClose() override;
     void generate_DestructureRestElement() override;
     void generate_DeleteProperty(int base, int index) override;
     void generate_DeleteName(int name) override;
diff --git a/src/qml/jsruntime/qv4arrayobject.cpp b/src/qml/jsruntime/qv4arrayobject.cpp
index 05e73d0295..a32017210a 100644
--- a/src/qml/jsruntime/qv4arrayobject.cpp
+++ b/src/qml/jsruntime/qv4arrayobject.cpp
@@ -194,9 +194,8 @@ ReturnedValue ArrayPrototype::method_from(const FunctionObject *builtin, const V
         // sets them into the created array.
         forever {
             if (k > (static_cast<qint64>(1) << 53) - 1) {
-                ScopedValue falsey(scope, Encode(false));
                 ScopedValue error(scope, scope.engine->throwTypeError());
-                return Runtime::IteratorClose::call(scope.engine, iterator, falsey);
+                return Runtime::IteratorClose::call(scope.engine, iterator);
             }
 
             // Retrieve the next value. If the iteration ends, we're done here.
@@ -218,7 +217,7 @@ ReturnedValue ArrayPrototype::method_from(const FunctionObject *builtin, const V
                 mapArguments[1] = Value::fromDouble(k);
                 mappedValue = mapfn->call(thisArg, mapArguments, 2);
                 if (scope.hasException())
-                    return Runtime::IteratorClose::call(scope.engine, iterator, Value::fromBoolean(false));
+                    return Runtime::IteratorClose::call(scope.engine, iterator);
             } else {
                 mappedValue = *nextValue;
             }
@@ -230,10 +229,8 @@ ReturnedValue ArrayPrototype::method_from(const FunctionObject *builtin, const V
                 scope.engine->throwTypeError(QString::fromLatin1("Cannot redefine property: %1").arg(k));
             }
 
-            if (scope.hasException()) {
-                ScopedValue falsey(scope, Encode(false));
-                return Runtime::IteratorClose::call(scope.engine, iterator, falsey);
-            }
+            if (scope.hasException())
+                return Runtime::IteratorClose::call(scope.engine, iterator);
 
             k++;
         }
diff --git a/src/qml/jsruntime/qv4mapobject.cpp b/src/qml/jsruntime/qv4mapobject.cpp
index 72e66e0e76..4bb9617b93 100644
--- a/src/qml/jsruntime/qv4mapobject.cpp
+++ b/src/qml/jsruntime/qv4mapobject.cpp
@@ -74,8 +74,7 @@ ReturnedValue WeakMapCtor::construct(const FunctionObject *f, const Value *argv,
                 if (scope.hasException())
                     break;
             }
-            ScopedValue falsey(scope, Encode(false));
-            return Runtime::IteratorClose::call(scope.engine, iter, falsey);
+            return Runtime::IteratorClose::call(scope.engine, iter);
         }
     }
     return a->asReturnedValue();
diff --git a/src/qml/jsruntime/qv4promiseobject.cpp b/src/qml/jsruntime/qv4promiseobject.cpp
index 5424545e79..16cffea124 100644
--- a/src/qml/jsruntime/qv4promiseobject.cpp
+++ b/src/qml/jsruntime/qv4promiseobject.cpp
@@ -575,7 +575,7 @@ ReturnedValue PromiseCtor::method_all(const FunctionObject *f, const Value *this
             }
 
             if (!doneValue->toBoolean())
-                completion = Runtime::IteratorClose::call(e, iteratorObject, doneValue);
+                completion = Runtime::IteratorClose::call(e, iteratorObject);
 
             reject->call(newPromise, completion, 1);
             return newPromise.asReturnedValue();
@@ -583,7 +583,9 @@ ReturnedValue PromiseCtor::method_all(const FunctionObject *f, const Value *this
 
         ScopedObject nextPromise(scope, Value::fromReturnedValue(resolve->call(thisObject, nextValue, 1)));
         if (scope.hasException() || !nextPromise) {
-            ScopedValue completion(scope, Runtime::IteratorClose::call(e, iteratorObject, doneValue));
+            ScopedValue completion(scope, doneValue->toBoolean()
+                                           ? Encode::undefined()
+                                           : Runtime::IteratorClose::call(e, iteratorObject));
             if (scope.hasException()) {
                 completion = e->exceptionValue->asReturnedValue();
                 dropException(e);
@@ -605,7 +607,7 @@ ReturnedValue PromiseCtor::method_all(const FunctionObject *f, const Value *this
             }
 
             if (!doneValue->toBoolean())
-                completion = Runtime::IteratorClose::call(scope.engine, iteratorObject, doneValue);
+                completion = Runtime::IteratorClose::call(scope.engine, iteratorObject);
 
             reject->call(newPromise, completion, 1);
             return newPromise.asReturnedValue();
@@ -624,7 +626,7 @@ ReturnedValue PromiseCtor::method_all(const FunctionObject *f, const Value *this
             dropException(e);
 
             if (!doneValue->toBoolean())
-                completion = Runtime::IteratorClose::call(scope.engine, iteratorObject, doneValue);
+                completion = Runtime::IteratorClose::call(scope.engine, iteratorObject);
 
             reject->call(newPromise, completion, 1);
             return newPromise.asReturnedValue();
@@ -686,7 +688,9 @@ ReturnedValue PromiseCtor::method_race(const FunctionObject *f, const Value *thi
         doneValue = Value::fromReturnedValue(Runtime::IteratorNext::call(e, iteratorObject, nextValue));
 
         if (scope.hasException()) {
-            ScopedValue completion(scope, Runtime::IteratorClose::call(e, iteratorObject, doneValue));
+            ScopedValue completion(scope, doneValue->toBoolean()
+                                           ? Encode::undefined()
+                                           : Runtime::IteratorClose::call(e, iteratorObject));
             if (scope.hasException()) {
                 completion = e->exceptionValue->asReturnedValue();
                 dropException(e);
@@ -721,7 +725,7 @@ ReturnedValue PromiseCtor::method_race(const FunctionObject *f, const Value *thi
             }
 
             if (!doneValue->toBoolean())
-                completion = Runtime::IteratorClose::call(e, iteratorObject, doneValue);
+                completion = Runtime::IteratorClose::call(e, iteratorObject);
 
             reject->call(newPromise, completion, 1);
             return newPromise.asReturnedValue();
@@ -729,7 +733,9 @@ ReturnedValue PromiseCtor::method_race(const FunctionObject *f, const Value *thi
 
         ScopedObject nextPromise(scope, Value::fromReturnedValue(resolve->call(thisObject, nextValue, 1)));
         if (scope.hasException() || !nextPromise) {
-            ScopedValue completion(scope, Runtime::IteratorClose::call(e, iteratorObject, doneValue));
+            ScopedValue completion(scope, doneValue->toBoolean()
+                                          ? Encode::undefined()
+                                          : Runtime::IteratorClose::call(e, iteratorObject));
             if (scope.hasException()) {
                 completion = e->exceptionValue->asReturnedValue();
                 dropException(e);
@@ -749,7 +755,7 @@ ReturnedValue PromiseCtor::method_race(const FunctionObject *f, const Value *thi
             }
 
             if (!doneValue->toBoolean())
-                completion = Runtime::IteratorClose::call(e, iteratorObject, doneValue);
+                completion = Runtime::IteratorClose::call(e, iteratorObject);
 
             reject->call(newPromise, completion, 1);
             return newPromise.asReturnedValue();
@@ -768,7 +774,7 @@ ReturnedValue PromiseCtor::method_race(const FunctionObject *f, const Value *thi
             dropException(e);
 
             if (!doneValue->toBoolean())
-                completion = Runtime::IteratorClose::call(e, iteratorObject, doneValue);
+                completion = Runtime::IteratorClose::call(e, iteratorObject);
 
             reject->call(newPromise, completion, 1);
             return newPromise.asReturnedValue();
diff --git a/src/qml/jsruntime/qv4runtime.cpp b/src/qml/jsruntime/qv4runtime.cpp
index f5b88f662b..01a378406a 100644
--- a/src/qml/jsruntime/qv4runtime.cpp
+++ b/src/qml/jsruntime/qv4runtime.cpp
@@ -862,6 +862,8 @@ ReturnedValue Runtime::IteratorNextForYieldStar::call(ExecutionEngine *engine, c
             engine->hasException = false;
 
             ScopedValue ret(scope, static_cast<const Object &>(iterator).get(engine->id_return()));
+            if (engine->hasException)
+                return Encode(true);
             if (ret->isUndefined()) {
                 // propagate return()
                 return Encode::undefined();
@@ -876,14 +878,13 @@ ReturnedValue Runtime::IteratorNextForYieldStar::call(ExecutionEngine *engine, c
 
             ScopedValue t(scope, static_cast<const Object &>(iterator).get(engine->id_throw()));
             if (engine->hasException)
-                return Encode::undefined();
+                return Encode(true);
             if (t->isUndefined()) {
                 // no throw method on the iterator
-                ScopedValue done(scope, Encode(false));
-                IteratorClose::call(engine, iterator, done);
-                if (engine->hasException)
-                    return Encode::undefined();
-                return engine->throwTypeError();
+                IteratorClose::call(engine, iterator);
+                if (!engine->hasException)
+                    engine->throwTypeError();
+                return Encode(true);
             }
             f = t->as<FunctionObject>();
             arg = exceptionValue;
@@ -894,14 +895,18 @@ ReturnedValue Runtime::IteratorNextForYieldStar::call(ExecutionEngine *engine, c
         f = next->as<FunctionObject>();
     }
 
-    if (!f)
-        return engine->throwTypeError();
+    if (!f) {
+        engine->throwTypeError();
+        return Encode(true);
+    }
 
     ScopedObject o(scope, f->call(&iterator, arg, 1));
     if (scope.hasException())
         return Encode(true);
-    if (!o)
-        return engine->throwTypeError();
+    if (!o) {
+        engine->throwTypeError();
+        return Encode(true);
+    }
 
     ScopedValue d(scope, o->get(engine->id_done()));
     if (scope.hasException())
@@ -909,18 +914,15 @@ ReturnedValue Runtime::IteratorNextForYieldStar::call(ExecutionEngine *engine, c
     bool done = d->toBoolean();
     if (done) {
         *object = o->get(engine->id_value());
-        return returnCalled ? Encode::undefined() : Encode(true);
+        return (returnCalled && !engine->hasException) ? Encode::undefined() : Encode(true);
     }
     *object = o;
     return Encode(false);
 }
 
-ReturnedValue Runtime::IteratorClose::call(ExecutionEngine *engine, const Value &iterator, const Value &done)
+ReturnedValue Runtime::IteratorClose::call(ExecutionEngine *engine, const Value &iterator)
 {
     Q_ASSERT(iterator.isObject());
-    Q_ASSERT(done.isBoolean());
-    if (done.booleanValue())
-        return Encode::undefined();
 
     Scope scope(engine);
     ScopedValue e(scope);
diff --git a/src/qml/jsruntime/qv4runtimeapi_p.h b/src/qml/jsruntime/qv4runtimeapi_p.h
index 5e11d15495..797de7e13f 100644
--- a/src/qml/jsruntime/qv4runtimeapi_p.h
+++ b/src/qml/jsruntime/qv4runtimeapi_p.h
@@ -43,7 +43,7 @@ struct Q_QML_PRIVATE_EXPORT Runtime {
         static constexpr bool lastArgumentIsOutputValue = out == LastArgumentIsOutputValue::Yes;
     };
     using PureMethod = Method<Throws::No, ChangesContext::No, Pure::Yes>;
-    using IteratorMethod = Method<Throws::Yes, ChangesContext::No, Pure::No,
+    using IteratorMethod = Method<Throws::No, ChangesContext::No, Pure::No,
                                   LastArgumentIsOutputValue::Yes>;
 
     /* call */
@@ -285,9 +285,9 @@ struct Q_QML_PRIVATE_EXPORT Runtime {
     {
         static ReturnedValue call(ExecutionEngine *, const Value &, const Value &, Value *);
     };
-    struct Q_QML_PRIVATE_EXPORT IteratorClose : Method<Throws::Yes>
+    struct Q_QML_PRIVATE_EXPORT IteratorClose : Method<Throws::No>
     {
-        static ReturnedValue call(ExecutionEngine *, const Value &, const Value &);
+        static ReturnedValue call(ExecutionEngine *, const Value &);
     };
     struct Q_QML_PRIVATE_EXPORT DestructureRestElement : Method<Throws::Yes>
     {
diff --git a/src/qml/jsruntime/qv4setobject.cpp b/src/qml/jsruntime/qv4setobject.cpp
index 2a4cdc1944..01fc62a4d4 100644
--- a/src/qml/jsruntime/qv4setobject.cpp
+++ b/src/qml/jsruntime/qv4setobject.cpp
@@ -54,10 +54,8 @@ ReturnedValue WeakSetCtor::construct(const FunctionObject *f, const Value *argv,
                     return a.asReturnedValue();
 
                 adder->call(a, nextValue, 1);
-                if (scope.hasException()) {
-                    ScopedValue falsey(scope, Encode(false));
-                    return Runtime::IteratorClose::call(scope.engine, iter, falsey);
-                }
+                if (scope.hasException())
+                    return Runtime::IteratorClose::call(scope.engine, iter);
             }
         }
     }
diff --git a/src/qml/jsruntime/qv4typedarray.cpp b/src/qml/jsruntime/qv4typedarray.cpp
index 43dc0fae4f..6c72eaba5f 100644
--- a/src/qml/jsruntime/qv4typedarray.cpp
+++ b/src/qml/jsruntime/qv4typedarray.cpp
@@ -1680,14 +1680,13 @@ ReturnedValue IntrinsicTypedArrayCtor::method_from(const FunctionObject *f, cons
         forever {
             // Here we calculate the length of the iterable range.
             if (iterableLength > (static_cast<qint64>(1) << 53) - 1) {
-                ScopedValue falsey(scope, Encode(false));
                 ScopedValue error(scope, scope.engine->throwTypeError());
-                return Runtime::IteratorClose::call(scope.engine, lengthIterator, falsey);
+                return Runtime::IteratorClose::call(scope.engine, lengthIterator);
             }
             // Retrieve the next value. If the iteration ends, we're done here.
             done = Value::fromReturnedValue(Runtime::IteratorNext::call(scope.engine, lengthIterator, nextValue));
             if (scope.hasException())
-                return Runtime::IteratorClose::call(scope.engine, lengthIterator, Value::fromBoolean(false));
+                return Runtime::IteratorClose::call(scope.engine, lengthIterator);
             if (done->toBoolean()) {
                 break;
             }
@@ -1720,21 +1719,21 @@ ReturnedValue IntrinsicTypedArrayCtor::method_from(const FunctionObject *f, cons
         for (qint64 k = 0; k < iterableLength; ++k) {
             done = Value::fromReturnedValue(Runtime::IteratorNext::call(scope.engine, iterator, nextValue));
             if (scope.hasException())
-                return Runtime::IteratorClose::call(scope.engine, iterator, Value::fromBoolean(false));
+                return Runtime::IteratorClose::call(scope.engine, iterator);
 
             if (mapfn) {
                 mapArguments[0] = *nextValue;
                 mapArguments[1] = Value::fromDouble(k);
                 mappedValue = mapfn->call(thisArg, mapArguments, 2);
                 if (scope.hasException())
-                    return Runtime::IteratorClose::call(scope.engine, iterator, Value::fromBoolean(false));
+                    return Runtime::IteratorClose::call(scope.engine, iterator);
             } else {
                 mappedValue = *nextValue;
             }
 
             a->put(k, mappedValue);
             if (scope.hasException())
-                return Runtime::IteratorClose::call(scope.engine, iterator, Value::fromBoolean(false));
+                return Runtime::IteratorClose::call(scope.engine, iterator);
         }
         return a.asReturnedValue();
     } else {
diff --git a/src/qml/jsruntime/qv4vme_moth.cpp b/src/qml/jsruntime/qv4vme_moth.cpp
index 0d4ba41fee..55f2261b4d 100644
--- a/src/qml/jsruntime/qv4vme_moth.cpp
+++ b/src/qml/jsruntime/qv4vme_moth.cpp
@@ -844,7 +844,8 @@ QV4::ReturnedValue VME::interpret(JSTypesStackFrame *frame, ExecutionEngine *eng
     MOTH_BEGIN_INSTR(IteratorNextForYieldStar)
         STORE_ACC();
         acc = Runtime::IteratorNextForYieldStar::call(engine, accumulator, STACK_VALUE(iterator), &STACK_VALUE(object));
-        CHECK_EXCEPTION;
+        if (ACC.toBoolean())
+            code += offset;
     MOTH_END_INSTR(IteratorNextForYieldStar)
 
     MOTH_BEGIN_INSTR(CallValue)
@@ -1054,15 +1055,14 @@ QV4::ReturnedValue VME::interpret(JSTypesStackFrame *frame, ExecutionEngine *eng
         STORE_IP();
         STORE_ACC();
         acc = Runtime::IteratorNext::call(engine, accumulator, &STACK_VALUE(value));
-        STACK_VALUE(done) = acc;
-        CHECK_EXCEPTION;
+        if (ACC.toBoolean())
+            code += offset;
     MOTH_END_INSTR(IteratorNext)
 
     MOTH_BEGIN_INSTR(IteratorClose)
         STORE_IP();
         STORE_ACC();
-        acc = Runtime::IteratorClose::call(engine, accumulator, STACK_VALUE(done));
-        CHECK_EXCEPTION;
+        acc = Runtime::IteratorClose::call(engine, accumulator);
     MOTH_END_INSTR(IteratorClose)
 
     MOTH_BEGIN_INSTR(DestructureRestElement)
diff --git a/src/qmlcompiler/qqmljscodegenerator.cpp b/src/qmlcompiler/qqmljscodegenerator.cpp
index c33b915441..d62e899dbc 100644
--- a/src/qmlcompiler/qqmljscodegenerator.cpp
+++ b/src/qmlcompiler/qqmljscodegenerator.cpp
@@ -2299,23 +2299,23 @@ void QQmlJSCodeGenerator::generate_GetIterator(int iterator)
     BYTECODE_UNIMPLEMENTED();
 }
 
-void QQmlJSCodeGenerator::generate_IteratorNext(int value, int done)
+void QQmlJSCodeGenerator::generate_IteratorNext(int value, int offset)
 {
     Q_UNUSED(value)
-    Q_UNUSED(done)
+    Q_UNUSED(offset)
     BYTECODE_UNIMPLEMENTED();
 }
 
-void QQmlJSCodeGenerator::generate_IteratorNextForYieldStar(int iterator, int object)
+void QQmlJSCodeGenerator::generate_IteratorNextForYieldStar(int iterator, int object, int offset)
 {
     Q_UNUSED(iterator)
     Q_UNUSED(object)
+    Q_UNUSED(offset)
     BYTECODE_UNIMPLEMENTED();
 }
 
-void QQmlJSCodeGenerator::generate_IteratorClose(int done)
+void QQmlJSCodeGenerator::generate_IteratorClose()
 {
-    Q_UNUSED(done)
     BYTECODE_UNIMPLEMENTED();
 }
 
diff --git a/src/qmlcompiler/qqmljscodegenerator_p.h b/src/qmlcompiler/qqmljscodegenerator_p.h
index 0a743127f9..b6ffd5ac7f 100644
--- a/src/qmlcompiler/qqmljscodegenerator_p.h
+++ b/src/qmlcompiler/qqmljscodegenerator_p.h
@@ -133,9 +133,9 @@ protected:
     void generate_PopScriptContext() override;
     void generate_PopContext() override;
     void generate_GetIterator(int iterator) override;
-    void generate_IteratorNext(int value, int done) override;
-    void generate_IteratorNextForYieldStar(int iterator, int object) override;
-    void generate_IteratorClose(int done) override;
+    void generate_IteratorNext(int value, int offset) override;
+    void generate_IteratorNextForYieldStar(int iterator, int object, int offset) override;
+    void generate_IteratorClose() override;
     void generate_DestructureRestElement() override;
     void generate_DeleteProperty(int base, int index) override;
     void generate_DeleteName(int name) override;
diff --git a/src/qmlcompiler/qqmljscompilepass_p.h b/src/qmlcompiler/qqmljscompilepass_p.h
index ea73169538..05f0816aa7 100644
--- a/src/qmlcompiler/qqmljscompilepass_p.h
+++ b/src/qmlcompiler/qqmljscompilepass_p.h
@@ -411,9 +411,9 @@ protected:
     void generate_GetTemplateObject(int) override {}
     void generate_Increment() override {}
     void generate_InitializeBlockDeadTemporalZone(int, int) override {}
-    void generate_IteratorClose(int) override {}
+    void generate_IteratorClose() override {}
     void generate_IteratorNext(int, int) override {}
-    void generate_IteratorNextForYieldStar(int, int) override {}
+    void generate_IteratorNextForYieldStar(int, int, int) override {}
     void generate_Jump(int) override {}
     void generate_JumpFalse(int) override {}
     void generate_JumpNoException(int) override {}
diff --git a/src/qmlcompiler/qqmljstypepropagator.cpp b/src/qmlcompiler/qqmljstypepropagator.cpp
index 4bddaf470e..3a02cd7714 100644
--- a/src/qmlcompiler/qqmljstypepropagator.cpp
+++ b/src/qmlcompiler/qqmljstypepropagator.cpp
@@ -1897,23 +1897,23 @@ void QQmlJSTypePropagator::generate_GetIterator(int iterator)
     INSTR_PROLOGUE_NOT_IMPLEMENTED();
 }
 
-void QQmlJSTypePropagator::generate_IteratorNext(int value, int done)
+void QQmlJSTypePropagator::generate_IteratorNext(int value, int offset)
 {
     Q_UNUSED(value)
-    Q_UNUSED(done)
+    Q_UNUSED(offset)
     INSTR_PROLOGUE_NOT_IMPLEMENTED();
 }
 
-void QQmlJSTypePropagator::generate_IteratorNextForYieldStar(int iterator, int object)
+void QQmlJSTypePropagator::generate_IteratorNextForYieldStar(int iterator, int object, int offset)
 {
     Q_UNUSED(iterator)
     Q_UNUSED(object)
+    Q_UNUSED(offset)
     INSTR_PROLOGUE_NOT_IMPLEMENTED();
 }
 
-void QQmlJSTypePropagator::generate_IteratorClose(int done)
+void QQmlJSTypePropagator::generate_IteratorClose()
 {
-    Q_UNUSED(done)
     INSTR_PROLOGUE_NOT_IMPLEMENTED();
 }
 
diff --git a/src/qmlcompiler/qqmljstypepropagator_p.h b/src/qmlcompiler/qqmljstypepropagator_p.h
index 6e9ac5e16a..282e3dad35 100644
--- a/src/qmlcompiler/qqmljstypepropagator_p.h
+++ b/src/qmlcompiler/qqmljstypepropagator_p.h
@@ -101,9 +101,9 @@ struct Q_QMLCOMPILER_PRIVATE_EXPORT QQmlJSTypePropagator : public QQmlJSCompileP
     void generate_PopScriptContext() override;
     void generate_PopContext() override;
     void generate_GetIterator(int iterator) override;
-    void generate_IteratorNext(int value, int done) override;
-    void generate_IteratorNextForYieldStar(int iterator, int object) override;
-    void generate_IteratorClose(int done) override;
+    void generate_IteratorNext(int value, int offset) override;
+    void generate_IteratorNextForYieldStar(int iterator, int object, int offset) override;
+    void generate_IteratorClose() override;
     void generate_DestructureRestElement() override;
     void generate_DeleteProperty(int base, int index) override;
     void generate_DeleteName(int name) override;