qqmltypecompiler: align runtime function table order to qmlcachegen

When we write runtime functions to compilation unit at run time, the
order of the functions in the unit (often) differs from the order of
functions in the unit produced ahead of time by qmlcachegen and friends.
Additionally, the order also differs from what qmltc expects (and
qmlcompiler library in general)

Fix the order by simplifying the procedure of JS code generation when
we create the compilation unit at run time: new logic just goes over
the objects in the document linearly, instead of relying on bindings
(which are known to be out of order w.r.t. AST)

Change-Id: I4070b9d061f03c4c76d03120654ad3f30725493a
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
(cherry picked from commit 8d0adee3b3317f1fab03742bdf0f7cdbe57df914)
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

8 files changed, 91 insertions, 79 deletions

diff --git a/src/qml/common/qv4compileddata_p.h b/src/qml/common/qv4compileddata_p.h
index 3db3010885..2edb73f7b0 100644
--- a/src/qml/common/qv4compileddata_p.h
+++ b/src/qml/common/qv4compileddata_p.h
@@ -43,7 +43,7 @@ QT_BEGIN_NAMESPACE
 // Also change the comment behind the number to describe the latest change. This has the added
 // benefit that if another patch changes the version too, it will result in a merge conflict, and
 // not get removed silently.
-#define QV4_DATA_STRUCTURE_VERSION 0x35 // dropped builtin value types
+#define QV4_DATA_STRUCTURE_VERSION 0x36 // reordered runtime functions when compiling at run time
 
 class QIODevice;
 class QQmlTypeNameCache;
diff --git a/src/qml/compiler/qqmlirbuilder.cpp b/src/qml/compiler/qqmlirbuilder.cpp
index d5bc17735f..5beeda930f 100644
--- a/src/qml/compiler/qqmlirbuilder.cpp
+++ b/src/qml/compiler/qqmlirbuilder.cpp
@@ -1895,61 +1895,24 @@ QVector<int> JSCodeGen::generateJSCodeForFunctionsAndBindings(
     return runtimeFunctionIndices;
 }
 
