diff --git a/src/qmlcompiler/qqmljstypepropagator.cpp b/src/qmlcompiler/qqmljstypepropagator.cpp index e5b0446e7d..76465ea214 100644 --- a/src/qmlcompiler/qqmljstypepropagator.cpp +++ b/src/qmlcompiler/qqmljstypepropagator.cpp @@ -1535,19 +1535,25 @@ void QQmlJSTypePropagator::setRegister(int index, QQmlJSRegisterContent content) } void QQmlJSTypePropagator::mergeRegister( - int index, QQmlJSRegisterContent a, QQmlJSRegisterContent b) + int index, const VirtualRegister &a, const VirtualRegister &b) { - const QQmlJSRegisterContent merged = (a == b) ? a : m_typeResolver->merge(a, b); - Q_ASSERT(merged.isValid()); + const VirtualRegister merged = { + (a.content == b.content) ? a.content : m_typeResolver->merge(a.content, b.content), + a.canMove && b.canMove, + a.affectedBySideEffects || b.affectedBySideEffects, + a.isShadowable || b.isShadowable, + }; - if (!merged.isConversion()) { + Q_ASSERT(merged.content.isValid()); + + if (!merged.content.isConversion()) { // The registers were the same. We're already tracking them. - m_state.annotations[currentInstructionOffset()].typeConversions[index].content = merged; - m_state.registers[index].content = merged; + m_state.annotations[currentInstructionOffset()].typeConversions[index] = merged; + m_state.registers[index] = merged; return; } - auto tryPrevStateConversion = [this](int index, QQmlJSRegisterContent merged) -> bool { + auto tryPrevStateConversion = [this](int index, const VirtualRegister &merged) -> bool { auto it = m_prevStateAnnotations.find(currentInstructionOffset()); if (it == m_prevStateAnnotations.end()) return false; @@ -1562,23 +1568,35 @@ void QQmlJSTypePropagator::mergeRegister( if (!lastTry.content.isConversion()) return false; - if (lastTry.content.conversionResultType() != merged.conversionResultType() - || lastTry.content.conversionOrigins() != merged.conversionOrigins()) { + if (lastTry.content.conversionResultType() != merged.content.conversionResultType() + || lastTry.content.conversionOrigins() != merged.content.conversionOrigins() + || lastTry.canMove != merged.canMove + || lastTry.affectedBySideEffects != merged.affectedBySideEffects + || lastTry.isShadowable != merged.isShadowable) { return false; } // We don't need to track it again if we've come to the same conclusion before. m_state.annotations[currentInstructionOffset()].typeConversions[index] = lastTry; + + // Do not reset the side effects + Q_ASSERT(!m_state.registers[index].affectedBySideEffects || lastTry.affectedBySideEffects); + m_state.registers[index] = lastTry; return true; }; if (!tryPrevStateConversion(index, merged)) { // if a != b, we have already re-tracked it. - QQmlJSRegisterContent cloned = (a == b) ? m_pool->clone(merged) : merged; - Q_ASSERT(cloned.isValid()); - m_state.annotations[currentInstructionOffset()].typeConversions[index].content = cloned; - m_state.registers[index].content = cloned; + const VirtualRegister cloned = { + (a == b) ? m_pool->clone(merged.content) : merged.content, + merged.canMove, + merged.affectedBySideEffects, + merged.isShadowable, + }; + Q_ASSERT(cloned.content.isValid()); + m_state.annotations[currentInstructionOffset()].typeConversions[index] = cloned; + m_state.registers[index] = cloned; } } @@ -3019,8 +3037,8 @@ QQmlJSTypePropagator::startInstruction(QV4::Moth::Instr::Type type) registerIt != end; ++registerIt) { const int registerIndex = registerIt.key(); - auto newType = registerIt.value().content; - if (!newType.isValid()) { + const VirtualRegister &newType = registerIt.value(); + if (!newType.content.isValid()) { addError(u"When reached from offset %1, %2 is undefined"_s .arg(stateToMerge.originatingOffset) .arg(registerName(registerIndex))); @@ -3029,7 +3047,7 @@ QQmlJSTypePropagator::startInstruction(QV4::Moth::Instr::Type type) auto currentRegister = m_state.registers.find(registerIndex); if (currentRegister != m_state.registers.end()) - mergeRegister(registerIndex, newType, currentRegister.value().content); + mergeRegister(registerIndex, newType, currentRegister.value()); else mergeRegister(registerIndex, newType, newType); } diff --git a/src/qmlcompiler/qqmljstypepropagator_p.h b/src/qmlcompiler/qqmljstypepropagator_p.h index 4413a295b9..3da4ac73e7 100644 --- a/src/qmlcompiler/qqmljstypepropagator_p.h +++ b/src/qmlcompiler/qqmljstypepropagator_p.h @@ -234,7 +234,7 @@ private: void setAccumulator(QQmlJSRegisterContent content); void setRegister(int index, QQmlJSRegisterContent content); - void mergeRegister(int index, QQmlJSRegisterContent a, QQmlJSRegisterContent b); + void mergeRegister(int index, const VirtualRegister &a, const VirtualRegister &b); void addReadRegister(int index); void addReadRegister(int index, QQmlJSRegisterContent convertTo); diff --git a/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt b/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt index 36fb7ab01b..dd5dc3ab73 100644 --- a/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt +++ b/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt @@ -223,6 +223,7 @@ set(qml_files mathMinMax.qml mathOperations.qml mathStaticProperties.qml + mergeSideEffects.qml mergedObjectRead.qml mergedObjectWrite.qml methodOnListLookup.qml diff --git a/tests/auto/qml/qmlcppcodegen/data/mergeSideEffects.qml b/tests/auto/qml/qmlcppcodegen/data/mergeSideEffects.qml new file mode 100644 index 0000000000..d4a8fec42e --- /dev/null +++ b/tests/auto/qml/qmlcppcodegen/data/mergeSideEffects.qml @@ -0,0 +1,20 @@ +import QtQml + +QtObject { + property bool no: false + property list a: [1] + property list b: [2] + + property int c: { + let numbers = a; + a = [3]; // create side effect affecting "numbers" + + if (no) { + // Force two branches to be merged on "numbers" + numbers = b + } + + // Side effect is still in effect + return numbers[0]; + } +} diff --git a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp index 00c00b4c0f..39292d9dd8 100644 --- a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp +++ b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp @@ -184,6 +184,7 @@ private slots: void mathMinMax(); void mathOperations(); void mathStaticProperties(); + void mergeSideEffects(); void mergedObjectReadWrite(); void methodOnListLookup(); void methods(); @@ -3712,6 +3713,16 @@ void tst_QmlCppCodegen::mathStaticProperties() QCOMPARE(object->property("sqrt2").toDouble(), 1.4142135623730951); } +void tst_QmlCppCodegen::mergeSideEffects() +{ + QQmlEngine engine; + QQmlComponent c(&engine, QUrl(u"qrc:/qt/qml/TestTypes/mergeSideEffects.qml"_s)); + QVERIFY2(c.isReady(), qPrintable(c.errorString())); + QScopedPointer o(c.create()); + QVERIFY(!o.isNull()); + QCOMPARE(o->property("c").toInt(), 3); +} + void tst_QmlCppCodegen::mergedObjectReadWrite() { QQmlEngine e;