Fix group property bindings for aliases that point to id objects

When declaring bindings within a group property and that group property
itself is a locally declared alias, then by the time we try to determine
property caches for the group property we will fail as the aliases
haven't been resolved yet.

To fix this we can keep track of such group property declarations
(encapsulated in the QQmlInstantiatingBindingContext that has all we
need) and after we've resolved the aliases (added them to the property
caches), we can go back and fill in the entries in the propertyCaches
array for the group properties.

Task-number: QTBUG-51043
Change-Id: I5613513db3977934bcc51a3df530de47d57326f9
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
Reviewed-by: Michael Brasser <michael.brasser@live.com>

5 files changed, 99 insertions, 40 deletions

diff --git a/src/qml/compiler/qqmlirbuilder_p.h b/src/qml/compiler/qqmlirbuilder_p.h
index 7f2cf40611..c2cf18e3c4 100644
--- a/src/qml/compiler/qqmlirbuilder_p.h
+++ b/src/qml/compiler/qqmlirbuilder_p.h
@@ -665,6 +665,17 @@ private:
 
 } // namespace QmlIR
 
+struct QQmlCompileError
+{
+    QQmlCompileError() {}
+    QQmlCompileError(const QV4::CompiledData::Location &location, const QString &description)
+        : location(location), description(description) {}
+    QV4::CompiledData::Location location;
+    QString description;
+
+    bool isSet() const { return !description.isEmpty(); }
+};
+
 QT_END_NAMESPACE
 
 #endif // QQMLIRBUILDER_P_H
diff --git a/src/qml/compiler/qqmlpropertycachecreator.cpp b/src/qml/compiler/qqmlpropertycachecreator.cpp
index 4f8b48f52e..fd22cd58f1 100644
--- a/src/qml/compiler/qqmlpropertycachecreator.cpp
+++ b/src/qml/compiler/qqmlpropertycachecreator.cpp
@@ -45,26 +45,54 @@ QT_BEGIN_NAMESPACE
 
 QAtomicInt QQmlPropertyCacheCreatorBase::classIndexCounter(0);
 
-QQmlBindingInstantiationContext::QQmlBindingInstantiationContext()
-    : referencingObjectIndex(-1)
-    , instantiatingBinding(nullptr)
-    , instantiatingProperty(nullptr)
+QQmlBindingInstantiationContext::QQmlBindingInstantiationContext(int referencingObjectIndex, const QV4::CompiledData::Binding *instantiatingBinding,
+                                                                 const QString &instantiatingPropertyName, QQmlPropertyCache *referencingObjectPropertyCache)
+    : referencingObjectIndex(referencingObjectIndex)
+    , instantiatingBinding(instantiatingBinding)
+    , instantiatingPropertyName(instantiatingPropertyName)
+    , referencingObjectPropertyCache(referencingObjectPropertyCache)
 {
+}
 
+bool QQmlBindingInstantiationContext::resolveInstantiatingProperty()
+{
+    if (!instantiatingBinding || instantiatingBinding->type != QV4::CompiledData::Binding::Type_GroupProperty)
+        return true;
+
+    Q_ASSERT(referencingObjectIndex >= 0);
+    Q_ASSERT(referencingObjectPropertyCache);
+    Q_ASSERT(instantiatingBinding->propertyNameIndex != 0);
+
+    bool notInRevision = false;
+    instantiatingProperty = QmlIR::PropertyResolver(referencingObjectPropertyCache).property(instantiatingPropertyName, &notInRevision, QmlIR::PropertyResolver::IgnoreRevision);
+    return instantiatingProperty != nullptr;
 }
 