-bool JSCodeGen::generateCodeForComponents(const QVector<quint32> &componentRoots)
+bool JSCodeGen::generateRuntimeFunctions(QmlIR::Object *object, bool storeSourceLocation)
 {
-    for (int i = 0; i < componentRoots.count(); ++i) {
-        if (!compileComponent(componentRoots.at(i)))
-            return false;
-    }
-
-    return compileComponent(/*root object*/0);
-}
-
-bool JSCodeGen::compileComponent(int contextObject)
-{
-    const QmlIR::Object *obj = document->objects.at(contextObject);
-    if (obj->flags & QV4::CompiledData::Object::IsComponent) {
-        Q_ASSERT(obj->bindingCount() == 1);
-        const QV4::CompiledData::Binding *componentBinding = obj->firstBinding();
-        Q_ASSERT(componentBinding->type() == QV4::CompiledData::Binding::Type_Object);
-        contextObject = componentBinding->value.objectIndex;
-    }
-
-    return compileJavaScriptCodeInObjectsRecursively(contextObject, contextObject);
-}
-
-bool JSCodeGen::compileJavaScriptCodeInObjectsRecursively(int objectIndex, int scopeObjectIndex)
-{
-    QmlIR::Object *object = document->objects.at(objectIndex);
-    if (object->flags & QV4::CompiledData::Object::IsComponent)
+    if (object->functionsAndExpressions->count == 0)
         return true;
 
-    for (auto it = object->inlineComponentsBegin(); it != object->inlineComponentsEnd(); ++it)
-        compileComponent(it->objectIndex);
-
-    if (object->functionsAndExpressions->count > 0) {
-        QList<QmlIR::CompiledFunctionOrExpression> functionsToCompile;
-        for (QmlIR::CompiledFunctionOrExpression *foe = object->functionsAndExpressions->first; foe; foe = foe->next)
-            functionsToCompile << *foe;
-        const QVector<int> runtimeFunctionIndices = generateJSCodeForFunctionsAndBindings(functionsToCompile);
-        if (hasError())
-            return false;
-
-        object->runtimeFunctionIndices.allocate(document->jsParserEngine.pool(),
-                                                runtimeFunctionIndices);
+    QList<QmlIR::CompiledFunctionOrExpression> functionsToCompile;
+    functionsToCompile.reserve(object->functionsAndExpressions->count);
+    for (QmlIR::CompiledFunctionOrExpression *foe = object->functionsAndExpressions->first; foe;
+         foe = foe->next) {
+        functionsToCompile << *foe;
     }
 
-    for (const QmlIR::Binding *binding = object->firstBinding(); binding; binding = binding->next) {
-        const Binding::Type bindingType = binding->type();
-        if (bindingType < QV4::CompiledData::Binding::Type_Object)
-            continue;
-
-        int target = binding->value.objectIndex;
-        int scope = bindingType == QV4::CompiledData::Binding::Type_Object ? target : scopeObjectIndex;
-
-        if (!compileJavaScriptCodeInObjectsRecursively(binding->value.objectIndex, scope))
-            return false;
-    }
+    const auto runtimeFunctionIndices =
+            generateJSCodeForFunctionsAndBindings(functionsToCompile, storeSourceLocation);
+    if (hasError())
+        return false;
 
+    object->runtimeFunctionIndices.allocate(document->jsParserEngine.pool(),
+                                            runtimeFunctionIndices);
     return true;
 }
diff --git a/src/qml/compiler/qqmlirbuilder_p.h b/src/qml/compiler/qqmlirbuilder_p.h
index be7bc591cf..e9c669b809 100644
--- a/src/qml/compiler/qqmlirbuilder_p.h
+++ b/src/qml/compiler/qqmlirbuilder_p.h
@@ -584,9 +584,7 @@ struct Q_QML_COMPILER_PRIVATE_EXPORT JSCodeGen : public QV4::Compiler::Codegen
     generateJSCodeForFunctionsAndBindings(const QList<CompiledFunctionOrExpression> &functions,
                                           bool storeSourceLocation = false);
 
-    bool generateCodeForComponents(const QVector<quint32> &componentRoots);
-    bool compileComponent(int contextObject);
-    bool compileJavaScriptCodeInObjectsRecursively(int objectIndex, int scopeObjectIndex);
+    bool generateRuntimeFunctions(QmlIR::Object *object, bool storeSourceLocation = true);
 
 private:
     Document *document;
diff --git a/src/qml/qml/qqmltypecompiler.cpp b/src/qml/qml/qqmltypecompiler.cpp
index b51970e5b2..64a9238739 100644
--- a/src/qml/qml/qqmltypecompiler.cpp
+++ b/src/qml/qml/qqmltypecompiler.cpp
@@ -117,11 +117,13 @@ QQmlRefPointer<QV4::ExecutableCompilationUnit> QQmlTypeCompiler::compile()
         document->jsModule.fileName = typeData->urlString();
         document->jsModule.finalUrl = typeData->finalUrlString();
         QmlIR::JSCodeGen v4CodeGenerator(document, engine->v4engine()->illegalNames());
-        if (!v4CodeGenerator.generateCodeForComponents(componentRoots())) {
-            recordError(v4CodeGenerator.error());
-            return nullptr;
+        for (QmlIR::Object *object : qAsConst(document->objects)) {
+            if (!v4CodeGenerator.generateRuntimeFunctions(object)) {
+                Q_ASSERT(v4CodeGenerator.hasError());
+                recordError(v4CodeGenerator.error());
+                return nullptr;
+            }
         }
-
         document->javaScriptCompilationUnit = v4CodeGenerator.generateCompilationUnit(/*generated unit data*/false);
     }
 
diff --git a/src/qmlcompiler/qqmljscompiler.cpp b/src/qmlcompiler/qqmljscompiler.cpp
index 227966bed2..76a7ed80e2 100644
--- a/src/qmlcompiler/qqmljscompiler.cpp
+++ b/src/qmlcompiler/qqmljscompiler.cpp
@@ -230,20 +230,13 @@ bool qCompileQmlFile(QmlIR::Document &irDocument, const QString &inputFileName,
         for (QmlIR::Object *object: qAsConst(irDocument.objects)) {
             if (object->functionsAndExpressions->count == 0 && object->bindingCount() == 0)
                 continue;
-            QList<QmlIR::CompiledFunctionOrExpression> functionsToCompile;
-            for (QmlIR::CompiledFunctionOrExpression *foe = object->functionsAndExpressions->first; foe; foe = foe->next)
-                functionsToCompile << *foe;
-            const QVector<int> runtimeFunctionIndices =
-                    v4CodeGen.generateJSCodeForFunctionsAndBindings(functionsToCompile,
-                                                                    storeSourceLocation);
-            if (v4CodeGen.hasError()) {
+
+            if (!v4CodeGen.generateRuntimeFunctions(object, storeSourceLocation)) {
+                Q_ASSERT(v4CodeGen.hasError());
                 error->appendDiagnostic(inputFileName, v4CodeGen.error());
                 return false;
             }
 
-            QQmlJS::MemoryPool *pool = irDocument.jsParserEngine.pool();
-            object->runtimeFunctionIndices.allocate(pool, runtimeFunctionIndices);
-
             if (!aotCompiler)
                 continue;
 
@@ -264,6 +257,12 @@ bool qCompileQmlFile(QmlIR::Document &irDocument, const QString &inputFileName,
             std::copy(object->functionsBegin(), object->functionsEnd(),
                       std::back_inserter(bindingsAndFunctions));
 
+            QList<QmlIR::CompiledFunctionOrExpression> functionsToCompile;
+            for (QmlIR::CompiledFunctionOrExpression *foe = object->functionsAndExpressions->first;
+                 foe; foe = foe->next) {
+                functionsToCompile << *foe;
+            }
+
             // AOT-compile bindings and functions in the same order as above so that the runtime
             // class indices match
             std::sort(bindingsAndFunctions.begin(), bindingsAndFunctions.end());
@@ -320,7 +319,8 @@ bool qCompileQmlFile(QmlIR::Document &irDocument, const QString &inputFileName,
                                            << diagnosticErrorMessage(inputFileName, *error);
                 } else if (auto *func = std::get_if<QQmlJSAotFunction>(&result)) {
                     qCDebug(lcAotCompiler) << "Generated code:" << func->code;
-                    aotFunctionsByIndex[runtimeFunctionIndices[bindingOrFunction.index()]] = *func;
+                    aotFunctionsByIndex[object->runtimeFunctionIndices[bindingOrFunction.index()]] =
+                            *func;
                 }
             });
         }
diff --git a/tests/auto/qml/qmltc/tst_qmltc.cpp b/tests/auto/qml/qmltc/tst_qmltc.cpp
index af0ba4878e..dfad4c15eb 100644
--- a/tests/auto/qml/qmltc/tst_qmltc.cpp
+++ b/tests/auto/qml/qmltc/tst_qmltc.cpp
@@ -1950,19 +1950,9 @@ void tst_qmltc::listView()
     QCOMPARE(model->count(), 0);
 
     created.appendDigit("5");
-    if (isCacheDisabled()) {
-        // TODO: doesn't work in no_disk_cache mode because
-        // QV4::Lookup::nextIndex values are different. These come from
-        // CompiledData::Lookup table of the compilation unit -- why would that
-        // change during QQmlTypeCompiler's CU generation?
-        QEXPECT_FAIL("", "Doesn't work without qmlcachegen - needs investigation", Continue);
-    }
     QCOMPARE(model->count(), 1);
 
     created.appendOperator("+");
-    if (isCacheDisabled()) { // same as above
-        QEXPECT_FAIL("", "Doesn't work without qmlcachegen - needs investigation", Continue);
-    }
     QCOMPARE(model->count(), 2);
 
     // TODO: add more testing (e.g. check that values are actually recorded)
diff --git a/tests/auto/qml/qqmljsscope/data/compilationUnitsCompatibility.qml b/tests/auto/qml/qqmljsscope/data/compilationUnitsCompatibility.qml
new file mode 100644
index 0000000000..bcc49537bb
--- /dev/null
+++ b/tests/auto/qml/qqmljsscope/data/compilationUnitsCompatibility.qml
@@ -0,0 +1,6 @@
+import QtQuick
+
+Text {
+    anchors.bottomMargin: Math.max(32, 31) + 10 // == 42
+    font.letterSpacing: Math.max(2, 3)
+}
diff --git a/tests/auto/qml/qqmljsscope/tst_qqmljsscope.cpp b/tests/auto/qml/qqmljsscope/tst_qqmljsscope.cpp
index 6bbb221396..a5efd3147e 100644
--- a/tests/auto/qml/qqmljsscope/tst_qqmljsscope.cpp
+++ b/tests/auto/qml/qqmljsscope/tst_qqmljsscope.cpp
@@ -23,6 +23,7 @@
 #include <private/qqmljstyperesolver_p.h>
 #include <QtQml/private/qqmljslexer_p.h>
 #include <QtQml/private/qqmljsparser_p.h>
+#include <private/qqmlcomponent_p.h>
 
 using namespace Qt::StringLiterals;
 
@@ -107,6 +108,7 @@ private Q_SLOTS:
     void emptyBlockBinding();
     void qualifiedName();
     void resolvedNonUniqueScopes();
+    void compilationUnitsAreCompatible();
 
 public:
     tst_qqmljsscope()
@@ -764,5 +766,56 @@ void tst_qqmljsscope::resolvedNonUniqueScopes()
     }
 }
 
