From ae76c8f428e9ca7ac5d398b51ffdb03407a26f04 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Tue, 2 Jul 2024 10:43:06 +0200 Subject: [PATCH] QmlCompiler: Fix side effect detection for array methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Array methods that manipulate the array they are called on always have side effects, on that array. In order to optimize them out we'd have to do some more involved tracking of affected values. Amends commit e84686415187455a7153d61ca82478053f13e3f9 Fixes: QTBUG-126834 Pick-to: 6.8 6.7 Change-Id: Ia4395ea21e89590e6ffe95e236f70b5e64402f5e Reviewed-by: Olivier De Cannière Reviewed-by: Fabian Kosmale --- src/qmlcompiler/qqmljstypepropagator.cpp | 16 ++++++---------- .../qmlcppcodegen/data/jsArrayMethodsUntyped.qml | 7 +++++++ .../auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp | 2 ++ 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/qmlcompiler/qqmljstypepropagator.cpp b/src/qmlcompiler/qqmljstypepropagator.cpp index 22854c00c4..fba1f43cce 100644 --- a/src/qmlcompiler/qqmljstypepropagator.cpp +++ b/src/qmlcompiler/qqmljstypepropagator.cpp @@ -1625,10 +1625,6 @@ bool QQmlJSTypePropagator::propagateArrayMethod( const auto valueContained = baseContained->valueType(); const auto valueType = m_typeResolver->globalType(valueContained); - const bool canHaveSideEffects = (baseType.isProperty() && baseType.isWritable()) - || baseContained->isListProperty() - || baseType.isConversion(); - const auto setReturnType = [&](const QQmlJSScope::ConstPtr type) { setAccumulator(m_typeResolver->returnType( type, QQmlJSRegisterContent::MethodReturnValue, baseContained)); @@ -1643,7 +1639,7 @@ bool QQmlJSTypePropagator::propagateArrayMethod( for (int i = 0; i < argc; ++i) addReadRegister(argv + i, intType); - m_state.setHasSideEffects(canHaveSideEffects); + m_state.setHasSideEffects(true); setReturnType(baseContained); return true; } @@ -1662,7 +1658,7 @@ bool QQmlJSTypePropagator::propagateArrayMethod( for (int i = 1; i < argc; ++i) addReadRegister(argv + i, intType); - m_state.setHasSideEffects(canHaveSideEffects); + m_state.setHasSideEffects(true); setReturnType(baseContained); return true; } @@ -1694,7 +1690,7 @@ bool QQmlJSTypePropagator::propagateArrayMethod( } if ((name == u"pop" || name == u"shift") && argc == 0) { - m_state.setHasSideEffects(canHaveSideEffects); + m_state.setHasSideEffects(true); setReturnType(valueContained); return true; } @@ -1708,13 +1704,13 @@ bool QQmlJSTypePropagator::propagateArrayMethod( for (int i = 0; i < argc; ++i) addReadRegister(argv + i, valueType); - m_state.setHasSideEffects(canHaveSideEffects); + m_state.setHasSideEffects(true); setReturnType(m_typeResolver->int32Type()); return true; } if (name == u"reverse" && argc == 0) { - m_state.setHasSideEffects(canHaveSideEffects); + m_state.setHasSideEffects(true); setReturnType(baseContained); return true; } @@ -1751,7 +1747,7 @@ bool QQmlJSTypePropagator::propagateArrayMethod( for (int i = 2; i < argc; ++i) addReadRegister(argv + i, valueType); - m_state.setHasSideEffects(canHaveSideEffects); + m_state.setHasSideEffects(true); setReturnType(baseContained); return true; } diff --git a/tests/auto/qml/qmlcppcodegen/data/jsArrayMethodsUntyped.qml b/tests/auto/qml/qmlcppcodegen/data/jsArrayMethodsUntyped.qml index 7426c692fe..ec61adce19 100644 --- a/tests/auto/qml/qmlcppcodegen/data/jsArrayMethodsUntyped.qml +++ b/tests/auto/qml/qmlcppcodegen/data/jsArrayMethodsUntyped.qml @@ -14,4 +14,11 @@ QtObject { property string jsArrayJoin: jsArray().join() property int jsArrayIndexOf: jsArray().indexOf(l2) property int jsArrayLastIndexOf: jsArray().lastIndexOf(l3) + + property string pushAndJoin: { + var s = []; + s.push("A") + s.push("B") + return s.join("+"); + } } diff --git a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp index 21d649a690..22479d3d9c 100644 --- a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp +++ b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp @@ -2752,6 +2752,8 @@ void tst_QmlCppCodegen::jsArrayMethods() QCOMPARE(object->property("listPropertyLastIndexOf"), object->property("jsArrayLastIndexOf")); QCOMPARE(object->property("listPropertyLastIndexOf").toInt(), 5); + + QCOMPARE(check->property("pushAndJoin").toString(), QStringLiteral("A+B")); } void tst_QmlCppCodegen::jsArrayMethodsWithParams()