Fix more issues with destructuring

Fix destructuring targets that are complex lhs expressions.

There are still some failures remaining, but this fixes
another larger chunk of test cases.

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

4 files changed, 50 insertions, 30 deletions

diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index 1a47730979..d71d36845b 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -469,10 +469,27 @@ void Codegen::variableDeclarationList(VariableDeclarationList *ast)
     }
 }
 
-void Codegen::initializeAndDestructureBindingElement(AST::PatternElement *e, const Reference &baseRef)
+Codegen::Reference Codegen::targetForPatternElement(AST::PatternElement *p)
+{
+    if (!p->bindingIdentifier.isNull())
+        return referenceForName(p->bindingIdentifier, true);
+    if (!p->bindingTarget || p->destructuringPattern())
+        return Codegen::Reference::fromStackSlot(this);
+    Reference lhs = expression(p->bindingTarget);
+    if (hasError)
+        return lhs;
+    lhs = lhs.asLValue();
+    return lhs;
+}
+
+void Codegen::initializeAndDestructureBindingElement(AST::PatternElement *e, const Reference &base)
 {
     RegisterScope scope(this);
-    Reference varToStore = e->bindingIdentifier.isNull() ? Reference::fromStackSlot(this, bytecodeGenerator->newRegister()) : referenceForName(e->bindingIdentifier, true);
+    Reference baseRef = (base.isAccumulator()) ? base.storeOnStack() : base;
+    Reference varToStore = targetForPatternElement(e);
+    if (hasError)
+        return;
+
     if (e->initializer) {
         if (!baseRef.isValid()) {
             // assignment
@@ -508,13 +525,17 @@ void Codegen::initializeAndDestructureBindingElement(AST::PatternElement *e, con
         baseRef.loadInAccumulator();
         varToStore.storeConsumeAccumulator();
     }
+    Pattern *p = e->destructuringPattern();
+    if (!p)
+        return;
+
     if (!varToStore.isStackSlot())
         varToStore = varToStore.storeOnStack();
     if (PatternElementList *l = e->elementList()) {
         destructureElementList(varToStore, l);
     } else if (PatternPropertyList *p = e->propertyList()) {
         destructurePropertyList(varToStore, p);
-    } else if (e->bindingPattern) {
+    } else if (e->bindingTarget) {
         // empty binding pattern. For spec compatibility, try to coerce the argument to an object
         varToStore.loadInAccumulator();
         Instruction::ToObject toObject;
@@ -537,7 +558,7 @@ void Codegen::destructurePropertyList(const Codegen::Reference &object, PatternP
             if (hasError)
                 return;
             computedName = computedName.storeOnStack();
-            property = Reference::fromSubscript(object, computedName);
+            property = Reference::fromSubscript(object, computedName).asLValue();
         } else {
             QString propertyName = p->name->asString();
             property = Reference::fromMember(object, propertyName);
@@ -2496,7 +2517,7 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast,
             bytecodeGenerator->addInstruction(rest);
             arg.storeConsumeAccumulator();
         } else {
-            if (e->bindingPattern || e->initializer) {
+            if (e->bindingTarget || e->initializer) {
                 initializeAndDestructureBindingElement(e, arg);
                 if (hasError)
                     break;
diff --git a/src/qml/compiler/qv4codegen_p.h b/src/qml/compiler/qv4codegen_p.h
index 39309f78ce..713b040ad1 100644
--- a/src/qml/compiler/qv4codegen_p.h
+++ b/src/qml/compiler/qv4codegen_p.h
@@ -516,6 +516,7 @@ protected:
     void variableDeclaration(AST::PatternElement *ast);
     void variableDeclarationList(AST::VariableDeclarationList *ast);
 
+    Reference targetForPatternElement(AST::PatternElement *p);
     void initializeAndDestructureBindingElement(AST::PatternElement *e, const Reference &baseRef = Reference());
     void destructurePropertyList(const Reference &object, AST::PatternPropertyList *bindingList);
     void destructureElementList(const Reference &array, AST::PatternElementList *bindingList);
diff --git a/src/qml/parser/qqmljsast.cpp b/src/qml/parser/qqmljsast.cpp
index b63feb3362..e9e33f7561 100644
--- a/src/qml/parser/qqmljsast.cpp
+++ b/src/qml/parser/qqmljsast.cpp
@@ -268,7 +268,7 @@ void ArrayPattern::accept0(Visitor *visitor)
 bool ArrayPattern::isValidArrayLiteral(SourceLocation *errorLocation) const {
     for (PatternElementList *it = elements; it != nullptr; it = it->next) {
         PatternElement *e = it->element;
-        if (e && e->bindingPattern != nullptr) {
+        if (e && e->bindingTarget != nullptr) {
             if (errorLocation)
                 *errorLocation = e->firstSourceLocation();
             return false;
@@ -386,8 +386,8 @@ bool PatternElement::convertLiteralToAssignmentPattern(MemoryPool *pool, SourceL
 {
     Q_ASSERT(type == Literal || type == SpreadElement);
     Q_ASSERT(bindingIdentifier.isNull());
-    Q_ASSERT(bindingPattern == nullptr);
-    Q_ASSERT(bindingPattern == nullptr);
+    Q_ASSERT(bindingTarget == nullptr);
+    Q_ASSERT(bindingTarget == nullptr);
     Q_ASSERT(initializer);
     ExpressionNode *init = initializer;
 
@@ -399,8 +399,7 @@ bool PatternElement::convertLiteralToAssignmentPattern(MemoryPool *pool, SourceL
             *errorMessage = QString::fromLatin1("Invalid lhs expression after '...' in destructuring expression.");
             return false;
         }
-        // ### Should be binding
-        initializer = lhs;
+        bindingTarget = lhs;
         return true;
     }
     type = PatternElement::Binding;
@@ -422,21 +421,18 @@ bool PatternElement::convertLiteralToAssignmentPattern(MemoryPool *pool, SourceL
         *errorMessage = QString::fromLatin1("Destructuring target is not a left hand side expression.");
         return false;
     }
-
-    if (auto *p = lhs->patternCast()) {
-        bindingPattern = p;
-        if (!p->convertLiteralToAssignmentPattern(pool, errorLocation, errorMessage))
-            return false;
-        return true;
-    }
     if (auto *i = cast<IdentifierExpression *>(lhs)) {
         bindingIdentifier = i->name.toString();
         identifierToken = i->identifierToken;
         return true;
     }
-    *errorLocation = lastSourceLocation();
-    *errorMessage = QLatin1String("Unimplemented!");
-    return false;
+
+    bindingTarget = lhs;
+    if (auto *p = lhs->patternCast()) {
+        if (!p->convertLiteralToAssignmentPattern(pool, errorLocation, errorMessage))
+            return false;
+    }
+    return true;
 }
 
 bool PatternProperty::convertLiteralToAssignmentPattern(MemoryPool *pool, SourceLocation *errorLocation, QString *errorMessage)
@@ -1209,7 +1205,7 @@ void TaggedTemplate::accept0(Visitor *visitor)
 void PatternElement::accept0(Visitor *visitor)
 {
     if (visitor->visit(this)) {
-        accept(bindingPattern, visitor);
+        accept(bindingTarget, visitor);
         accept(initializer, visitor);
     }
 
@@ -1218,7 +1214,7 @@ void PatternElement::accept0(Visitor *visitor)
 
 void PatternElement::boundNames(QStringList *names)
 {
-    if (bindingPattern) {
+    if (bindingTarget) {
         if (PatternElementList *e = elementList())
             e->boundNames(names);
         else if (PatternPropertyList *p = propertyList())
@@ -1252,7 +1248,7 @@ void PatternProperty::accept0(Visitor *visitor)
 {
     if (visitor->visit(this)) {
         accept(name, visitor);
-        accept(bindingPattern, visitor);
+        accept(bindingTarget, visitor);
         accept(initializer, visitor);
     }
 
diff --git a/src/qml/parser/qqmljsast_p.h b/src/qml/parser/qqmljsast_p.h
index 2d4d5662bd..66a4e357e9 100644
--- a/src/qml/parser/qqmljsast_p.h
+++ b/src/qml/parser/qqmljsast_p.h
@@ -678,7 +678,7 @@ public:
     }
 
     PatternElement(Pattern *pattern, ExpressionNode *i = nullptr, Type t = Binding)
-        : bindingPattern(pattern), initializer(i), type(t)
+        : bindingTarget(pattern), initializer(i), type(t)
     {
         Q_ASSERT(t >= RestElement);
         kind = K;
@@ -688,13 +688,15 @@ public:
     virtual bool convertLiteralToAssignmentPattern(MemoryPool *pool, SourceLocation *errorLocation, QString *errorMessage);
 
     SourceLocation firstSourceLocation() const override
-    { return identifierToken.isValid() ? identifierToken : (bindingPattern ? bindingPattern->firstSourceLocation() : initializer->firstSourceLocation()); }
+    { return identifierToken.isValid() ? identifierToken : (bindingTarget ? bindingTarget->firstSourceLocation() : initializer->firstSourceLocation()); }
 
     SourceLocation lastSourceLocation() const override
-    { return initializer ? initializer->lastSourceLocation() : (bindingPattern ? bindingPattern->lastSourceLocation() : identifierToken); }
+    { return initializer ? initializer->lastSourceLocation() : (bindingTarget ? bindingTarget->lastSourceLocation() : identifierToken); }
 
-    PatternElementList *elementList() const { ArrayPattern *a = cast<ArrayPattern *>(bindingPattern); return a ? a->elements : nullptr; }
-    PatternPropertyList *propertyList() const { ObjectPattern *o = cast<ObjectPattern *>(bindingPattern); return o ? o->properties : nullptr;  }
+    ExpressionNode *destructuringTarget() const { return bindingTarget; }
+    Pattern *destructuringPattern() const { return bindingTarget ? bindingTarget->patternCast() : nullptr; }
+    PatternElementList *elementList() const { ArrayPattern *a = cast<ArrayPattern *>(bindingTarget); return a ? a->elements : nullptr; }
+    PatternPropertyList *propertyList() const { ObjectPattern *o = cast<ObjectPattern *>(bindingTarget); return o ? o->properties : nullptr;  }
 
     bool isVariableDeclaration() const { return scope != VariableScope::NoScope; }
     bool isLexicallyScoped() const { return scope == VariableScope::Let || scope == VariableScope::Const; }
@@ -704,7 +706,7 @@ public:
 // attributes
     SourceLocation identifierToken;
     QString bindingIdentifier;
-    Pattern *bindingPattern = nullptr;
+    ExpressionNode *bindingTarget = nullptr;
     ExpressionNode *initializer = nullptr;
     Type type = Literal;
     // when used in a VariableDeclarationList
@@ -2164,7 +2166,7 @@ public:
             PatternElement *e = formals->element;
             if (e && e->type == PatternElement::RestElement)
                 return false;
-            if (e && (e->initializer || e->bindingPattern))
+            if (e && (e->initializer || e->bindingTarget))
                 return false;
             formals = formals->next;
         }