+static void
+getRuntimeInfoFromCompilationUnit(const QV4::CompiledData::Unit *unit,
+                                  QList<const QV4::CompiledData::Function *> &runtimeFunctions)
+{
+    QVERIFY(unit);
+    for (uint i = 0; i < unit->functionTableSize; ++i) {
+        const QV4::CompiledData::Function *function = unit->functionAt(i);
+        QVERIFY(function);
+        runtimeFunctions << function;
+    }
+}
+
+// Note: this test is here because we never explicitly test qCompileQmlFile()
+void tst_qqmljsscope::compilationUnitsAreCompatible()
+{
+    const QString url = u"compilationUnitsCompatibility.qml"_s;
+    QList<const QV4::CompiledData::Function *> componentFunctions;
+    QList<const QV4::CompiledData::Function *> cachegenFunctions;
+
+    QQmlEngine engine;
+    QQmlComponent component(&engine);
+    component.loadUrl(testFileUrl(url));
+    QVERIFY2(component.isReady(), qPrintable(component.errorString()));
+    QScopedPointer<QObject> root(component.create());
+    QVERIFY2(root, qPrintable(component.errorString()));
+    QQmlComponentPrivate *cPriv = QQmlComponentPrivate::get(&component);
+    QVERIFY(cPriv);
+    auto unit = cPriv->compilationUnit;
+    QVERIFY(unit);
+    QVERIFY(unit->unitData());
+    getRuntimeInfoFromCompilationUnit(unit->unitData(), componentFunctions);
+
+    if (QTest::currentTestFailed())
+        return;
+
+    QmlIR::Document document(false); // we need QmlIR information here
+    QVERIFY(run(url, &document));
+    QVERIFY(document.javaScriptCompilationUnit.unitData());
+    getRuntimeInfoFromCompilationUnit(document.javaScriptCompilationUnit.unitData(),
+                                      cachegenFunctions);
+    if (QTest::currentTestFailed())
+        return;
+
+    QCOMPARE(cachegenFunctions.size(), componentFunctions.size());
+    // name index should be fairly unique to distinguish different functions
+    // within a document. their order must be the same for both qmlcachegen and
+    // qqmltypecompiler (runtime)
+    for (qsizetype i = 0; i < cachegenFunctions.size(); ++i)
+        QCOMPARE(uint(cachegenFunctions[i]->nameIndex), uint(componentFunctions[i]->nameIndex));
+}
+
 QTEST_MAIN(tst_qqmljsscope)
 #include "tst_qqmljsscope.moc"