diff --git a/src/qml/qml/qqml.cpp b/src/qml/qml/qqml.cpp index 46e83d532e..23cea45f9d 100644 --- a/src/qml/qml/qqml.cpp +++ b/src/qml/qml/qqml.cpp @@ -1195,10 +1195,17 @@ static ObjectPropertyResult changeObjectProperty(QV4::Lookup *l, QObject *object } template -static ObjectPropertyResult resetObjectProperty(QV4::Lookup *l, QObject *object) +static ObjectPropertyResult resetObjectProperty( + QV4::Lookup *l, QObject *object, QV4::ExecutionEngine *v4) { return changeObjectProperty(l, object, [&](const QQmlPropertyData *property) { - property->resetProperty(object, {}); + if (property->isResettable()) { + property->resetProperty(object, {}); + } else { + v4->throwError( + QLatin1String("Cannot assign [undefined] to ") + + QLatin1String(property->propType().name())); + } }); } @@ -1232,11 +1239,18 @@ static ObjectPropertyResult storeFallbackProperty(QV4::Lookup *l, QObject *objec }); } -static ObjectPropertyResult resetFallbackProperty(QV4::Lookup *l, QObject *object) +static ObjectPropertyResult resetFallbackProperty( + QV4::Lookup *l, QObject *object, const QMetaProperty *property, QV4::ExecutionEngine *v4) { return changeFallbackProperty(l, object, [&](const QMetaObject *metaObject, int coreIndex) { - void *args[] = { nullptr }; - metaObject->metacall(object, QMetaObject::ResetProperty, coreIndex, args); + if (property->isResettable()) { + void *args[] = { nullptr }; + metaObject->metacall(object, QMetaObject::ResetProperty, coreIndex, args); + } else { + v4->throwError( + QLatin1String("Cannot assign [undefined] to ") + + QLatin1String(property->typeName())); + } }); } @@ -1313,7 +1327,7 @@ static ObjectPropertyResult storeObjectAsVariant( return storeObjectProperty(l, object, variant); if (!variant->isValid()) - return resetObjectProperty(l, object); + return resetObjectProperty(l, object, v4); if (isTypeCompatible(variant->metaType(), propType)) return storeObjectProperty(l, object, variant->data()); @@ -1332,12 +1346,13 @@ static ObjectPropertyResult storeFallbackAsVariant( = reinterpret_cast(l->qobjectFallbackLookup.metaObject - 1); Q_ASSERT(metaObject); - const QMetaType propType = metaObject->property(l->qobjectFallbackLookup.coreIndex).metaType(); + const QMetaProperty property = metaObject->property(l->qobjectFallbackLookup.coreIndex); + const QMetaType propType = property.metaType(); if (propType == QMetaType::fromType()) return storeFallbackProperty(l, object, variant); - if (!propType.isValid()) - return resetFallbackProperty(l, object); + if (!variant->isValid()) + return resetFallbackProperty(l, object, &property, v4); if (isTypeCompatible(variant->metaType(), propType)) return storeFallbackProperty(l, object, variant->data()); @@ -1590,7 +1605,7 @@ void AOTCompiledContext::storeNameSloppy(uint nameIndex, void *value, QMetaType if (isTypeCompatible(type, propType)) { storeResult = storeObjectProperty(&l, qmlScopeObject, value); } else if (isUndefined(value, type)) { - storeResult = resetObjectProperty(&l, qmlScopeObject); + storeResult = resetObjectProperty(&l, qmlScopeObject, engine->handle()); } else { QVariant var(propType); QV4::ExecutionEngine *v4 = engine->handle(); @@ -1605,12 +1620,12 @@ void AOTCompiledContext::storeNameSloppy(uint nameIndex, void *value, QMetaType case ObjectLookupResult::Fallback: { const QMetaObject *metaObject = reinterpret_cast(l.qobjectFallbackLookup.metaObject - 1); - const QMetaType propType - = metaObject->property(l.qobjectFallbackLookup.coreIndex).metaType(); + const QMetaProperty property = metaObject->property(l.qobjectFallbackLookup.coreIndex); + const QMetaType propType = property.metaType(); if (isTypeCompatible(type, propType)) { storeResult = storeFallbackProperty(&l, qmlScopeObject, value); } else if (isUndefined(value, type)) { - storeResult = resetFallbackProperty(&l, qmlScopeObject); + storeResult = resetFallbackProperty(&l, qmlScopeObject, &property, engine->handle()); } else { QVariant var(propType); QV4::ExecutionEngine *v4 = engine->handle(); diff --git a/src/qmlcompiler/qqmljsshadowcheck.cpp b/src/qmlcompiler/qqmljsshadowcheck.cpp index 3b4bda7adc..94b908effb 100644 --- a/src/qmlcompiler/qqmljsshadowcheck.cpp +++ b/src/qmlcompiler/qqmljsshadowcheck.cpp @@ -87,11 +87,11 @@ void QQmlJSShadowCheck::handleStore(int base, const QString &memberName) if (checkShadowing(baseType, memberName, base) == Shadowable) return; - // If the property isn't shadowable but resettable, we have to turn the read register into + // If the property isn't shadowable, we have to turn the read register into // var if the accumulator can hold undefined. This has to be done in a second pass // because the accumulator may still turn into var due to its own shadowing. const QQmlJSRegisterContent member = m_typeResolver->memberType(baseType, memberName); - if (member.isProperty() && !member.property().reset().isEmpty()) + if (member.isProperty()) m_resettableStores.append({m_state.accumulatorIn(), instructionOffset}); } diff --git a/src/qmlcompiler/qqmljstypepropagator.cpp b/src/qmlcompiler/qqmljstypepropagator.cpp index 7df0d3a498..beb846bf81 100644 --- a/src/qmlcompiler/qqmljstypepropagator.cpp +++ b/src/qmlcompiler/qqmljstypepropagator.cpp @@ -653,9 +653,7 @@ void QQmlJSTypePropagator::generate_StoreNameCommon(int nameIndex) if (m_typeResolver->canHoldUndefined(in) && !m_typeResolver->canHoldUndefined(type)) { - if (type.property().reset().isEmpty()) - setError(u"Cannot assign potential undefined to %1"_s.arg(type.descriptiveName())); - else if (m_typeResolver->registerIsStoredIn(in, m_typeResolver->voidType())) + if (m_typeResolver->registerIsStoredIn(in, m_typeResolver->voidType())) addReadAccumulator(m_typeResolver->globalType(m_typeResolver->varType())); else addReadAccumulator(in); @@ -990,19 +988,19 @@ void QQmlJSTypePropagator::generate_StoreProperty(int nameIndex, int base) getCurrentBindingSourceLocation())); } - // If the property is resettable we must not coerce the input to the property type + // If the input can hold undefined we must not coerce it to the property type // as that might eliminate an undefined value. For example, undefined -> string // becomes "undefined". - // Therefore we explicitly require the value to be given as QVariant. This triggers - // the QVariant fallback path that also used for shadowable properties. QVariant can - // hold undefined and the lookup functions will handle that appropriately. + // We need the undefined value for either resetting the property if that is supported + // or generating an exception otherwise. Therefore we explicitly require the value to + // be given as QVariant. This triggers the QVariant fallback path that's also used for + // shadowable properties. QVariant can hold undefined and the lookup functions will + // handle that appropriately. const QQmlJSScope::ConstPtr varType = m_typeResolver->varType(); - const QQmlJSRegisterContent readType - = (property.property().reset().isEmpty() - || !m_typeResolver->canHoldUndefined(m_state.accumulatorIn())) - ? property - : property.storedIn(varType).castTo(varType); + const QQmlJSRegisterContent readType = m_typeResolver->canHoldUndefined(m_state.accumulatorIn()) + ? property.storedIn(varType).castTo(varType) + : property; addReadAccumulator(readType); addReadRegister(base, callBase); m_state.setHasSideEffects(true); diff --git a/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt b/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt index 67d8962983..8b56bc77ad 100644 --- a/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt +++ b/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt @@ -135,6 +135,7 @@ set(qml_files extendedTypes.qml failures.qml fallbacklookups.qml + fallbackresettable.qml fileDialog.qml flagEnum.qml fromBoolValue.qml diff --git a/tests/auto/qml/qmlcppcodegen/data/dynamicmeta.h b/tests/auto/qml/qmlcppcodegen/data/dynamicmeta.h index 64c2850bda..100e9d825c 100644 --- a/tests/auto/qml/qmlcppcodegen/data/dynamicmeta.h +++ b/tests/auto/qml/qmlcppcodegen/data/dynamicmeta.h @@ -5,34 +5,44 @@ #define DYNAMICMETA_H #include +#include #include -struct FreeDeleter { - void operator()(QMetaObject *meta) { free(meta); } -}; - template class MetaObjectData : public QDynamicMetaObjectData { Q_DISABLE_COPY_MOVE(MetaObjectData) public: - MetaObjectData() = default; - ~MetaObjectData() = default; + MetaObjectData() + { + QMetaObjectBuilder builder; + builder.setSuperClass(&T::staticMetaObject); + builder.setFlags(builder.flags() | DynamicMetaObject); + metaObject = builder.toMetaObject(); + }; + + ~MetaObjectData() { + free(metaObject); + }; QMetaObject *toDynamicMetaObject(QObject *) override { - return const_cast(&T::staticMetaObject); + return metaObject; } int metaCall(QObject *o, QMetaObject::Call call, int idx, void **argv) override { return o->qt_metacall(call, idx, argv); } + + QMetaObject *metaObject = nullptr; }; class DynamicMeta : public QObject { Q_OBJECT Q_PROPERTY(int foo READ foo WRITE setFoo NOTIFY fooChanged FINAL) + Q_PROPERTY(qreal value READ value WRITE setValue RESET resetValue NOTIFY valueChanged FINAL) + Q_PROPERTY(qreal shadowable READ shadowable CONSTANT) QML_ELEMENT public: @@ -54,11 +64,26 @@ public: Q_INVOKABLE int bar(int baz) { return baz + 12; } + qreal value() const { return m_value; } + qreal shadowable() const { return 25; } + +public slots: + void resetValue() { setValue(0); } + void setValue(qreal value) + { + if (m_value == value) + return; + m_value = value; + emit valueChanged(); + } + Q_SIGNALS: void fooChanged(); + void valueChanged(); private: int m_foo = 0; + qreal m_value = 0; }; class DynamicMetaSingleton : public DynamicMeta diff --git a/tests/auto/qml/qmlcppcodegen/data/failures.qml b/tests/auto/qml/qmlcppcodegen/data/failures.qml index ca46c806b3..3b0e4908ab 100644 --- a/tests/auto/qml/qmlcppcodegen/data/failures.qml +++ b/tests/auto/qml/qmlcppcodegen/data/failures.qml @@ -35,12 +35,6 @@ QtObject { onPartyStarted: (foozle) => { objectName = foozle } } - signal foo() - signal bar() - - // Cannot assign potential undefined - onFoo: objectName = self.bar() - property int enumFromGadget1: GadgetWithEnum.CONNECTED + 1 property int enumFromGadget2: TT2.GadgetWithEnum.CONNECTED + 1 diff --git a/tests/auto/qml/qmlcppcodegen/data/fallbackresettable.qml b/tests/auto/qml/qmlcppcodegen/data/fallbackresettable.qml new file mode 100644 index 0000000000..44b55e245a --- /dev/null +++ b/tests/auto/qml/qmlcppcodegen/data/fallbackresettable.qml @@ -0,0 +1,23 @@ +pragma Strict +import QtQml +import TestTypes + +DynamicMeta { + id: self + value: 999 + + property double notResettable: 10 + property double notResettable2: { return undefined } + + property DynamicMeta shadowing: DynamicMeta { + property var shadowable: undefined + } + + function doReset() { self.value = undefined } + function doReset2() { self.value = shadowing.shadowable } + function doNotReset() { self.notResettable = undefined } + + signal aaa() + signal bbb() + onAaa: objectName = self.bbb() +} diff --git a/tests/auto/qml/qmlcppcodegen/data/resettable.qml b/tests/auto/qml/qmlcppcodegen/data/resettable.qml index 59fa31645e..561655032d 100644 --- a/tests/auto/qml/qmlcppcodegen/data/resettable.qml +++ b/tests/auto/qml/qmlcppcodegen/data/resettable.qml @@ -6,10 +6,18 @@ Resettable { id: self value: 999 + property double notResettable: 10 + property double notResettable2: { return undefined } + property Resettable shadowing: Resettable { property var shadowable: undefined } function doReset() { self.value = undefined } function doReset2() { self.value = shadowing.shadowable } + function doNotReset() { self.notResettable = undefined } + + signal aaa() + signal bbb() + onAaa: objectName = self.bbb() } diff --git a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp index 548ad20c2b..2fc5129230 100644 --- a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp +++ b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp @@ -187,7 +187,10 @@ private slots: void registerElimination(); void registerPropagation(); void renameAdjust(); + void resettableProperty(); + void resettableProperty_data(); + void returnAfterReject(); void revisions(); void scopeIdLookup(); @@ -3841,23 +3844,46 @@ void tst_QmlCppCodegen::renameAdjust() void tst_QmlCppCodegen::resettableProperty() { + QFETCH(QString, url); + QQmlEngine engine; - QQmlComponent c(&engine, QUrl(u"qrc:/qt/qml/TestTypes/resettable.qml"_s)); + QQmlComponent c(&engine, QUrl(url)); QVERIFY2(c.isReady(), qPrintable(c.errorString())); + + QTest::ignoreMessage( + QtWarningMsg, qPrintable(url + u":10:5: Unable to assign [undefined] to double"_s)); + QScopedPointer o(c.create()); QVERIFY(o); - ResettableProperty *resettable = qobject_cast(o.data()); - QVERIFY(resettable); + QCOMPARE(o->property("value").toDouble(), 999); + QMetaObject::invokeMethod(o.data(), "doReset"); + QCOMPARE(o->property("value").toDouble(), 0); - QCOMPARE(resettable->value(), 999); - QMetaObject::invokeMethod(resettable, "doReset"); - QCOMPARE(resettable->value(), 0); + o->setProperty("value", double(82)); + QCOMPARE(o->property("value").toDouble(), 82); + QMetaObject::invokeMethod(o.data(), "doReset2"); + QCOMPARE(o->property("value").toDouble(), 0); - resettable->setValue(82); - QCOMPARE(resettable->value(), 82); - QMetaObject::invokeMethod(resettable, "doReset2"); - QCOMPARE(resettable->value(), 0); + QTest::ignoreMessage( + QtWarningMsg, qPrintable(url + u":18: Error: Cannot assign [undefined] to double"_s)); + QCOMPARE(o->property("notResettable").toDouble(), 10); + QMetaObject::invokeMethod(o.data(), "doNotReset"); + QCOMPARE(o->property("notResettable").toDouble(), 10); + QCOMPARE(o->property("notResettable2").toDouble(), 0); // not NaN + + o->setObjectName(u"namename"_s); + QTest::ignoreMessage( + QtWarningMsg, qPrintable(url + u":22: Error: Cannot assign [undefined] to QString"_s)); + QMetaObject::invokeMethod(o.data(), "aaa"); + QCOMPARE(o->objectName(), u"namename"_s); +} + +void tst_QmlCppCodegen::resettableProperty_data() +{ + QTest::addColumn("url"); + QTest::addRow("object lookups") << u"qrc:/qt/qml/TestTypes/resettable.qml"_s; + QTest::addRow("fallback lookups") << u"qrc:/qt/qml/TestTypes/fallbackresettable.qml"_s; } void tst_QmlCppCodegen::returnAfterReject()