diff --git a/src/qmlcompiler/qqmljsshadowcheck.cpp b/src/qmlcompiler/qqmljsshadowcheck.cpp index 167396f614..3b4bda7adc 100644 --- a/src/qmlcompiler/qqmljsshadowcheck.cpp +++ b/src/qmlcompiler/qqmljsshadowcheck.cpp @@ -43,6 +43,9 @@ void QQmlJSShadowCheck::run( m_error = error; m_state = initialState(function); decode(m_function->code.constData(), static_cast(m_function->code.size())); + + for (const auto &store : m_resettableStores) + checkResettable(store.accumulatorIn, store.instructionOffset); } void QQmlJSShadowCheck::generate_LoadProperty(int nameIndex) @@ -70,15 +73,37 @@ void QQmlJSShadowCheck::generate_GetLookup(int index) } } +void QQmlJSShadowCheck::handleStore(int base, const QString &memberName) +{ + const int instructionOffset = currentInstructionOffset(); + const QQmlJSRegisterContent &readAccumulator + = (*m_annotations)[instructionOffset].readRegisters[Accumulator].content; + + // If the accumulator is already read as var, we don't have to do anything. + if (m_typeResolver->registerContains(readAccumulator, m_typeResolver->varType())) + return; + + const auto baseType = m_state.registers[base].content; + if (checkShadowing(baseType, memberName, base) == Shadowable) + return; + + // If the property isn't shadowable but resettable, 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()) + m_resettableStores.append({m_state.accumulatorIn(), instructionOffset}); +} + + void QQmlJSShadowCheck::generate_StoreProperty(int nameIndex, int base) { - checkShadowing( - m_state.registers[base].content, m_jsUnitGenerator->stringForIndex(nameIndex), base); + handleStore(base, m_jsUnitGenerator->stringForIndex(nameIndex)); } void QQmlJSShadowCheck::generate_SetLookup(int index, int base) { - checkShadowing(m_state.registers[base].content, m_jsUnitGenerator->lookupName(index), base); + handleStore(base, m_jsUnitGenerator->lookupName(index)); } void QQmlJSShadowCheck::generate_CallProperty(int nameIndex, int base, int argc, int argv) @@ -107,11 +132,11 @@ void QQmlJSShadowCheck::endInstruction(QV4::Moth::Instr::Type) { } -void QQmlJSShadowCheck::checkShadowing( +QQmlJSShadowCheck::Shadowability QQmlJSShadowCheck::checkShadowing( const QQmlJSRegisterContent &baseType, const QString &memberName, int baseRegister) { if (baseType.storedType()->accessSemantics() != QQmlJSScope::AccessSemantics::Reference) - return; + return NotShadowable; switch (baseType.variant()) { case QQmlJSRegisterContent::ExtensionObjectProperty: @@ -128,14 +153,14 @@ void QQmlJSShadowCheck::checkShadowing( // those are not shadowable. if (!member.isValid()) { Q_ASSERT(m_typeResolver->isPrefix(memberName)); - return; + return NotShadowable; } if (member.isProperty()) { if (member.property().isFinal()) - return; // final properties can't be shadowed + return NotShadowable; // final properties can't be shadowed } else if (!member.isMethod()) { - return; // Only properties and methods can be shadowed + return NotShadowable; // Only properties and methods can be shadowed } m_logger->log( @@ -160,15 +185,33 @@ void QQmlJSShadowCheck::checkShadowing( if (it.key() != baseRegister) it->second.content = m_typeResolver->convert(it->second.content, varContent); } - - return; + return Shadowable; } default: // In particular ObjectById is fine as that cannot change into something else // Singleton should also be fine, unless the factory function creates an object // with different property types than the declared class. - return; + return NotShadowable; } } +void QQmlJSShadowCheck::checkResettable( + const QQmlJSRegisterContent &accumulatorIn, int instructionOffset) +{ + const QQmlJSScope::ConstPtr varType = m_typeResolver->varType(); + + // The stored type is not necessarily updated by the shadow check, but it + // will be in the basic blocks pass. For the purpose of adjusting newly + // shadowable types we can ignore it. We only want to know if any of the + // contents can hold undefined. + if (!m_typeResolver->canHoldUndefined(accumulatorIn.storedIn(varType))) + return; + + const QQmlJSRegisterContent varContent = m_typeResolver->globalType(varType); + + QQmlJSRegisterContent &readAccumulator + = (*m_annotations)[instructionOffset].readRegisters[Accumulator].content; + readAccumulator = m_typeResolver->convert(readAccumulator, varContent); +} + QT_END_NAMESPACE diff --git a/src/qmlcompiler/qqmljsshadowcheck_p.h b/src/qmlcompiler/qqmljsshadowcheck_p.h index f583c5f072..a5fb350b9d 100644 --- a/src/qmlcompiler/qqmljsshadowcheck_p.h +++ b/src/qmlcompiler/qqmljsshadowcheck_p.h @@ -32,6 +32,13 @@ public: QQmlJS::DiagnosticMessage *error); private: + struct ResettableStore { + QQmlJSRegisterContent accumulatorIn; + int instructionOffset = -1; + }; + + void handleStore(int base, const QString &memberName); + void generate_LoadProperty(int nameIndex) override; void generate_GetLookup(int index) override; void generate_StoreProperty(int nameIndex, int base) override; @@ -42,9 +49,13 @@ private: QV4::Moth::ByteCodeHandler::Verdict startInstruction(QV4::Moth::Instr::Type) override; void endInstruction(QV4::Moth::Instr::Type) override; - void checkShadowing( + enum Shadowability { NotShadowable, Shadowable }; + Shadowability checkShadowing( const QQmlJSRegisterContent &baseType, const QString &propertyName, int baseRegister); + void checkResettable(const QQmlJSRegisterContent &accumulatorIn, int instructionOffset); + + QList m_resettableStores; InstructionAnnotations *m_annotations = nullptr; State m_state; }; diff --git a/src/qmlcompiler/qqmljstypepropagator.cpp b/src/qmlcompiler/qqmljstypepropagator.cpp index f2c12a5420..7df0d3a498 100644 --- a/src/qmlcompiler/qqmljstypepropagator.cpp +++ b/src/qmlcompiler/qqmljstypepropagator.cpp @@ -990,7 +990,20 @@ void QQmlJSTypePropagator::generate_StoreProperty(int nameIndex, int base) getCurrentBindingSourceLocation())); } - addReadAccumulator(property); + // If the property is resettable we must not coerce the input 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. + + 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); + 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 aa63fafe7f..655f08271f 100644 --- a/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt +++ b/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt @@ -17,6 +17,7 @@ set(cpp_sources multiforeign.h objectwithmethod.h person.cpp person.h + resettable.h sequenceToIterable.h sequencetypeexample.cpp sequencetypeexample.h state.h @@ -221,6 +222,7 @@ set(qml_files registerPropagation.qml registerelimination.qml renameAdjust.qml + resettable.qml returnAfterReject.qml revisions.qml scopeIdLookup.qml diff --git a/tests/auto/qml/qmlcppcodegen/data/resettable.h b/tests/auto/qml/qmlcppcodegen/data/resettable.h new file mode 100644 index 0000000000..129ef36ef4 --- /dev/null +++ b/tests/auto/qml/qmlcppcodegen/data/resettable.h @@ -0,0 +1,39 @@ +// Copyright (C) 2024 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#ifndef RESETTABLE_H +#define RESETTABLE_H + +#include +#include + +class ResettableProperty : public QObject +{ + Q_OBJECT + QML_NAMED_ELEMENT(Resettable) + Q_PROPERTY(qreal value READ value WRITE setValue RESET resetValue NOTIFY valueChanged FINAL) + Q_PROPERTY(qreal shadowable READ shadowable CONSTANT) + +public: + explicit ResettableProperty(QObject *parent = nullptr) : QObject(parent) {} + 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(); + } + +signals: + void valueChanged(); + +private: + qreal m_value = 0; +}; + +#endif // RESETTABLE_H diff --git a/tests/auto/qml/qmlcppcodegen/data/resettable.qml b/tests/auto/qml/qmlcppcodegen/data/resettable.qml new file mode 100644 index 0000000000..59fa31645e --- /dev/null +++ b/tests/auto/qml/qmlcppcodegen/data/resettable.qml @@ -0,0 +1,15 @@ +pragma Strict +import QtQml +import TestTypes + +Resettable { + id: self + value: 999 + + property Resettable shadowing: Resettable { + property var shadowable: undefined + } + + function doReset() { self.value = undefined } + function doReset2() { self.value = shadowing.shadowable } +} diff --git a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp index b11754b081..cf8d122261 100644 --- a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp +++ b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -187,6 +188,7 @@ private slots: void registerElimination(); void registerPropagation(); void renameAdjust(); + void resettableProperty(); void returnAfterReject(); void revisions(); void scopeIdLookup(); @@ -3850,6 +3852,27 @@ void tst_QmlCppCodegen::renameAdjust() QVERIFY(o); } +void tst_QmlCppCodegen::resettableProperty() +{ + QQmlEngine engine; + QQmlComponent c(&engine, QUrl(u"qrc:/qt/qml/TestTypes/resettable.qml"_s)); + QVERIFY2(c.isReady(), qPrintable(c.errorString())); + QScopedPointer o(c.create()); + QVERIFY(o); + + ResettableProperty *resettable = qobject_cast(o.data()); + QVERIFY(resettable); + + QCOMPARE(resettable->value(), 999); + QMetaObject::invokeMethod(resettable, "doReset"); + QCOMPARE(resettable->value(), 0); + + resettable->setValue(82); + QCOMPARE(resettable->value(), 82); + QMetaObject::invokeMethod(resettable, "doReset2"); + QCOMPARE(resettable->value(), 0); +} + void tst_QmlCppCodegen::returnAfterReject() { QQmlEngine engine;