Fix handling of elisions in destructuring expressions

We need to iterator over elisions at the end, as those could
trigger side effects by calling iterator.next()

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

5 files changed, 17 insertions, 24 deletions

diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index 2aa1cef451..1a47730979 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -595,7 +595,7 @@ void Codegen::destructureElementList(const Codegen::Reference &array, PatternEle
             Instruction::IteratorNext next;
             next.value = iteratorValue.stackSlot();
             bytecodeGenerator->addInstruction(next);
-            bool last = !p->next;
+            bool last = !p->next || (!p->next->elision && !p->next->element);
             if (last)
                 iteratorDone.storeConsumeAccumulator();
             if (e->type != PatternElement::RestElement) {
@@ -833,6 +833,9 @@ bool Codegen::visit(ArrayPattern *ast)
     };
 
     for (PatternElementList *it = ast->elements; it; it = it->next) {
+        for (Elision *elision = it->elision; elision; elision = elision->next)
+            push(nullptr);
+
         PatternElement *e = it->element;
         if (!e)
             continue;
@@ -841,15 +844,10 @@ bool Codegen::visit(ArrayPattern *ast)
             return false;
         }
 
-        for (Elision *elision = it->elision; elision; elision = elision->next)
-            push(nullptr);
-
         push(e->initializer);
         if (hasError)
             return false;
     }
-    for (Elision *elision = ast->elision; elision; elision = elision->next)
-        push(nullptr);
 
     if (args == -1) {
         Q_ASSERT(argc == 0);
diff --git a/src/qml/compiler/qv4compilerscanfunctions.cpp b/src/qml/compiler/qv4compilerscanfunctions.cpp
index 9605b72b76..e371280b55 100644
--- a/src/qml/compiler/qv4compilerscanfunctions.cpp
+++ b/src/qml/compiler/qv4compilerscanfunctions.cpp
@@ -184,10 +184,6 @@ bool ScanFunctions::visit(ArrayPattern *ast)
             ++index;
         ++index;
     }
-    if (ast->elision) {
-        for (Elision *elision = ast->elision->next; elision; elision = elision->next)
-            ++index;
-    }
     _context->maxNumberOfArguments = qMax(_context->maxNumberOfArguments, index);
     return true;
 }
diff --git a/src/qml/parser/qqmljs.g b/src/qml/parser/qqmljs.g
index bc38655607..f927473e53 100644
--- a/src/qml/parser/qqmljs.g
+++ b/src/qml/parser/qqmljs.g
@@ -1585,7 +1585,10 @@ RegularExpressionLiteral: T_DIVIDE_EQ;
 ArrayLiteral: T_LBRACKET ElisionOpt T_RBRACKET;
 /.
     case $rule_number: {
-        AST::ArrayPattern *node = new (pool) AST::ArrayPattern(sym(2).Elision);
+        AST::PatternElementList *list = nullptr;
+        if (sym(2).Elision)
+            list = (new (pool) AST::PatternElementList(sym(2).Elision, nullptr))->finish();
+        AST::ArrayPattern *node = new (pool) AST::ArrayPattern(list);
         node->lbracketToken = loc(1);
         node->rbracketToken = loc(3);
         sym(1).Node = node;
@@ -1605,7 +1608,12 @@ ArrayLiteral: T_LBRACKET ElementList T_RBRACKET;
 ArrayLiteral: T_LBRACKET ElementList T_COMMA ElisionOpt T_RBRACKET;
 /.
     case $rule_number: {
-        AST::ArrayPattern *node = new (pool) AST::ArrayPattern(sym(2).PatternElementList->finish(), sym(4).Elision);
+        auto *list = sym(2).PatternElementList;
+        if (sym(4).Elision) {
+            AST::PatternElementList *l = new (pool) AST::PatternElementList(sym(4).Elision, nullptr);
+            list = list->append(l);
+        }
+        AST::ArrayPattern *node = new (pool) AST::ArrayPattern(list->finish());
         node->lbracketToken = loc(1);
         node->commaToken = loc(3);
         node->rbracketToken = loc(5);
diff --git a/src/qml/parser/qqmljsast.cpp b/src/qml/parser/qqmljsast.cpp
index 814262d98b..b63feb3362 100644
--- a/src/qml/parser/qqmljsast.cpp
+++ b/src/qml/parser/qqmljsast.cpp
@@ -259,10 +259,8 @@ void RegExpLiteral::accept0(Visitor *visitor)
 
 void ArrayPattern::accept0(Visitor *visitor)
 {
-    if (visitor->visit(this)) {
+    if (visitor->visit(this))
         accept(elements, visitor);
-        accept(elision, visitor);
-    }
 
     visitor->endVisit(this);
 }
@@ -358,6 +356,8 @@ bool ArrayPattern::convertLiteralToAssignmentPattern(MemoryPool *pool, SourceLoc
     if (parseMode == Binding)
         return true;
     for (auto *it = elements; it; it = it->next) {
+        if (!it->element)
+            continue;
         if (it->element->type == PatternElement::SpreadElement && it->next) {
             *errorLocation = it->element->firstSourceLocation();
             *errorMessage = QString::fromLatin1("'...' can only appear as last element in a destructuring list.");
diff --git a/src/qml/parser/qqmljsast_p.h b/src/qml/parser/qqmljsast_p.h
index 91d6410132..2d4d5662bd 100644
--- a/src/qml/parser/qqmljsast_p.h
+++ b/src/qml/parser/qqmljsast_p.h
@@ -542,18 +542,10 @@ class QML_PARSER_EXPORT ArrayPattern : public Pattern
 public:
     QQMLJS_DECLARE_AST_NODE(ArrayPattern)
 
-    ArrayPattern(Elision *e)
-        : elision(e)
-    { kind = K; }
-
     ArrayPattern(PatternElementList *elts)
         : elements(elts)
     { kind = K; }
 
-    ArrayPattern(PatternElementList *elts, Elision *e)
-        : elements(elts), elision(e)
-    { kind = K; }
-
     void accept0(Visitor *visitor) override;
 
     SourceLocation firstSourceLocation() const override
@@ -568,7 +560,6 @@ public:
 
 // attributes
     PatternElementList *elements = nullptr;
-    Elision *elision = nullptr;
     SourceLocation lbracketToken;
     SourceLocation commaToken;
     SourceLocation rbracketToken;