Fix array destructuring

Array destructuring should use iterator objects, not integer
indexes.

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

8 files changed, 36 insertions, 16 deletions

diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index 97b71ddbcc..ffb318168c 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -476,27 +476,44 @@ void Codegen::destructureElementList(const Codegen::Reference &array, PatternEle
 {
     RegisterScope scope(this);
 
-    int index = 0;
+    Reference iterator = Reference::fromStackSlot(this);
+
+    array.loadInAccumulator();
+    Instruction::GetIterator iteratorObjInstr;
+    iteratorObjInstr.iterator = 1; // ForEachType::Of
+    bytecodeGenerator->addInstruction(iteratorObjInstr);
+    iterator.storeConsumeAccumulator();
+
+    BytecodeGenerator::Label end = bytecodeGenerator->newLabel();
+
     for (PatternElementList *p = bindingList; p; p = p->next) {
-        for (Elision *elision = p->elision; elision; elision = elision->next)
-            ++index;
+        for (Elision *elision = p->elision; elision; elision = elision->next) {
+            iterator.loadInAccumulator();
+            Instruction::IteratorNext next;
+            next.returnUndefinedWhenDone = true;
+            bytecodeGenerator->addInstruction(next);
+        }
 
         RegisterScope scope(this);
 
-        Reference idx = Reference::fromConst(this, Encode(index));
-        Reference property = Reference::fromSubscript(array, idx);
+        iterator.loadInAccumulator();
+        Instruction::IteratorNext next;
+        next.returnUndefinedWhenDone = true;
+        bytecodeGenerator->addInstruction(next);
         PatternElement *e = p->element;
         if (!e)
             continue;
         if (e->type != PatternElement::RestElement) {
-            initializeAndDestructureBindingElement(e, property);
-            if (hasError)
+            initializeAndDestructureBindingElement(e, Reference::fromAccumulator(this));
+            if (hasError) {
+                end.link();
                 return;
+            }
         } else {
             throwSyntaxError(bindingList->firstSourceLocation(), QString::fromLatin1("Support for rest elements in binding arrays not implemented!"));
         }
-        ++index;
     }
+    end.link();
 }
 
 void Codegen::destructurePattern(Pattern *p, const Reference &rhs)
@@ -2666,6 +2683,7 @@ bool Codegen::visit(ForEachStatement *ast)
     in.link();
     iterator.loadInAccumulator();
     Instruction::IteratorNext next;
+    next.returnUndefinedWhenDone = false;
     bytecodeGenerator->addInstruction(next);
     Instruction::JumpEmpty jump;
     BytecodeGenerator::Jump done = bytecodeGenerator->addJumpInstruction(jump);
diff --git a/src/qml/compiler/qv4instr_moth.cpp b/src/qml/compiler/qv4instr_moth.cpp
index 58aa7d7cfd..7446848fb5 100644
--- a/src/qml/compiler/qv4instr_moth.cpp
+++ b/src/qml/compiler/qv4instr_moth.cpp
@@ -434,6 +434,7 @@ void dumpBytecode(const char *code, int len, int nLocals, int nFormals, int /*st
         MOTH_END_INSTR(GetIterator)
 
         MOTH_BEGIN_INSTR(IteratorNext)
+            d << returnUndefinedWhenDone;
         MOTH_END_INSTR(IteratorNext)
 
         MOTH_BEGIN_INSTR(DeleteMember)
diff --git a/src/qml/compiler/qv4instr_moth_p.h b/src/qml/compiler/qv4instr_moth_p.h
index 301a43a519..de01b808e9 100644
--- a/src/qml/compiler/qv4instr_moth_p.h
+++ b/src/qml/compiler/qv4instr_moth_p.h
@@ -122,7 +122,7 @@ QT_BEGIN_NAMESPACE
 #define INSTR_PopScriptContext(op) INSTRUCTION(op, PopScriptContext, 0)
 #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, 0)
+#define INSTR_IteratorNext(op) INSTRUCTION(op, IteratorNext, 1, returnUndefinedWhenDone)
 #define INSTR_DeleteMember(op) INSTRUCTION(op, DeleteMember, 2, member, base)
 #define INSTR_DeleteSubscript(op) INSTRUCTION(op, DeleteSubscript, 2, base, index)
 #define INSTR_DeleteName(op) INSTRUCTION(op, DeleteName, 1, name)
diff --git a/src/qml/jit/qv4jit.cpp b/src/qml/jit/qv4jit.cpp
index 77024176ae..e0a5a0590c 100644
--- a/src/qml/jit/qv4jit.cpp
+++ b/src/qml/jit/qv4jit.cpp
@@ -714,10 +714,11 @@ void BaselineJIT::generate_GetIterator(int iterator)
     as->checkException();
 }
 
-void BaselineJIT::generate_IteratorNext()
+void BaselineJIT::generate_IteratorNext(int returnUndefinedWhenDone)
 {
     as->saveAccumulatorInFrame();
-    as->prepareCallWithArgCount(2);
+    as->prepareCallWithArgCount(3);
+    as->passInt32AsArg(returnUndefinedWhenDone, 2);
     as->passAccumulatorAsArg(1);
     as->passEngineAsArg(0);
     JIT_GENERATE_RUNTIME_CALL(Runtime::method_iteratorNext, Assembler::ResultInAccumulator);
diff --git a/src/qml/jit/qv4jit_p.h b/src/qml/jit/qv4jit_p.h
index d395ea908c..942b96071f 100644
--- a/src/qml/jit/qv4jit_p.h
+++ b/src/qml/jit/qv4jit_p.h
@@ -186,7 +186,7 @@ public:
     void generate_PopScriptContext() override;
     void generate_PopContext(int reg) override;
     void generate_GetIterator(int iterator) override;
-    void generate_IteratorNext() override;
+    void generate_IteratorNext(int returnUndefinedWhenDone) override;
     void generate_DeleteMember(int member, int base) override;
     void generate_DeleteSubscript(int base, int index) override;
     void generate_DeleteName(int name) override;
diff --git a/src/qml/jsruntime/qv4runtime.cpp b/src/qml/jsruntime/qv4runtime.cpp
index eadbcb9557..312ab6f2d3 100644
--- a/src/qml/jsruntime/qv4runtime.cpp
+++ b/src/qml/jsruntime/qv4runtime.cpp
@@ -715,7 +715,7 @@ ReturnedValue Runtime::method_getIterator(ExecutionEngine *engine, const Value &
     return engine->newForInIteratorObject(o)->asReturnedValue();
 }
 
-ReturnedValue Runtime::method_iteratorNext(ExecutionEngine *engine, const Value &iterator)
+ReturnedValue Runtime::method_iteratorNext(ExecutionEngine *engine, const Value &iterator, int returnUndefinedWhenDone)
 {
     Q_ASSERT(iterator.isObject());
 
@@ -729,7 +729,7 @@ ReturnedValue Runtime::method_iteratorNext(ExecutionEngine *engine, const Value
         return engine->throwTypeError();
     ScopedValue v(scope, o->get(engine->id_done()));
     if (v->toBoolean() == true)
-        return Primitive::emptyValue().asReturnedValue();
+        return returnUndefinedWhenDone ? Encode::undefined() : Primitive::emptyValue().asReturnedValue();
     return o->get(engine->id_value());
 }
 
diff --git a/src/qml/jsruntime/qv4runtimeapi_p.h b/src/qml/jsruntime/qv4runtimeapi_p.h
index 1e14c23d7e..0b46a1627d 100644
--- a/src/qml/jsruntime/qv4runtimeapi_p.h
+++ b/src/qml/jsruntime/qv4runtimeapi_p.h
@@ -146,7 +146,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)) \
+    F(ReturnedValue, iteratorNext, (ExecutionEngine *engine, const Value &iterator, int returnUndefinedWhenDone)) \
     \
     /* unary operators */ \
     F(ReturnedValue, uMinus, (const Value &value)) \
diff --git a/src/qml/jsruntime/qv4vme_moth.cpp b/src/qml/jsruntime/qv4vme_moth.cpp
index 9fd57e0dcb..3565ec10ae 100644
--- a/src/qml/jsruntime/qv4vme_moth.cpp
+++ b/src/qml/jsruntime/qv4vme_moth.cpp
@@ -961,7 +961,7 @@ QV4::ReturnedValue VME::interpret(CppStackFrame &frame, const uchar *code)
 
     MOTH_BEGIN_INSTR(IteratorNext)
         STORE_ACC();
-        acc = Runtime::method_iteratorNext(engine, accumulator);
+        acc = Runtime::method_iteratorNext(engine, accumulator, returnUndefinedWhenDone);
         CHECK_EXCEPTION;
     MOTH_END_INSTR(IteratorNext)