QmlCompiler: Preserve external side effects across jumps

Now that we discern internal and external side effects, we cannot
implicitly rely on every jump to generate a side effect anymore. We need
to actually update the virtual registers and also merge the side effects
(along with other flags).

Amends commit 6b14ba5c2f

Pick-to: 6.10 6.9 6.8 6.5
Task-number: QTBUG-137540
Change-Id: I6b46c7a4773759c8f6f30308ba72082555ce3e61
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Olivier De Cannière <olivier.decanniere@qt.io>

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);
+    const VirtualRegister merged = {
-    Q_ASSERT(merged.isValid());
+        (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.annotations[currentInstructionOffset()].typeConversions[index] = merged;
-        m_state.registers[index].content = 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()
+        if (lastTry.content.conversionResultType() != merged.content.conversionResultType()
-                || lastTry.content.conversionOrigins() != merged.conversionOrigins()) {
+                || 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;
+        const VirtualRegister cloned = {
-        Q_ASSERT(cloned.isValid());
+            (a == b) ? m_pool->clone(merged.content) : merged.content,
-        m_state.annotations[currentInstructionOffset()].typeConversions[index].content = cloned;
+            merged.canMove,
-        m_state.registers[index].content = cloned;
+            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;
+            const VirtualRegister &newType = registerIt.value();
-            if (!newType.isValid()) {
+            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);
         }

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

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

tests/auto/qml/qmlcppcodegen/data/mergeSideEffects.qml

@@ -0,0 +1,20 @@
+import QtQml
+
+QtObject {
+    property bool no: false
+    property list<int> a: [1]
+    property list<int> 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];
+    }
+}

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<QObject> o(c.create());
+    QVERIFY(!o.isNull());
+    QCOMPARE(o->property("c").toInt(), 3);
+}
+
 void tst_QmlCppCodegen::mergedObjectReadWrite()
 {
     QQmlEngine e;