From 60a34c33934e264e42d7aef00500f5ebf7f03bff Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Tue, 9 May 2023 10:47:06 +0200 Subject: QmlCompiler: Optimize list manipulations We should never store a list in a wrapper type that is itself a different list. Wrapping and unwrapping requires rebuilding the list in such cases. We can, however, store lists of builtins as-is. There is no need to transform them. Other lists can still be stored in QVariant. As a result, we now need to discern between the access semantics of the stored type and the access semantics of the contained type. They are not guaranteed to be the same anymore. Furthermore, we need to reject "internal" manipulation of QVariant-wrapped lists for now. We might implement them using QMetaSequence, though. Task-number: QTBUG-113465 Change-Id: If09ea345b2fac39bf2abd62a2fce2d354df85b6b Reviewed-by: Fabian Kosmale --- src/qmlcompiler/qqmljscodegenerator.cpp | 40 +++++++++++++++----------- src/qmlcompiler/qqmljstypepropagator.cpp | 6 ++-- src/qmlcompiler/qqmljstyperesolver.cpp | 28 +++++++----------- tests/auto/qml/qmlcppcodegen/data/failures.qml | 5 ++++ 4 files changed, 41 insertions(+), 38 deletions(-) diff --git a/src/qmlcompiler/qqmljscodegenerator.cpp b/src/qmlcompiler/qqmljscodegenerator.cpp index 2072b2a8ad..c1df2d9cca 100644 --- a/src/qmlcompiler/qqmljscodegenerator.cpp +++ b/src/qmlcompiler/qqmljscodegenerator.cpp @@ -725,6 +725,8 @@ void QQmlJSCodeGenerator::generate_LoadElement(int base) access = u"QString("_s + access + u")"_s; else if (m_state.isRegisterAffectedBySideEffects(base)) reject(u"LoadElement on a sequence potentially affected by side effects"_s); + else if (baseType.storedType()->accessSemantics() != QQmlJSScope::AccessSemantics::Sequence) + reject(u"LoadElement on a sequence wrapped in a non-sequence type"_s); m_body += u"if ("_s + indexName + u" < "_s + baseName + u".size())\n"_s; m_body += u" "_s + m_state.accumulatorVariableOut + u" = "_s + @@ -1091,15 +1093,28 @@ void QQmlJSCodeGenerator::generate_GetLookup(int index) const QString preparation = getLookupPreparation( m_state.accumulatorOut(), m_state.accumulatorVariableOut, index); generateLookup(lookup, initialization, preparation); - } else if (m_typeResolver->registerIsStoredIn(accumulatorIn, m_typeResolver->listPropertyType()) + } else if ((accumulatorIn.isList() + || m_typeResolver->registerContains(accumulatorIn, m_typeResolver->stringType())) && m_jsUnitGenerator->lookupName(index) == u"length"_s) { - m_body += m_state.accumulatorVariableOut + u" = "_s; - m_body += conversion( - m_typeResolver->globalType(m_typeResolver->int32Type()), - m_state.accumulatorOut(), - m_state.accumulatorVariableIn + u".count("_s + u'&' - + m_state.accumulatorVariableIn + u')'); - m_body += u";\n"_s; + const QQmlJSScope::ConstPtr stored = accumulatorIn.storedType(); + if (stored->isListProperty()) { + m_body += m_state.accumulatorVariableOut + u" = "_s; + m_body += conversion( + m_typeResolver->globalType(m_typeResolver->int32Type()), + m_state.accumulatorOut(), + m_state.accumulatorVariableIn + u".count("_s + u'&' + + m_state.accumulatorVariableIn + u')'); + m_body += u";\n"_s; + } else if (stored->accessSemantics() == QQmlJSScope::AccessSemantics::Sequence + || m_typeResolver->equals(stored, m_typeResolver->stringType())) { + m_body += m_state.accumulatorVariableOut + u" = "_s + + conversion(m_typeResolver->globalType(m_typeResolver->int32Type()), + m_state.accumulatorOut(), + m_state.accumulatorVariableIn + u".length()"_s) + + u";\n"_s; + } else { + reject(u"access to 'length' property of sequence wrapped in non-sequence"_s); + } } else if (m_typeResolver->registerIsStoredIn(accumulatorIn, m_typeResolver->variantMapType())) { QString mapLookup = m_state.accumulatorVariableIn + u"["_s @@ -1108,15 +1123,6 @@ void QQmlJSCodeGenerator::generate_GetLookup(int index) m_body += conversion(m_typeResolver->globalType(m_typeResolver->varType()), m_state.accumulatorOut(), mapLookup); m_body += u";\n"_s; - } else if ((m_typeResolver->registerIsStoredIn(accumulatorIn, m_typeResolver->stringType()) - || accumulatorIn.storedType()->accessSemantics() - == QQmlJSScope::AccessSemantics::Sequence) - && m_jsUnitGenerator->lookupName(index) == u"length"_s) { - m_body += m_state.accumulatorVariableOut + u" = "_s - + conversion(m_typeResolver->globalType(m_typeResolver->int32Type()), - m_state.accumulatorOut(), - m_state.accumulatorVariableIn + u".length()"_s) - + u";\n"_s; } else { if (m_state.isRegisterAffectedBySideEffects(Accumulator)) reject(u"reading from a value that's potentially affected by side effects"_s); diff --git a/src/qmlcompiler/qqmljstypepropagator.cpp b/src/qmlcompiler/qqmljstypepropagator.cpp index 19d8e8a695..609f8d8100 100644 --- a/src/qmlcompiler/qqmljstypepropagator.cpp +++ b/src/qmlcompiler/qqmljstypepropagator.cpp @@ -660,8 +660,8 @@ void QQmlJSTypePropagator::generate_LoadElement(int base) { const QQmlJSRegisterContent baseRegister = m_state.registers[base].content; - if ((baseRegister.storedType()->accessSemantics() != QQmlJSScope::AccessSemantics::Sequence - && !m_typeResolver->registerIsStoredIn(baseRegister, m_typeResolver->stringType())) + if ((!baseRegister.isList() + && !m_typeResolver->registerContains(baseRegister, m_typeResolver->stringType())) || !m_typeResolver->isNumeric(m_state.accumulatorIn())) { const auto jsValue = m_typeResolver->globalType(m_typeResolver->jsValueType()); addReadAccumulator(jsValue); @@ -690,7 +690,7 @@ void QQmlJSTypePropagator::generate_StoreElement(int base, int index) const QQmlJSRegisterContent baseRegister = m_state.registers[base].content; const QQmlJSRegisterContent indexRegister = checkedInputRegister(index); - if (baseRegister.storedType()->accessSemantics() != QQmlJSScope::AccessSemantics::Sequence + if (!baseRegister.isList() || !m_typeResolver->isNumeric(indexRegister)) { const auto jsValue = m_typeResolver->globalType(m_typeResolver->jsValueType()); addReadAccumulator(jsValue); diff --git a/src/qmlcompiler/qqmljstyperesolver.cpp b/src/qmlcompiler/qqmljstyperesolver.cpp index 7fb37f3dfa..17c79095a2 100644 --- a/src/qmlcompiler/qqmljstyperesolver.cpp +++ b/src/qmlcompiler/qqmljstyperesolver.cpp @@ -814,26 +814,18 @@ QQmlJSScope::ConstPtr QQmlJSTypeResolver::genericType( if (type->scopeType() == QQmlJSScope::EnumScope) return type->baseType(); - if (isPrimitive(type) || equals(type, m_jsValueType) || equals(type, m_urlType) - || equals(type, m_dateTimeType) || equals(type, m_dateType) || equals(type, m_timeType) - || equals(type, m_variantListType) || equals(type, m_variantMapType) - || equals(type, m_varType) || equals(type, m_stringListType) - || equals(type, m_byteArrayType)) { + if (isPrimitive(type)) return type; - } - if (type->accessSemantics() == QQmlJSScope::AccessSemantics::Sequence) { - if (const QQmlJSScope::ConstPtr valueType = type->valueType()) { - switch (valueType->accessSemantics()) { - case QQmlJSScope::AccessSemantics::Value: - return genericType(valueType)->listType(); - case QQmlJSScope::AccessSemantics::Reference: - return m_qObjectListType; - default: - break; - } - } - return m_variantListType; + for (const QQmlJSScope::ConstPtr &builtin : { + m_realType, m_floatType, m_int8Type, m_uint8Type, m_int16Type, m_uint16Type, + m_int32Type, m_uint32Type, m_int64Type, m_uint64Type, m_boolType, m_stringType, + m_stringListType, m_byteArrayType, m_urlType, m_dateTimeType, m_dateType, + m_timeType, m_variantListType, m_variantMapType, m_varType, m_jsValueType, + m_jsPrimitiveType, m_listPropertyType, m_qObjectType, m_qObjectListType, + m_metaObjectType }) { + if (equals(type, builtin) || equals(type, builtin->listType())) + return type; } return m_varType; diff --git a/tests/auto/qml/qmlcppcodegen/data/failures.qml b/tests/auto/qml/qmlcppcodegen/data/failures.qml index b19b7f9ea2..e6f3af5e87 100644 --- a/tests/auto/qml/qmlcppcodegen/data/failures.qml +++ b/tests/auto/qml/qmlcppcodegen/data/failures.qml @@ -82,4 +82,9 @@ QtObject { } return a; } + + // TODO: Drop these once we can manipulate QVariant-wrapped lists. + property list withLengths + property int l: withLengths.length + property withLength w: withLengths[10] } -- cgit v1.2.1