Call iterator.return when required in destructuring assignments

Array destructuring assignments require a call to iterator.return if
the iterator hasn't been exhausted during destructuring.

Change-Id: I39fe4bc01bef6fb2ad3bda92caf6779fbbddc8e2
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

10 files changed, 95 insertions, 5 deletions

diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index 99c45d19fa..897f104093 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -478,6 +478,7 @@ void Codegen::destructureElementList(const Codegen::Reference &array, PatternEle
 
     Reference iterator = Reference::fromStackSlot(this);
     Reference iteratorValue = Reference::fromStackSlot(this);
+    Reference iteratorDone = Reference::fromStackSlot(this);
 
     array.loadInAccumulator();
     Instruction::GetIterator iteratorObjInstr;
@@ -485,29 +486,42 @@ void Codegen::destructureElementList(const Codegen::Reference &array, PatternEle
     bytecodeGenerator->addInstruction(iteratorObjInstr);
     iterator.storeConsumeAccumulator();
 
+    bool hadNext = false;
+    bool hasRest = false;
+
     BytecodeGenerator::Label end = bytecodeGenerator->newLabel();
 
     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();
             bytecodeGenerator->addInstruction(next);
+            hadNext = true;
+            bool last = !elision->next && !e && !p->next;
+            if (last)
+                iteratorDone.storeConsumeAccumulator();
         }
 
+        if (!e)
+            continue;
+
+        hadNext = true;
         RegisterScope scope(this);
         iterator.loadInAccumulator();
 
-        PatternElement *e = p->element;
-        if (e && e->type == PatternElement::RestElement) {
+        if (e->type == PatternElement::RestElement) {
             bytecodeGenerator->addInstruction(Instruction::DestructureRestElement());
             initializeAndDestructureBindingElement(e, Reference::fromAccumulator(this));
+            hasRest = true;
         } else {
             Instruction::IteratorNext next;
             next.value = iteratorValue.stackSlot();
             bytecodeGenerator->addInstruction(next);
-            if (!e)
-                continue;
+            bool last = !p->next;
+            if (last)
+                iteratorDone.storeConsumeAccumulator();
             if (e->type != PatternElement::RestElement) {
                 initializeAndDestructureBindingElement(e, iteratorValue);
                 if (hasError) {
@@ -517,6 +531,18 @@ void Codegen::destructureElementList(const Codegen::Reference &array, PatternEle
             }
         }
     }
+
+    if (!hadNext) {
+        Reference::storeConstOnStack(this, Encode(false), iteratorDone.stackSlot());
+    }
+
+    if (!hasRest) {
+        iterator.loadInAccumulator();
+        Instruction::IteratorClose close;
+        close.done = iteratorDone.stackSlot();
+        bytecodeGenerator->addInstruction(close);
+    }
+
     end.link();
 }
 
diff --git a/src/qml/compiler/qv4instr_moth.cpp b/src/qml/compiler/qv4instr_moth.cpp
index 7d8a1774f1..5e7fe816b2 100644
--- a/src/qml/compiler/qv4instr_moth.cpp
+++ b/src/qml/compiler/qv4instr_moth.cpp
@@ -434,9 +434,13 @@ void dumpBytecode(const char *code, int len, int nLocals, int nFormals, int /*st
         MOTH_END_INSTR(GetIterator)
 
         MOTH_BEGIN_INSTR(IteratorNext)
-            d << value;
+            d << dumpRegister(value, nFormals);
         MOTH_END_INSTR(IteratorNext)
 
+        MOTH_BEGIN_INSTR(IteratorClose)
+            d << dumpRegister(done, nFormals);
+        MOTH_END_INSTR(IteratorClose)
+
         MOTH_BEGIN_INSTR(DestructureRestElement)
         MOTH_END_INSTR(DestructureRestElement)
 
diff --git a/src/qml/compiler/qv4instr_moth_p.h b/src/qml/compiler/qv4instr_moth_p.h
index b4eaae4f6c..83ff11a128 100644
--- a/src/qml/compiler/qv4instr_moth_p.h
+++ b/src/qml/compiler/qv4instr_moth_p.h
@@ -123,6 +123,7 @@ QT_BEGIN_NAMESPACE
 #define INSTR_PopContext(op) INSTRUCTION(op, PopContext, 1, reg)
 #define INSTR_GetIterator(op) INSTRUCTION(op, GetIterator, 1, iterator)
 #define INSTR_IteratorNext(op) INSTRUCTION(op, IteratorNext, 1, value)
+#define INSTR_IteratorClose(op) INSTRUCTION(op, IteratorClose, 1, done)
 #define INSTR_DestructureRestElement(op) INSTRUCTION(op, DestructureRestElement, 0)
 #define INSTR_DeleteMember(op) INSTRUCTION(op, DeleteMember, 2, member, base)
 #define INSTR_DeleteSubscript(op) INSTRUCTION(op, DeleteSubscript, 2, base, index)
@@ -252,6 +253,7 @@ QT_BEGIN_NAMESPACE
     F(PopContext) \
     F(GetIterator) \
     F(IteratorNext) \
+    F(IteratorClose) \
     F(DestructureRestElement) \
     F(DeleteMember) \
     F(DeleteSubscript) \
diff --git a/src/qml/jit/qv4jit.cpp b/src/qml/jit/qv4jit.cpp
index a480cfbc30..15ece7e903 100644
--- a/src/qml/jit/qv4jit.cpp
+++ b/src/qml/jit/qv4jit.cpp
@@ -725,6 +725,17 @@ void BaselineJIT::generate_IteratorNext(int value)
     as->checkException();
 }
 
+void BaselineJIT::generate_IteratorClose(int done)
+{
+    as->saveAccumulatorInFrame();
+    as->prepareCallWithArgCount(3);
+    as->passRegAsArg(done, 2);
+    as->passAccumulatorAsArg(1);
+    as->passEngineAsArg(0);
+    JIT_GENERATE_RUNTIME_CALL(Runtime::method_iteratorClose, Assembler::ResultInAccumulator);
+    as->checkException();
+}
+
 void BaselineJIT::generate_DestructureRestElement()
 {
     as->saveAccumulatorInFrame();
@@ -1291,6 +1302,9 @@ void BaselineJIT::collectLabelsInBytecode()
         MOTH_BEGIN_INSTR(IteratorNext)
         MOTH_END_INSTR(IteratorNext)
 
+        MOTH_BEGIN_INSTR(IteratorClose)
+        MOTH_END_INSTR(IteratorClose)
+
         MOTH_BEGIN_INSTR(DestructureRestElement)
         MOTH_END_INSTR(DestructureRestElement)
 
diff --git a/src/qml/jit/qv4jit_p.h b/src/qml/jit/qv4jit_p.h
index fa4a367e91..d48041d954 100644
--- a/src/qml/jit/qv4jit_p.h
+++ b/src/qml/jit/qv4jit_p.h
@@ -187,6 +187,7 @@ public:
     void generate_PopContext(int reg) override;
     void generate_GetIterator(int iterator) override;
     void generate_IteratorNext(int value) override;
+    void generate_IteratorClose(int done) override;
     void generate_DestructureRestElement() override;
     void generate_DeleteMember(int member, int base) override;
     void generate_DeleteSubscript(int base, int index) override;
diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp
index 8227fe0184..96855b6155 100644
--- a/src/qml/jsruntime/qv4engine.cpp
+++ b/src/qml/jsruntime/qv4engine.cpp
@@ -270,6 +270,7 @@ ExecutionEngine::ExecutionEngine(QJSEngine *jsEngine)
     jsStrings[String_lastIndex] = newIdentifier(QStringLiteral("lastIndex"));
     jsStrings[String_next] = newIdentifier(QStringLiteral("next"));
     jsStrings[String_done] = newIdentifier(QStringLiteral("done"));
+    jsStrings[String_return] = newIdentifier(QStringLiteral("return"));
 
     jsSymbols[Symbol_hasInstance] = Symbol::create(this, QStringLiteral("@Symbol.hasInstance"));
     jsSymbols[Symbol_isConcatSpreadable] = Symbol::create(this, QStringLiteral("@Symbol.isConcatSpreadable"));
diff --git a/src/qml/jsruntime/qv4engine_p.h b/src/qml/jsruntime/qv4engine_p.h
index 46bf58de07..e90be8e896 100644
--- a/src/qml/jsruntime/qv4engine_p.h
+++ b/src/qml/jsruntime/qv4engine_p.h
@@ -329,6 +329,7 @@ public:
         String_lastIndex,
         String_next,
         String_done,
+        String_return,
 
         NJSStrings
     };
@@ -389,6 +390,7 @@ public:
     String *id_lastIndex() const { return reinterpret_cast<String *>(jsStrings + String_lastIndex); }
     String *id_next() const { return reinterpret_cast<String *>(jsStrings + String_next); }
     String *id_done() const { return reinterpret_cast<String *>(jsStrings + String_done); }
+    String *id_return() const { return reinterpret_cast<String *>(jsStrings + String_return); }
 
     Symbol *symbol_hasInstance() const { return reinterpret_cast<Symbol *>(jsSymbols + Symbol_hasInstance); }
     Symbol *symbol_isConcatSpreadable() const { return reinterpret_cast<Symbol *>(jsSymbols + Symbol_isConcatSpreadable); }
diff --git a/src/qml/jsruntime/qv4runtime.cpp b/src/qml/jsruntime/qv4runtime.cpp
index 650f5c6735..6eff9c50db 100644
--- a/src/qml/jsruntime/qv4runtime.cpp
+++ b/src/qml/jsruntime/qv4runtime.cpp
@@ -737,6 +737,39 @@ ReturnedValue Runtime::method_iteratorNext(ExecutionEngine *engine, const Value
     return Encode(done);
 }
 
+ReturnedValue Runtime::method_iteratorClose(ExecutionEngine *engine, const Value &iterator, const Value &done)
+{
+    Q_ASSERT(iterator.isObject());
+    Q_ASSERT(done.isBoolean());
+    if (done.booleanValue())
+        return Encode::undefined();
+
+    Scope scope(engine);
+    bool hadException = engine->hasException;
+    ScopedValue e(scope);
+    if (hadException) {
+        e = *engine->exceptionValue;
+        engine->hasException = false;
+    }
+    ScopedFunctionObject f(scope, static_cast<const Object &>(iterator).get(engine->id_return()));
+    ScopedObject o(scope);
+    if (f) {
+        JSCallData cData(scope, 0, nullptr, &iterator);
+        o = f->call(cData);
+    }
+    if (hadException || !f) {
+        *engine->exceptionValue = e;
+        engine->hasException = hadException;
+        return Encode::undefined();
+    }
+    if (engine->hasException)
+        return Encode::undefined();
+
+    if (!o)
+        return engine->throwTypeError();
+    return Encode::undefined();
+}
+
 ReturnedValue Runtime::method_destructureRestElement(ExecutionEngine *engine, const Value &iterator)
 {
     Q_ASSERT(iterator.isObject());
diff --git a/src/qml/jsruntime/qv4runtimeapi_p.h b/src/qml/jsruntime/qv4runtimeapi_p.h
index a22a053cd1..266dc36680 100644
--- a/src/qml/jsruntime/qv4runtimeapi_p.h
+++ b/src/qml/jsruntime/qv4runtimeapi_p.h
@@ -147,6 +147,7 @@ struct ExceptionCheck<void (*)(QV4::NoThrowEngine *, A, B, C)> {
     /* for-in, for-of and array destructuring */ \
     F(ReturnedValue, getIterator, (ExecutionEngine *engine, const Value &in, int iterator)) \
     F(ReturnedValue, iteratorNext, (ExecutionEngine *engine, const Value &iterator, Value *value)) \
+    F(ReturnedValue, iteratorClose, (ExecutionEngine *engine, const Value &iterator, const Value &done)) \
     F(ReturnedValue, destructureRestElement, (ExecutionEngine *engine, const Value &iterator)) \
     \
     /* unary operators */ \
diff --git a/src/qml/jsruntime/qv4vme_moth.cpp b/src/qml/jsruntime/qv4vme_moth.cpp
index 17ed1ae044..f39fd39a98 100644
--- a/src/qml/jsruntime/qv4vme_moth.cpp
+++ b/src/qml/jsruntime/qv4vme_moth.cpp
@@ -965,6 +965,12 @@ QV4::ReturnedValue VME::interpret(CppStackFrame &frame, const uchar *code)
         CHECK_EXCEPTION;
     MOTH_END_INSTR(IteratorNext)
 
+    MOTH_BEGIN_INSTR(IteratorClose)
+        STORE_ACC();
+        acc = Runtime::method_iteratorClose(engine, accumulator, STACK_VALUE(done));
+        CHECK_EXCEPTION;
+    MOTH_END_INSTR(IteratorClose)
+
     MOTH_BEGIN_INSTR(DestructureRestElement)
         STORE_ACC();
         acc = Runtime::method_destructureRestElement(engine, ACC);