From 0de0b0ffdb44d73c605e20f00934dfb44bdf7ad9 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Mon, 06 Oct 2025 10:09:01 +0200 Subject: [PATCH] QmlCompiler: Fix write access to QVariantMap Without this, it tries to resolve the metaObject of QVariantMap, which crashes. Amends commit cca07aa78841f2d743f0b4d933abb0dd66f0b948. Fixes: QTBUG-139626 Pick-to: 6.8 Change-Id: Id747429ed0d558932b9a6cb8f59e3740982efb56 Reviewed-by: Olivier De Cannière (cherry picked from commit f5e34266ea15c6e44e9816f01f4e627d5f038f0c) Reviewed-by: Qt Cherry-pick Bot --- diff --git a/src/qmlcompiler/qqmljscodegenerator.cpp b/src/qmlcompiler/qqmljscodegenerator.cpp index ce50e4c..b8ad569 100644 --- a/src/qmlcompiler/qqmljscodegenerator.cpp +++ b/src/qmlcompiler/qqmljscodegenerator.cpp @@ -1472,7 +1472,7 @@ generate_GetLookupHelper(index); } -QString QQmlJSCodeGenerator::generateVariantMapLookup( +QString QQmlJSCodeGenerator::generateVariantMapGetLookup( const QString &map, const int nameIndex) { const QString mapLookup = map @@ -1483,6 +1483,18 @@ + u";\n"_s; } +QString QQmlJSCodeGenerator::generateVariantMapSetLookup( + const QString &map, const int nameIndex, + const QQmlJSScope::ConstPtr &property, const QString &variableIn) +{ + const QString mapLookup = map + + u"["_s + QQmlJSUtils::toLiteral(m_jsUnitGenerator->lookupName(nameIndex)) + u"]"_s; + + return mapLookup + u" = "_s + + conversion(property, m_typeResolver->varType(), variableIn) + + u";\n"_s; +} + void QQmlJSCodeGenerator::generate_GetLookupHelper(int index) { if (m_state.accumulatorOut().isMethod()) @@ -1628,7 +1640,7 @@ REJECT(u"access to 'length' property of sequence wrapped in non-sequence"_s); } } else if (accumulatorIn.isStoredIn(m_typeResolver->variantMapType())) { - m_body += generateVariantMapLookup(m_state.accumulatorVariableIn, index); + m_body += generateVariantMapGetLookup(m_state.accumulatorVariableIn, index); } else { if (m_state.isRegisterAffectedBySideEffects(Accumulator)) REJECT(u"reading from a value that's potentially affected by side effects"_s); @@ -1639,7 +1651,7 @@ m_jsUnitGenerator->lookupName(index))); if (scope.contains(m_typeResolver->variantMapType())) { - m_body += generateVariantMapLookup( + m_body += generateVariantMapGetLookup( u"(*static_cast("_s + inputContentPointer + u"))"_s, index); return; @@ -1699,6 +1711,15 @@ REJECT(u"StoreProperty"_s); } +// TODO: This shouldn't be necessary. If the content can be stored directly, then it should +// be stored and used directly. If it cannot be stored directly, it should be stored +// as QVariant, but then we cannot dereference the content pointer either. +static QString derefContentPointer(const QString &contentPointer) +{ + Q_ASSERT(contentPointer.startsWith(u'&') || contentPointer[0].isLetterOrNumber()); + return contentPointer.startsWith(u'&') ? contentPointer.mid(1) : (u'*' + contentPointer); +} + void QQmlJSCodeGenerator::generate_SetLookup(int index, int baseReg) { INJECT_TRACE_INFO(generate_SetLookup); @@ -1707,8 +1728,9 @@ const QQmlJSScope::ConstPtr valueType = m_state.accumulatorIn().storedType(); const QQmlJSRegisterContent property = m_state.readAccumulator(); Q_ASSERT(property.isConversion()); - const QQmlJSScope::ConstPtr originalScope - = m_typeResolver->original(property.conversionResultScope()).containedType(); + const QQmlJSRegisterContent original + = m_typeResolver->original(property.conversionResultScope()); + const QQmlJSScope::ConstPtr originalScope = original.containedType(); if (property.storedType().isNull()) { REJECT(u"SetLookup. Could not find property " @@ -1758,9 +1780,7 @@ // We can resize without write back on a list property because it's actually a reference. m_body += u"const int begin = "_s + object + u".count(&" + object + u");\n"_s; - m_body += u"const int end = "_s - + (variableIn.startsWith(u'&') ? variableIn.mid(1) : (u'*' + variableIn)) - + u";\n"_s; + m_body += u"const int end = "_s + derefContentPointer(variableIn) + u";\n"_s; m_body += u"for (int i = begin; i < end; ++i)\n"_s; m_body += u" "_s + object + u".append(&"_s + object + u", nullptr);\n"_s; m_body += u"for (int i = begin; i > end; --i)\n"_s; @@ -1770,10 +1790,26 @@ } case QQmlJSScope::AccessSemantics::Value: { const QQmlJSRegisterContent base = registerType(baseReg); + if (base.isStoredIn(m_typeResolver->variantMapType())) { + m_body += generateVariantMapSetLookup( + registerVariable(baseReg), index, property.storedType(), + derefContentPointer(variableIn)); + generateWriteBack(baseReg); + break; + } const QString baseContentPointer = resolveValueTypeContentPointer( originalScope, base, object, u"TypeError: Value is %1 and could not be converted to an object"_s); + if (original.contains(m_typeResolver->variantMapType())) { + m_body += generateVariantMapSetLookup( + u"(*static_cast("_s + + baseContentPointer + u"))"_s, index, property.storedType(), + derefContentPointer(variableIn)); + generateWriteBack(baseReg); + break; + } + const QString lookup = u"aotContext->setValueLookup("_s + indexString + u", "_s + baseContentPointer + u", "_s + variableIn + u')'; diff --git a/src/qmlcompiler/qqmljscodegenerator_p.h b/src/qmlcompiler/qqmljscodegenerator_p.h index ce160bf..46da0bb 100644 --- a/src/qmlcompiler/qqmljscodegenerator_p.h +++ b/src/qmlcompiler/qqmljscodegenerator_p.h @@ -366,7 +366,10 @@ const QQmlJSMetaMethod &ctor, const QList &argumentTypes, const QStringList &arguments, const QString &metaType, const QString &metaObject); - QString generateVariantMapLookup(const QString &map, const int nameIndex); + QString generateVariantMapGetLookup(const QString &map, const int nameIndex); + QString generateVariantMapSetLookup( + const QString &map, const int nameIndex, const QQmlJSScope::ConstPtr &property, + const QString &variableIn); QQmlJSRegisterContent originalType(QQmlJSRegisterContent tracked) { diff --git a/tests/auto/qml/qmlcppcodegen/data/variantMapLookup.h b/tests/auto/qml/qmlcppcodegen/data/variantMapLookup.h index de7ac4f..c1e5206 100644 --- a/tests/auto/qml/qmlcppcodegen/data/variantMapLookup.h +++ b/tests/auto/qml/qmlcppcodegen/data/variantMapLookup.h @@ -7,19 +7,32 @@ { Q_OBJECT QML_ELEMENT - Q_PROPERTY(QVariantMap data READ data CONSTANT) - Q_PROPERTY(QList many READ many CONSTANT) + Q_PROPERTY(QVariantMap data READ data WRITE setData NOTIFY dataChanged) + Q_PROPERTY(QList many READ many NOTIFY dataChanged) public: VariantMapLookupFoo(QObject *parent = nullptr) : QObject(parent) { } -private: - QVariantMap data() const { return { { QStringLiteral("value"), 42 } }; } + QVariantMap data() const { return m_data; } + void setData(const QVariantMap &data) + { + if (data == m_data) + return; + m_data = data; + emit dataChanged(); + } + QList many() const { const QVariantMap one = data(); return QList({one, one, one}); } + +signals: + void dataChanged(); + +private: + QVariantMap m_data; }; diff --git a/tests/auto/qml/qmlcppcodegen/data/variantMapLookup.qml b/tests/auto/qml/qmlcppcodegen/data/variantMapLookup.qml index 45cb0ed..75b4fd0 100644 --- a/tests/auto/qml/qmlcppcodegen/data/variantMapLookup.qml +++ b/tests/auto/qml/qmlcppcodegen/data/variantMapLookup.qml @@ -3,10 +3,28 @@ import QtQuick Item { + id: root property int i: moo.data.value property int j: moo.many[1].value + property string foo: moo.data.foo VariantMapLookupFoo { id: moo + data: { + let result = { value: 42 }; + switch(root.visible) { + case true: + result.foo = "blue"; + break; + case false: + result.foo = "green"; + break; + } + return result; + } + } + + function doI() { + moo.data.value = i + 1 } } diff --git a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp index 6a9c50c..9f85f64 100644 --- a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp +++ b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp @@ -6097,6 +6097,12 @@ QVERIFY(!o.isNull()); QCOMPARE(o->property("i"), 42); QCOMPARE(o->property("j"), 42); + QCOMPARE(o->property("foo"), u"blue"_s); + + QMetaObject::invokeMethod(o.data(), "doI"); + QCOMPARE(o->property("i"), 43); + QCOMPARE(o->property("j"), 43); + QCOMPARE(o->property("foo"), u"blue"_s); } void tst_QmlCppCodegen::variantReturn()