-QQmlBindingInstantiationContext::QQmlBindingInstantiationContext(int referencingObjectIndex, const QV4::CompiledData::Binding *instantiatingBinding, const QString &instantiatingPropertyName, const QQmlPropertyCache *referencingObjectPropertyCache)
-    : referencingObjectIndex(referencingObjectIndex)
-    , instantiatingBinding(instantiatingBinding)
-    , instantiatingProperty(nullptr)
+QQmlRefPointer<QQmlPropertyCache> QQmlBindingInstantiationContext::instantiatingPropertyCache(QQmlEnginePrivate *enginePrivate) const
 {
-    if (instantiatingBinding && instantiatingBinding->type == QV4::CompiledData::Binding::Type_GroupProperty) {
-        Q_ASSERT(referencingObjectIndex >= 0);
-        Q_ASSERT(referencingObjectPropertyCache);
-        Q_ASSERT(instantiatingBinding->propertyNameIndex != 0);
+    if (instantiatingProperty) {
+        if (instantiatingProperty->isQObject()) {
+            return enginePrivate->rawPropertyCacheForType(instantiatingProperty->propType(), instantiatingProperty->typeMinorVersion());
+        } else if (const QMetaObject *vtmo = QQmlValueTypeFactory::metaObjectForMetaType(instantiatingProperty->propType())) {
+            return enginePrivate->cache(vtmo);
+        }
+    }
+    return QQmlRefPointer<QQmlPropertyCache>();
+}
+
+void QQmlPendingGroupPropertyBindings::resolveMissingPropertyCaches(QQmlEnginePrivate *enginePrivate, QQmlPropertyCacheVector *propertyCaches) const
+{
+    for (QQmlBindingInstantiationContext pendingBinding: *this) {
+        const int groupPropertyObjectIndex = pendingBinding.instantiatingBinding->value.objectIndex;
+
+        if (propertyCaches->at(groupPropertyObjectIndex))
+            continue;
+
+        if (!pendingBinding.resolveInstantiatingProperty())
+            continue;
 
-        bool notInRevision = false;
-        instantiatingProperty = QmlIR::PropertyResolver(referencingObjectPropertyCache).property(instantiatingPropertyName, &notInRevision, QmlIR::PropertyResolver::IgnoreRevision);
+        auto cache = pendingBinding.instantiatingPropertyCache(enginePrivate);
+        propertyCaches->set(groupPropertyObjectIndex, cache);
     }
 }
 
diff --git a/src/qml/compiler/qqmlpropertycachecreator_p.h b/src/qml/compiler/qqmlpropertycachecreator_p.h
index 2af0fe0036..8bbc8291b4 100644
--- a/src/qml/compiler/qqmlpropertycachecreator_p.h
+++ b/src/qml/compiler/qqmlpropertycachecreator_p.h
@@ -50,18 +50,31 @@
 // We mean it.
 //
 
-#include "qqmltypecompiler_p.h"
 #include <private/qqmlvaluetype_p.h>
 #include <private/qqmlengine_p.h>
 
 QT_BEGIN_NAMESPACE
 
 struct QQmlBindingInstantiationContext {
-    QQmlBindingInstantiationContext();
-    QQmlBindingInstantiationContext(int referencingObjectIndex, const QV4::CompiledData::Binding *instantiatingBinding, const QString &instantiatingPropertyName, const QQmlPropertyCache *referencingObjectPropertyCache);
-    int referencingObjectIndex;
-    const QV4::CompiledData::Binding *instantiatingBinding;
-    QQmlPropertyData *instantiatingProperty;
+    QQmlBindingInstantiationContext() {}
+    QQmlBindingInstantiationContext(int referencingObjectIndex,
+                                    const QV4::CompiledData::Binding *instantiatingBinding,
+                                    const QString &instantiatingPropertyName,
+                                    QQmlPropertyCache *referencingObjectPropertyCache);
+
+    bool resolveInstantiatingProperty();
+    QQmlRefPointer<QQmlPropertyCache> instantiatingPropertyCache(QQmlEnginePrivate *enginePrivate) const;
+
+    int referencingObjectIndex = -1;
+    const QV4::CompiledData::Binding *instantiatingBinding = nullptr;
+    QString instantiatingPropertyName;
+    QQmlRefPointer<QQmlPropertyCache> referencingObjectPropertyCache;
+    QQmlPropertyData *instantiatingProperty = nullptr;
+};
+
+struct QQmlPendingGroupPropertyBindings : public QVector<QQmlBindingInstantiationContext>
+{
+    void resolveMissingPropertyCaches(QQmlEnginePrivate *enginePrivate, QQmlPropertyCacheVector *propertyCaches) const;
 };
 
 struct QQmlPropertyCacheCreatorBase
@@ -77,7 +90,10 @@ class QQmlPropertyCacheCreator : public QQmlPropertyCacheCreatorBase
 public:
     typedef typename ObjectContainer::CompiledObject CompiledObject;
 
-    QQmlPropertyCacheCreator(QQmlPropertyCacheVector *propertyCaches, QQmlEnginePrivate *enginePrivate, const ObjectContainer *objectContainer, const QQmlImports *imports);
+    QQmlPropertyCacheCreator(QQmlPropertyCacheVector *propertyCaches,
+                             QQmlPendingGroupPropertyBindings *pendingGroupPropertyBindings,
+                             QQmlEnginePrivate *enginePrivate,
+                             const ObjectContainer *objectContainer, const QQmlImports *imports);
 
     QQmlCompileError buildMetaObjects();
 
@@ -92,14 +108,19 @@ protected:
     const ObjectContainer * const objectContainer;
     const QQmlImports * const imports;
     QQmlPropertyCacheVector *propertyCaches;
+    QQmlPendingGroupPropertyBindings *pendingGroupPropertyBindings;
 };
 
 template <typename ObjectContainer>
-inline QQmlPropertyCacheCreator<ObjectContainer>::QQmlPropertyCacheCreator(QQmlPropertyCacheVector *propertyCaches, QQmlEnginePrivate *enginePrivate, const ObjectContainer *objectContainer, const QQmlImports *imports)
+inline QQmlPropertyCacheCreator<ObjectContainer>::QQmlPropertyCacheCreator(QQmlPropertyCacheVector *propertyCaches,
+                                                                           QQmlPendingGroupPropertyBindings *pendingGroupPropertyBindings,
+                                                                           QQmlEnginePrivate *enginePrivate,
+                                                                           const ObjectContainer *objectContainer, const QQmlImports *imports)
     : enginePrivate(enginePrivate)
     , objectContainer(objectContainer)
     , imports(imports)
     , propertyCaches(propertyCaches)
