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 <fabian.kosmale@qt.io>

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<withLength> withLengths
+    property int l: withLengths.length
+    property withLength w: withLengths[10]
 }