diff --git a/src/qml/qml/qqmlcomponentandaliasresolver_p.h b/src/qml/qml/qqmlcomponentandaliasresolver_p.h index e1c3cb2dea..a2758d385c 100644 --- a/src/qml/qml/qqmlcomponentandaliasresolver_p.h +++ b/src/qml/qml/qqmlcomponentandaliasresolver_p.h @@ -27,6 +27,12 @@ QT_BEGIN_NAMESPACE Q_DECLARE_LOGGING_CATEGORY(lcQmlTypeCompiler); +// This class primarily resolves component boundaries in a document. +// With the information about boundaries, it then goes on to resolve aliases and generalized +// group properties. Both rely on IDs as first part of their expressions and the IDs have +// to be located in surrounding components. That's why we have to do this with the component +// boundaries in mind. + template class QQmlComponentAndAliasResolver { @@ -55,12 +61,14 @@ private: [[nodiscard]] bool markAsComponent(int index) const; [[nodiscard]] AliasResolutionResult resolveAliasesInObject( const CompiledObject &component, int objectIndex, QQmlError *error); + void resolveGeneralizedGroupProperty(const CompiledObject &component, CompiledBinding *binding); [[nodiscard]] bool wrapImplicitComponent(CompiledBinding *binding); [[nodiscard]] QQmlError findAndRegisterImplicitComponents( const CompiledObject *obj, const QQmlPropertyCache::ConstPtr &propertyCache); [[nodiscard]] QQmlError collectIdsAndAliases(int objectIndex); [[nodiscard]] QQmlError resolveAliases(int componentIndex); + void resolveGeneralizedGroupProperties(int componentIndex); [[nodiscard]] QQmlError resolveComponentsInInlineComponentRoot(int root); QString stringAt(int idx) const { return m_compiler->stringAt(idx); } @@ -111,6 +119,7 @@ private: // indices of the objects that are actually Component {} QVector m_componentRoots; QVector m_objectsWithAliases; + QVector m_generalizedGroupProperties; typename ObjectContainer::IdToObjectMap m_idToObjectIndex; }; @@ -330,6 +339,7 @@ QQmlError QQmlComponentAndAliasResolver::resolve(int root) m_idToObjectIndex.clear(); m_objectsWithAliases.clear(); + m_generalizedGroupProperties.clear(); if (const QQmlError error = collectIdsAndAliases(rootBinding->value.objectIndex); error.isValid()) { @@ -340,17 +350,24 @@ QQmlError QQmlComponentAndAliasResolver::resolve(int root) if (const QQmlError error = resolveAliases(m_componentRoots.at(i)); error.isValid()) return error; + + resolveGeneralizedGroupProperties(m_componentRoots.at(i)); } // Collect ids and aliases for root m_idToObjectIndex.clear(); m_objectsWithAliases.clear(); + m_generalizedGroupProperties.clear(); if (const QQmlError error = collectIdsAndAliases(root); error.isValid()) return error; allocateNamedObjects(m_compiler->objectAt(root)); - return resolveAliases(root); + if (const QQmlError error = resolveAliases(root); error.isValid()) + return error; + + resolveGeneralizedGroupProperties(root); + return QQmlError(); } template @@ -375,9 +392,19 @@ QQmlError QQmlComponentAndAliasResolver::collectIdsAndAliases(i for (auto binding = obj->bindingsBegin(), end = obj->bindingsEnd(); binding != end; ++binding) { switch (binding->type()) { + case QV4::CompiledData::Binding::Type_GroupProperty: { + const auto *inner = m_compiler->objectAt(binding->value.objectIndex); + if (m_compiler->stringAt(inner->inheritedTypeNameIndex).isEmpty()) { + const auto cache = m_propertyCaches->at(objectIndex); + if (!cache || !cache->property( + m_compiler->stringAt(binding->propertyNameIndex), nullptr, nullptr)) { + m_generalizedGroupProperties.append(binding); + } + } + } + Q_FALLTHROUGH(); case QV4::CompiledData::Binding::Type_Object: case QV4::CompiledData::Binding::Type_AttachedProperty: - case QV4::CompiledData::Binding::Type_GroupProperty: if (const QQmlError error = collectIdsAndAliases(binding->value.objectIndex); error.isValid()) { return error; @@ -439,6 +466,15 @@ QQmlError QQmlComponentAndAliasResolver::resolveAliases(int com return QQmlError(); } +template +void QQmlComponentAndAliasResolver::resolveGeneralizedGroupProperties( + int componentIndex) +{ + const auto &component = *m_compiler->objectAt(componentIndex); + for (CompiledBinding *binding : m_generalizedGroupProperties) + resolveGeneralizedGroupProperty(component, binding); +} + QT_END_NAMESPACE #endif // QQMLCOMPONENTANDALIASRESOLVER_P_H diff --git a/src/qml/qml/qqmlpropertycachecreator.cpp b/src/qml/qml/qqmlpropertycachecreator.cpp index 780418380a..f6efe8d358 100644 --- a/src/qml/qml/qqmlpropertycachecreator.cpp +++ b/src/qml/qml/qqmlpropertycachecreator.cpp @@ -183,12 +183,7 @@ void QQmlPendingGroupPropertyBindings::resolveMissingPropertyCaches( if (propertyCaches->at(groupPropertyObjectIndex)) continue; - if (pendingBinding.instantiatingPropertyName.isEmpty()) { - // Generalized group property. - auto cache = propertyCaches->at(pendingBinding.referencingObjectIndex); - propertyCaches->set(groupPropertyObjectIndex, cache); - continue; - } + Q_ASSERT(!pendingBinding.instantiatingPropertyName.isEmpty()); if (!pendingBinding.referencingObjectPropertyCache) { pendingBinding.referencingObjectPropertyCache diff --git a/src/qml/qml/qqmlpropertycachecreator_p.h b/src/qml/qml/qqmlpropertycachecreator_p.h index 1067dd4c22..93987a1123 100644 --- a/src/qml/qml/qqmlpropertycachecreator_p.h +++ b/src/qml/qml/qqmlpropertycachecreator_p.h @@ -375,23 +375,6 @@ inline QQmlPropertyCache::ConstPtr QQmlPropertyCacheCreator::pr return nullptr; } return QQmlMetaType::propertyCache(attachedMo); - } else if (binding->isGroupProperty()) { - const auto *obj = objectContainer->objectAt(binding->value.objectIndex); - if (!stringAt(obj->inheritedTypeNameIndex).isEmpty()) - return nullptr; - - for (int i = 0, end = objectContainer->objectCount(); i != end; ++i) { - const auto *ext = objectContainer->objectAt(i); - if (ext->idNameIndex != binding->propertyNameIndex) - continue; - - if (ext->inheritedTypeNameIndex == 0) - return nullptr; - - QQmlBindingInstantiationContext pendingContext(i, &(*binding), QString(), nullptr); - pendingGroupPropertyBindings->append(pendingContext); - return nullptr; - } } } return nullptr; diff --git a/src/qml/qml/qqmltypecompiler.cpp b/src/qml/qml/qqmltypecompiler.cpp index eb07ebc223..cdabd6d5bc 100644 --- a/src/qml/qml/qqmltypecompiler.cpp +++ b/src/qml/qml/qqmltypecompiler.cpp @@ -806,6 +806,17 @@ bool QQmlComponentAndAliasResolver::wrapImplicitComponent(QmlI return true; } +template<> +void QQmlComponentAndAliasResolver::resolveGeneralizedGroupProperty( + const CompiledObject &component, CompiledBinding *binding) +{ + Q_UNUSED(component); + // We cannot make it fail here. It might be a custom-parsed property + const int targetObjectIndex = m_idToObjectIndex.value(binding->propertyNameIndex, -1); + if (targetObjectIndex != -1) + m_propertyCaches->set(binding->value.objectIndex, m_propertyCaches->at(targetObjectIndex)); +} + template<> typename QQmlComponentAndAliasResolver::AliasResolutionResult QQmlComponentAndAliasResolver::resolveAliasesInObject( diff --git a/src/qml/qml/qqmltypedata.cpp b/src/qml/qml/qqmltypedata.cpp index bc456d5a7e..9acd801672 100644 --- a/src/qml/qml/qqmltypedata.cpp +++ b/src/qml/qml/qqmltypedata.cpp @@ -186,6 +186,20 @@ void QQmlComponentAndAliasResolver::setObjec // in the CU vs the IR. } +template<> +void QQmlComponentAndAliasResolver::resolveGeneralizedGroupProperty( + const CompiledObject &component, CompiledBinding *binding) +{ + // We cannot make it fail here. It might be a custom-parsed property + for (int i = 0, count = component.namedObjectsInComponentCount(); i < count; ++i) { + const int candidateIndex = component.namedObjectsInComponentTable()[i]; + if (m_compiler->objectAt(candidateIndex)->idNameIndex == binding->propertyNameIndex) { + m_propertyCaches->set(binding->value.objectIndex, m_propertyCaches->at(candidateIndex)); + return; + } + } +} + template<> typename QQmlComponentAndAliasResolver::AliasResolutionResult QQmlComponentAndAliasResolver::resolveAliasesInObject( diff --git a/tests/auto/qml/qmldiskcache/tst_qmldiskcache.cpp b/tests/auto/qml/qmldiskcache/tst_qmldiskcache.cpp index c1df8ec467..810fdecafd 100644 --- a/tests/auto/qml/qmldiskcache/tst_qmldiskcache.cpp +++ b/tests/auto/qml/qmldiskcache/tst_qmldiskcache.cpp @@ -42,6 +42,7 @@ private slots: void cacheModuleScripts(); void reuseStaticMappings(); void invalidateSaveLoadCache(); + void duplicateIdsInInlineComponents(); void inlineComponentDoesNotCauseConstantInvalidation_data(); void inlineComponentDoesNotCauseConstantInvalidation(); @@ -1186,6 +1187,77 @@ void tst_qmldiskcache::invalidateSaveLoadCache() QVERIFY(unit->unitData() != oldUnit->unitData()); } +void tst_qmldiskcache::duplicateIdsInInlineComponents() +{ + // Exercise the case of loading strange generalized group properties from .qmlc. + + QQmlEngine engine; + + TestCompiler testCompiler(&engine); + QVERIFY(testCompiler.tempDir.isValid()); + + const QByteArray contents = QByteArrayLiteral(R"( + import QtQml + QtObject { + component First : QtObject { + property QtObject aa: QtObject { + id: a + } + property Binding bb: Binding { + a.objectName: "test1" + } + } + + component Second : QtObject { + property QtObject aa: QtObject { + id: a + } + property Binding bb: Binding { + a.objectName: "test2" + } + + property Component cc: QtObject { + property QtObject aa: QtObject { + id: a + } + property Binding bb: Binding { + a.objectName: "test3" + } + } + } + + property First first: First {} + property Second second: Second {} + property QtObject third: second.cc.createObject(); + + objectName: first.aa.objectName + second.aa.objectName + third.aa.objectName; + } + )"); + + { + testCompiler.clearCache(); + QVERIFY2(testCompiler.compile(contents), qPrintable(testCompiler.lastErrorString)); + QVERIFY2(testCompiler.verify(), qPrintable(testCompiler.lastErrorString)); + } + + { + CleanlyLoadingComponent component(&engine, testCompiler.testFilePath); + + QScopedPointer obj(component.create()); + QVERIFY(!obj.isNull()); + QCOMPARE(obj->objectName(), "test1test2test3"); + } + + engine.clearComponentCache(); + + { + CleanlyLoadingComponent component(&engine, testCompiler.testFilePath); + QScopedPointer obj(component.create()); + QVERIFY(!obj.isNull()); + QCOMPARE(obj->objectName(), "test1test2test3"); + } +} + void tst_qmldiskcache::inlineComponentDoesNotCauseConstantInvalidation_data() { QTest::addColumn("code"); diff --git a/tests/auto/qml/qqmlpropertycache/data/duplicateIdsAndGeneralizedGroupProperties.qml b/tests/auto/qml/qqmlpropertycache/data/duplicateIdsAndGeneralizedGroupProperties.qml new file mode 100644 index 0000000000..2dd2cd8e21 --- /dev/null +++ b/tests/auto/qml/qqmlpropertycache/data/duplicateIdsAndGeneralizedGroupProperties.qml @@ -0,0 +1,64 @@ +import QtQuick 2.15 + +Item { + component First : Item { + Item { + id: a + } + + states: [ + State { + name: "test1" + + PropertyChanges { + a.enabled: false + } + } + ] + } + + component Second : Item { + QtObject { + id: a + property bool enabled: true + } + + states: [ + State { + name: "test2" + + PropertyChanges { + a.enabled: false + } + } + ] + + property Component cc: Item { + Item { id: a } + + states: [ + State { + name: "test3" + + PropertyChanges { + a.enabled: false + } + } + ] + } + } + + First { id: first } + Second { id: second } + property Item third: second.cc.createObject(); + + Component.onCompleted: { + console.log(1, first.data[0].enabled, second.data[0].enabled, third.data[0].enabled); + first.state = "test1"; + console.log(2, first.data[0].enabled, second.data[0].enabled, third.data[0].enabled); + second.state = "test2"; + console.log(3, first.data[0].enabled, second.data[0].enabled, third.data[0].enabled); + third.state = "test3"; + console.log(4, first.data[0].enabled, second.data[0].enabled, third.data[0].enabled); + } +} diff --git a/tests/auto/qml/qqmlpropertycache/tst_qqmlpropertycache.cpp b/tests/auto/qml/qqmlpropertycache/tst_qqmlpropertycache.cpp index 2d7a7b954c..6af2a3e371 100644 --- a/tests/auto/qml/qqmlpropertycache/tst_qqmlpropertycache.cpp +++ b/tests/auto/qml/qqmlpropertycache/tst_qqmlpropertycache.cpp @@ -36,6 +36,7 @@ private slots: void restrictRegistrationVersion(); void rejectOverriddenFinal(); void overriddenSignals(); + void duplicateIdsAndGeneralizedGroupProperties(); private: QQmlEngine engine; @@ -774,4 +775,18 @@ void tst_qqmlpropertycache::overriddenSignals() // Should be an error, but we can't enforce it yet. } +void tst_qqmlpropertycache::duplicateIdsAndGeneralizedGroupProperties() +{ + QQmlEngine engine; + QQmlComponent c(&engine, testFileUrl("duplicateIdsAndGeneralizedGroupProperties.qml")); + QVERIFY2(c.isReady(), qPrintable(c.errorString())); + + QTest::ignoreMessage(QtDebugMsg, "1 true true true"); + QTest::ignoreMessage(QtDebugMsg, "2 false true true"); + QTest::ignoreMessage(QtDebugMsg, "3 false false true"); + QTest::ignoreMessage(QtDebugMsg, "4 false false false"); + + QScopedPointer o(c.create()); +} + QTEST_MAIN(tst_qqmlpropertycache)