+    , pendingGroupPropertyBindings(pendingGroupPropertyBindings)
 {
     propertyCaches->resize(objectContainer->objectCount());
 }
@@ -169,6 +190,14 @@ inline QQmlCompileError QQmlPropertyCacheCreator<ObjectContainer>::buildMetaObje
         for ( ; binding != end; ++binding)
             if (binding->type >= QV4::CompiledData::Binding::Type_Object) {
                 QQmlBindingInstantiationContext context(objectIndex, &(*binding), stringAt(binding->propertyNameIndex), thisCache);
+
+                // Binding to group property where we failed to look up the type of the
+                // property? Possibly a group property that is an alias that's not resolved yet.
+                // Let's attempt to resolve it after we're done with the aliases and fill in the
+                // propertyCaches entry then.
+                if (!context.resolveInstantiatingProperty())
+                    pendingGroupPropertyBindings->append(context);
+
                 QQmlCompileError error = buildMetaObjectRecursively(binding->value.objectIndex, context);
                 if (error.isSet())
                     return error;
@@ -183,11 +212,7 @@ template <typename ObjectContainer>
 inline QQmlPropertyCache *QQmlPropertyCacheCreator<ObjectContainer>::propertyCacheForObject(const CompiledObject *obj, const QQmlBindingInstantiationContext &context, QQmlCompileError *error) const
 {
     if (context.instantiatingProperty) {
-        if (context.instantiatingProperty->isQObject()) {
-            return enginePrivate->rawPropertyCacheForType(context.instantiatingProperty->propType(), context.instantiatingProperty->typeMinorVersion());
-        } else if (const QMetaObject *vtmo = QQmlValueTypeFactory::metaObjectForMetaType(context.instantiatingProperty->propType())) {
-            return enginePrivate->cache(vtmo);
-        }
+        return context.instantiatingPropertyCache(enginePrivate);
     } else if (obj->inheritedTypeNameIndex != 0) {
         auto *typeRef = objectContainer->resolvedTypes.value(obj->inheritedTypeNameIndex);
         Q_ASSERT(typeRef);
diff --git a/src/qml/compiler/qqmltypecompiler.cpp b/src/qml/compiler/qqmltypecompiler.cpp
index 3415e1c073..a896745b3f 100644
--- a/src/qml/compiler/qqmltypecompiler.cpp
+++ b/src/qml/compiler/qqmltypecompiler.cpp
@@ -45,7 +45,6 @@
 #include <private/qqmlvmemetaobject_p.h>
 #include <private/qqmlcomponent_p.h>
 
-#include "qqmlpropertycachecreator_p.h"
 //#include "qv4jssimplifier_p.h"
 
 #define COMPILE_EXCEPTION(token, desc) \
@@ -79,8 +78,12 @@ QV4::CompiledData::CompilationUnit *QQmlTypeCompiler::compile()
             customParsers.insert(it.key(), customParser);
     }
 
+    QQmlPendingGroupPropertyBindings pendingGroupPropertyBindings;
+
+
     {
-        QQmlPropertyCacheCreator<QQmlTypeCompiler> propertyCacheBuilder(&m_propertyCaches, engine, this, imports());
+        QQmlPropertyCacheCreator<QQmlTypeCompiler> propertyCacheBuilder(&m_propertyCaches, &pendingGroupPropertyBindings,
+                                                                        engine, this, imports());
         QQmlCompileError error = propertyCacheBuilder.buildMetaObjects();
         if (error.isSet()) {
             recordError(error);
@@ -122,6 +125,8 @@ QV4::CompiledData::CompilationUnit *QQmlTypeCompiler::compile()
         QQmlComponentAndAliasResolver resolver(this);
         if (!resolver.resolve())
             return nullptr;
+
+        pendingGroupPropertyBindings.resolveMissingPropertyCaches(engine, &m_propertyCaches);
     }
 
     {
diff --git a/src/qml/compiler/qqmltypecompiler_p.h b/src/qml/compiler/qqmltypecompiler_p.h
index d905b956c7..b8eddcb9b2 100644
--- a/src/qml/compiler/qqmltypecompiler_p.h
+++ b/src/qml/compiler/qqmltypecompiler_p.h
@@ -55,6 +55,7 @@
 #include <qhash.h>
 #include <private/qqmltypeloader_p.h>
 #include <private/qqmlirbuilder_p.h>
+#include <private/qqmlpropertycachecreator_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -74,17 +75,6 @@ struct Location;
 }
 }
 
-struct QQmlCompileError
-{
-    QQmlCompileError() {}
-    QQmlCompileError(const QV4::CompiledData::Location &location, const QString &description)
-        : location(location), description(description) {}
-    QV4::CompiledData::Location location;
-    QString description;
-
-    bool isSet() const { return !description.isEmpty(); }
-};
-
 struct QQmlTypeCompiler
 {
     Q_DECLARE_TR_FUNCTIONS(QQmlTypeCompiler)