QtQml: Resolve generalized group properties during alias resolution

This way we can know the scope in which to search for the IDs.

Pick-to: 6.7 6.5
Fixes: QTBUG-123865
Change-Id: I1dff9bdc69e3edaa80d85c757e3bb2b24e174cd0
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
This commit is contained in:
Ulf Hermann 2024-04-03 17:55:45 +02:00
parent 4b5f091c49
commit b91a26ae3b
8 changed files with 215 additions and 25 deletions

View File

@ -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<typename ObjectContainer>
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<quint32> m_componentRoots;
QVector<int> m_objectsWithAliases;
QVector<CompiledBinding *> m_generalizedGroupProperties;
typename ObjectContainer::IdToObjectMap m_idToObjectIndex;
};
@ -330,6 +339,7 @@ QQmlError QQmlComponentAndAliasResolver<ObjectContainer>::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<ObjectContainer>::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<typename ObjectContainer>
@ -375,9 +392,19 @@ QQmlError QQmlComponentAndAliasResolver<ObjectContainer>::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<ObjectContainer>::resolveAliases(int com
return QQmlError();
}
template<typename ObjectContainer>
void QQmlComponentAndAliasResolver<ObjectContainer>::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

View File

@ -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

View File

@ -375,23 +375,6 @@ inline QQmlPropertyCache::ConstPtr QQmlPropertyCacheCreator<ObjectContainer>::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;

View File

@ -806,6 +806,17 @@ bool QQmlComponentAndAliasResolver<QQmlTypeCompiler>::wrapImplicitComponent(QmlI
return true;
}
template<>
void QQmlComponentAndAliasResolver<QQmlTypeCompiler>::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<QQmlTypeCompiler>::AliasResolutionResult
QQmlComponentAndAliasResolver<QQmlTypeCompiler>::resolveAliasesInObject(

View File

@ -186,6 +186,20 @@ void QQmlComponentAndAliasResolver<QV4::CompiledData::CompilationUnit>::setObjec
// in the CU vs the IR.
}
template<>
void QQmlComponentAndAliasResolver<QV4::CompiledData::CompilationUnit>::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<QV4::CompiledData::CompilationUnit>::AliasResolutionResult
QQmlComponentAndAliasResolver<QV4::CompiledData::CompilationUnit>::resolveAliasesInObject(

View File

@ -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<QObject> obj(component.create());
QVERIFY(!obj.isNull());
QCOMPARE(obj->objectName(), "test1test2test3");
}
engine.clearComponentCache();
{
CleanlyLoadingComponent component(&engine, testCompiler.testFilePath);
QScopedPointer<QObject> obj(component.create());
QVERIFY(!obj.isNull());
QCOMPARE(obj->objectName(), "test1test2test3");
}
}
void tst_qmldiskcache::inlineComponentDoesNotCauseConstantInvalidation_data()
{
QTest::addColumn<QByteArray>("code");

View File

@ -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);
}
}

View File

@ -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<QObject> o(c.create());
}
QTEST_MAIN(tst_qqmlpropertycache)