From dc4bd4fb3f98967f65dc510e5d42831e1cd3b4f1 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Tue, 7 Nov 2023 15:16:32 +0100 Subject: [PATCH] QtQml: Fix some problems with deep aliases We cannot get the property cache for an inline component the usual way during type compilation. We have to ask the compilation unit for it. This will usually not work in 6.6 or earlier since the compilation unit does not know the IC's metatypes. However, it shouldn't crash. Guard against still not being able to retrieve the property cache for any reason. Abort the compilation rather than crashing. Also, unify the setting of property attributes. Those should really work the same way everywhere. Finally, disallow writing aliases to value type properties where the property holding the value type itself is not writable. Pick-to: 6.2 Task-number: QTBUG-115579 Change-Id: I029eb56a9a390085d0c696a787a64c48acf0d620 Reviewed-by: Fabian Kosmale (cherry picked from commit 3ea55bf398412d373daab9c92b1498f45de70e96) (cherry picked from commit 7326d41bef7c0ba362e66f9b3668b81d3e74716a) --- .../qv4executablecompilationunit.cpp | 9 ++ .../qv4executablecompilationunit_p.h | 1 + src/qml/qml/qqmlpropertycachecreator_p.h | 91 ++++++++++++------- src/qml/qml/qqmltypecompiler_p.h | 9 ++ .../qml/qqmllanguage/data/DeepAliasOnIC.qml | 25 +++++ .../qqmllanguage/data/deepAliasOnICUser.qml | 8 ++ .../qqmllanguage/data/deepAliasOnReadonly.qml | 5 + .../qml/qqmllanguage/tst_qqmllanguage.cpp | 27 ++++++ 8 files changed, 140 insertions(+), 35 deletions(-) create mode 100644 tests/auto/qml/qqmllanguage/data/DeepAliasOnIC.qml create mode 100644 tests/auto/qml/qqmllanguage/data/deepAliasOnICUser.qml create mode 100644 tests/auto/qml/qqmllanguage/data/deepAliasOnReadonly.qml diff --git a/src/qml/jsruntime/qv4executablecompilationunit.cpp b/src/qml/jsruntime/qv4executablecompilationunit.cpp index 7b7772d3f0..499620c9bf 100644 --- a/src/qml/jsruntime/qv4executablecompilationunit.cpp +++ b/src/qml/jsruntime/qv4executablecompilationunit.cpp @@ -480,6 +480,15 @@ int ExecutableCompilationUnit::totalObjectCount() const { return inlineComponentData[icRoot].totalObjectCount; } +ResolvedTypeReference *ExecutableCompilationUnit::resolvedType(QMetaType type) const +{ + for (ResolvedTypeReference *ref : std::as_const(resolvedTypes)) { + if (ref->type().typeId() == type) + return ref; + } + return nullptr; +} + int ExecutableCompilationUnit::totalParserStatusCount() const { if (icRoot == -1) return m_totalParserStatusCount; diff --git a/src/qml/jsruntime/qv4executablecompilationunit_p.h b/src/qml/jsruntime/qv4executablecompilationunit_p.h index a1cf60249b..9ba63b5adb 100644 --- a/src/qml/jsruntime/qv4executablecompilationunit_p.h +++ b/src/qml/jsruntime/qv4executablecompilationunit_p.h @@ -141,6 +141,7 @@ public: QVector> dependentScripts; ResolvedTypeReferenceMap resolvedTypes; ResolvedTypeReference *resolvedType(int id) const { return resolvedTypes.value(id); } + ResolvedTypeReference *resolvedType(QMetaType type) const; bool verifyChecksum(const CompiledData::DependentTypesHasher &dependencyHasher) const; diff --git a/src/qml/qml/qqmlpropertycachecreator_p.h b/src/qml/qml/qqmlpropertycachecreator_p.h index da2063a7a8..71f1cd7f0b 100644 --- a/src/qml/qml/qqmlpropertycachecreator_p.h +++ b/src/qml/qml/qqmlpropertycachecreator_p.h @@ -849,53 +849,74 @@ inline QQmlError QQmlPropertyCacheAliasCreator::propertyDataFor const QQmlPropertyData *targetProperty = targetCache->property(coreIndex); Q_ASSERT(targetProperty); - // for deep aliases, valueTypeIndex is always set - if (!QQmlMetaType::isValueType(targetProperty->propType()) && valueTypeIndex != -1) { - // deep alias property - *type = targetProperty->propType(); - QQmlPropertyCache::ConstPtr typeCache = QQmlMetaType::propertyCacheForType(*type); - Q_ASSERT(typeCache); - const QQmlPropertyData *typeProperty = typeCache->property(valueTypeIndex); + const QMetaType targetPropType = targetProperty->propType(); - if (typeProperty == nullptr) { - return qQmlCompileError(alias.referenceLocation, - QQmlPropertyCacheCreatorBase::tr("Invalid alias target")); + const auto resolveType = [](QMetaType targetPropType) { + if (targetPropType.flags() & QMetaType::IsEnumeration) + return QMetaType::fromType(); + else + return targetPropType; + }; + + const auto populateWithPropertyData = [&](const QQmlPropertyData *property) { + *type = resolveType(property->propType()); + writable = property->isWritable(); + resettable = property->isResettable(); + bindable = property->isBindable(); + + // Copy type flags + propertyFlags->copyPropertyTypeFlags(property->flags()); + if (property->isVarProperty()) + propertyFlags->type = QQmlPropertyData::Flags::QVariantType; + }; + + // for deep aliases, valueTypeIndex is always set + if (!QQmlMetaType::isValueType(targetPropType) && valueTypeIndex != -1) { + // deep alias property + + QQmlPropertyCache::ConstPtr typeCache + = QQmlMetaType::propertyCacheForType(targetPropType); + + if (!typeCache) { + // See if it's a half-resolved composite type + if (const QV4::ResolvedTypeReference *typeRef + = objectContainer->resolvedType(targetPropType)) { + typeCache = typeRef->typePropertyCache(); + } } - *type = typeProperty->propType(); - writable = typeProperty->isWritable(); - resettable = typeProperty->isResettable(); - bindable = typeProperty->isBindable(); + const QQmlPropertyData *typeProperty = typeCache + ? typeCache->property(valueTypeIndex) + : nullptr; + if (typeProperty == nullptr) { + return qQmlCompileError( + alias.referenceLocation, + QQmlPropertyCacheCreatorBase::tr("Invalid alias target")); + } + populateWithPropertyData(typeProperty); } else { // value type or primitive type or enum - *type = targetProperty->propType(); - - writable = targetProperty->isWritable(); - resettable = targetProperty->isResettable(); - bindable = targetProperty->isBindable(); + populateWithPropertyData(targetProperty); if (valueTypeIndex != -1) { - const QMetaObject *valueTypeMetaObject = QQmlMetaType::metaObjectForValueType(*type); - if (valueTypeMetaObject->property(valueTypeIndex).isEnumType()) - *type = QMetaType::fromType(); - else - *type = valueTypeMetaObject->property(valueTypeIndex).metaType(); - } else { - if (targetProperty->isEnum()) { - *type = QMetaType::fromType(); - } else { - // Copy type flags - propertyFlags->copyPropertyTypeFlags(targetProperty->flags()); + const QMetaObject *valueTypeMetaObject + = QQmlMetaType::metaObjectForValueType(*type); + const QMetaProperty valueTypeMetaProperty + = valueTypeMetaObject->property(valueTypeIndex); + *type = resolveType(valueTypeMetaProperty.metaType()); - if (targetProperty->isVarProperty()) - propertyFlags->type = QQmlPropertyData::Flags::QVariantType; - } + // We can only write or reset the value type property if we can write + // the value type itself. + resettable = writable && valueTypeMetaProperty.isResettable(); + writable = writable && valueTypeMetaProperty.isWritable(); + + bindable = valueTypeMetaProperty.isBindable(); } } } - propertyFlags->setIsWritable(!(alias.hasFlag(QV4::CompiledData::Alias::IsReadOnly)) - && writable); + propertyFlags->setIsWritable( + writable && !alias.hasFlag(QV4::CompiledData::Alias::IsReadOnly)); propertyFlags->setIsResettable(resettable); propertyFlags->setIsBindable(bindable); return QQmlError(); diff --git a/src/qml/qml/qqmltypecompiler_p.h b/src/qml/qml/qqmltypecompiler_p.h index 0b1bfb9e9d..e20aea92e9 100644 --- a/src/qml/qml/qqmltypecompiler_p.h +++ b/src/qml/qml/qqmltypecompiler_p.h @@ -108,6 +108,15 @@ public: return resolvedTypes->value(id); } + QV4::ResolvedTypeReference *resolvedType(QMetaType type) const + { + for (QV4::ResolvedTypeReference *ref : std::as_const(*resolvedTypes)) { + if (ref->type().typeId() == type) + return ref; + } + return nullptr; + } + CompositeMetaTypeIds typeIdsForComponent(int objectId = 0) const; private: diff --git a/tests/auto/qml/qqmllanguage/data/DeepAliasOnIC.qml b/tests/auto/qml/qqmllanguage/data/DeepAliasOnIC.qml new file mode 100644 index 0000000000..36a8cc6078 --- /dev/null +++ b/tests/auto/qml/qqmllanguage/data/DeepAliasOnIC.qml @@ -0,0 +1,25 @@ +import QtQml + +QtObject { + id: root + + component ObjectWithColor: QtObject { + property string color + property var varvar + } + + property ObjectWithColor border: ObjectWithColor { + id: border + color: root.trueBorderColor + varvar: root.trueBorderVarvar + } + + readonly property rect readonlyRect: ({x: 12, y: 13, width: 14, height: 15}) + + property alias borderColor: border.color + property alias borderVarvar: border.varvar + property alias readonlyRectX: root.readonlyRect.x + + property string trueBorderColor: "green" + property var trueBorderVarvar: 1234 +} diff --git a/tests/auto/qml/qqmllanguage/data/deepAliasOnICUser.qml b/tests/auto/qml/qqmllanguage/data/deepAliasOnICUser.qml new file mode 100644 index 0000000000..ef6001cd89 --- /dev/null +++ b/tests/auto/qml/qqmllanguage/data/deepAliasOnICUser.qml @@ -0,0 +1,8 @@ +import QtQml + +DeepAliasOnIC { + borderColor: "black" + borderVarvar: "mauve" +} + + diff --git a/tests/auto/qml/qqmllanguage/data/deepAliasOnReadonly.qml b/tests/auto/qml/qqmllanguage/data/deepAliasOnReadonly.qml new file mode 100644 index 0000000000..f5ae62406b --- /dev/null +++ b/tests/auto/qml/qqmllanguage/data/deepAliasOnReadonly.qml @@ -0,0 +1,5 @@ +import QtQml + +DeepAliasOnIC { + readonlyRectX: 55 +} diff --git a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp index 0665b98f4e..bc2baa6aef 100644 --- a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp +++ b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp @@ -422,6 +422,7 @@ private slots: void typeAnnotationCycle(); void objectInQmlListAndGc(); + void deepAliasOnICOrReadonly(); private: QQmlEngine engine; @@ -8116,6 +8117,32 @@ void tst_qqmllanguage::objectInQmlListAndGc() QCOMPARE(child->objectName(), QLatin1String("child")); } +void tst_qqmllanguage::deepAliasOnICOrReadonly() +{ + QQmlEngine engine; + QQmlComponent c(&engine, testFileUrl("deepAliasOnICUser.qml")); + QVERIFY2(c.isReady(), qPrintable(c.errorString())); + QScopedPointer o(c.create()); + QVERIFY(!o.isNull()); + + // We are mostly testing that it doesn't crash here. The actual bug is fixed separately. + + QEXPECT_FAIL("", "QTBUG-115579 is not fixed yet", Continue); + QCOMPARE(o->property("borderColor").toString(), QLatin1String("black")); + + const QVariant var = o->property("borderVarvar"); + QEXPECT_FAIL("", "QTBUG-115579 is not fixed yet", Continue); + QCOMPARE(var.metaType(), QMetaType::fromType()); + QEXPECT_FAIL("", "QTBUG-115579 is not fixed yet", Continue); + QCOMPARE(var.toString(), QLatin1String("mauve")); + + QQmlComponent c2(&engine, testFileUrl("deepAliasOnReadonly.qml")); + QVERIFY(c2.isError()); + QVERIFY(c2.errorString().contains( + QLatin1String( + "Invalid property assignment: \"readonlyRectX\" is a read-only property"))); +} + QTEST_MAIN(tst_qqmllanguage) #include "tst_qqmllanguage.moc"