QtQml: Fix calling of method with QML_USING types

On the engine side, types made available with QML_USING are recognizable by their QMetaType conversions. In order to actually call the right methods, we need to slightly prefer methods to whose arguments we can convert over ones we have not idea about. This, however, adds the problem of converting to QString, double, QVariant and QJSValue, which is possible for any type. Since such conversions are less specific than manually added converters, we de-prioritize them a bit. On the compiler side we need to transmit the knowledge about the overload to be called from the compiler (which has already done its own overload selection) to the lookup methods. This is easily done using the relative method index. This way we do not need to do any run time overload selection at all and we do not need to pass any types to the lookup methods. We can do this without further compatibility adaptations because the current version of those lookup methods was only introduced in 6.9. Excluded, of course, are shadowable calls, which are performed with only QVariant arguments. These carry the arguments themselves and trigger the engine's overload selection. Internally nothing changes about them. Passing a list of QMetaType::fromType<QVariant>() to the lookup methods for them has always been a waste. Only the engine changes are relevant for 6.8. In 6.8, the lookup methods defer the overload selection to the engine and take the types on every call. We still cannot separate the engine changes from the compiler changes in dev because the same test is run once in interpreted and once in compiled mode. Pick-to: 6.8 Task-number: QTBUG-127174 Change-Id: I6ab52ddf3be65dcc94547266c5dcc5ac1050c93c Reviewed-by: Olivier De Cannière <olivier.decanniere@qt.io>
author: Ulf Hermann <ulf.hermann@qt.io> 2024-11-08 17:00:38 +0100
committer: Ulf Hermann <ulf.hermann@qt.io> 2024-11-13 11:58:50 +0100
commit: 12fb7d27431a7db2a19e1954c85d5dc03264b356 (patch)
tree: 5e998ab686c48c3c2d49084cb1eafd5e26c28e97
parent: 9c2fc83be55647e0519cdbd6b19701fa022743e7 (diff)
12 files changed, 214 insertions, 191 deletions
diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp
index abe7a96512..b771a938e4 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper.cpp
+++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp
@@ -1711,8 +1711,19 @@ int MatchVariant(QMetaType conversionMetaType, Retrieve &&retrieve) {
             return 1;
     }
 
-    if (QMetaType::canConvert(type, conversionMetaType))
+    if (QMetaType::canConvert(type, conversionMetaType)) {
+        if (conversionMetaType == QMetaType::fromType<QJSValue>()
+                || conversionMetaType == QMetaType::fromType<double>()
+                || conversionMetaType == QMetaType::fromType<QString>()) {
+            // Unspecific conversions receive lower score. You can convert anything
+            // to QString or double via toString() and valueOf(), respectively.
+            // And anything can be wrapped into QJSValue, but that's inefficient.
+            return 6;
+        }
+
+        // We have an explicitly defined conversion method to a non-boring type.
         return 5;
+    }
 
     return 10;
 };
@@ -1726,6 +1737,33 @@ int MatchVariant(QMetaType conversionMetaType, Retrieve &&retrieve) {
 static int MatchScore(const Value &actual, QMetaType conversionMetaType)
 {
     const int conversionType = conversionMetaType.id();
+    const auto convertibleScore = [&](QMetaType actualType) {
+        // There are a number of things we can do in JavaScript to subvert this, but
+        // if the conversion is not explicitly defined in C++, we don't want to prioritize it.
+        if (!QMetaType::canConvert(actualType, conversionMetaType))
+            return 10;
+
+        // You can convert anything to QJSValue, but that's inefficient.
+        // If we have a better option, we should use it.
+        if (conversionMetaType == QMetaType::fromType<QJSValue>())
+            return 9;
+
+        // You can also convert anything to QVariant, but that's also suboptimal.
+        // You can convert anything to string or double via toString() and valueOf().
+        // Those are also rather unspecific.
+        switch (conversionType) {
+        case QMetaType::QVariant:
+        case QMetaType::Double:
+        case QMetaType::QString:
+            return 9;
+        default:
+            break;
+        }
+
+        // We have an explicitly defined conversion method to a non-boring type.
+        return 8;
+    };
+
     if (actual.isNumber()) {
         switch (conversionType) {
         case QMetaType::Double:
@@ -1751,7 +1789,9 @@ static int MatchScore(const Value &actual, QMetaType conversionMetaType)
         case QMetaType::QJsonValue:
             return 5;
         default:
-            return 10;
+            return convertibleScore(actual.isInteger()
+                                            ? QMetaType::fromType<int>()
+                                            : QMetaType::fromType<double>());
         }
     } else if (actual.isString()) {
         switch (conversionType) {
@@ -1761,8 +1801,21 @@ static int MatchScore(const Value &actual, QMetaType conversionMetaType)
             return 5;
         case QMetaType::QUrl:
             return 6; // we like to convert strings to URLs in QML
-        default:
+        case QMetaType::Double:
+        case QMetaType::Float:
+        case QMetaType::LongLong:
+        case QMetaType::ULongLong:
+        case QMetaType::Int:
+        case QMetaType::UInt:
+        case QMetaType::Short:
+        case QMetaType::UShort:
+        case QMetaType::Char:
+        case QMetaType::UChar:
+            // QMetaType can natively convert strings to numbers.
+            // However, in the general case it's of course extremely lossy.
             return 10;
+        default:
+            return convertibleScore(QMetaType::fromType<QString>());
         }
     } else if (actual.isBoolean()) {
         switch (conversionType) {
@@ -1771,7 +1824,7 @@ static int MatchScore(const Value &actual, QMetaType conversionMetaType)
         case QMetaType::QJsonValue:
             return 5;
         default:
-            return 10;
+            return convertibleScore(QMetaType::fromType<bool>());
         }
     } else if (actual.as<DateObject>()) {
         switch (conversionType) {
@@ -1782,23 +1835,26 @@ static int MatchScore(const Value &actual, QMetaType conversionMetaType)
         case QMetaType::QTime:
             return 2;
         default:
-            return 10;
+            return convertibleScore(QMetaType::fromType<QDateTime>());
         }
     } else if (actual.as<RegExpObject>()) {
         switch (conversionType) {
 #if QT_CONFIG(regularexpression)
         case QMetaType::QRegularExpression:
             return 0;
-#endif
         default:
-            return 10;
+            return convertibleScore(QMetaType::fromType<QRegularExpression>());
+#else
+        default:
+            return convertibleScore(QMetaType());
+#endif
         }
     } else if (actual.as<ArrayBuffer>()) {
         switch (conversionType) {
         case QMetaType::QByteArray:
             return 0;
         default:
-            return 10;
+            return convertibleScore(QMetaType::fromType<QByteArray>());
         }
     } else if (actual.as<ArrayObject>()) {
         switch (conversionType) {
@@ -1813,7 +1869,7 @@ static int MatchScore(const Value &actual, QMetaType conversionMetaType)
         case QMetaType::QVector3D:
             return 7;
         default:
-            return 10;
+            return convertibleScore(QMetaType());
         }
     } else if (actual.isNull()) {
         switch (conversionType) {
@@ -1826,7 +1882,7 @@ static int MatchScore(const Value &actual, QMetaType conversionMetaType)
             if (conversionMetaType.flags().testFlag(QMetaType::IsPointer))
                 return 0;
             else
-                return 10;
+                return convertibleScore(QMetaType());
         }
         }
     } else if (const Object *obj = actual.as<Object>()) {
@@ -1849,7 +1905,8 @@ static int MatchScore(const Value &actual, QMetaType conversionMetaType)
                         return 0;
                 }
             }
-            return 10;
+
+            return convertibleScore(QMetaType::fromType<QObject *>());
         }
 
         if (const QQmlTypeWrapper *wrapper = obj->as<QQmlTypeWrapper>()) {
@@ -1871,14 +1928,15 @@ static int MatchScore(const Value &actual, QMetaType conversionMetaType)
                 }
             }
 
-            return 10;
+            return convertibleScore(QMetaType());
         }
 
         if (const Sequence *sequence = obj->as<Sequence>()) {
-            if (SequencePrototype::metaTypeForSequence(sequence) == conversionMetaType)
+            const QMetaType sequenceType = SequencePrototype::metaTypeForSequence(sequence);
+            if (sequenceType == conversionMetaType)
                 return 1;
-            else
-                return 10;
+
+            return convertibleScore(sequenceType);
         }
 
         if (const QQmlValueTypeWrapper *wrapper = obj->as<QQmlValueTypeWrapper>()) {
@@ -1889,15 +1947,20 @@ static int MatchScore(const Value &actual, QMetaType conversionMetaType)
             });
         }
 
-        if (conversionType == QMetaType::QJsonObject)
-            return 5;
-        if (conversionType == qMetaTypeId<QJSValue>())
+        if (conversionMetaType == QMetaType::fromType<QJSValue>())
             return 0;
-        if (conversionType == QMetaType::QVariantMap)
+
+        switch (conversionType) {
+        case QMetaType::QJsonObject:
+        case QMetaType::QVariantMap:
             return 5;
+        default:
+            break;
+        }
+
     }
 
-    return 10;
+    return convertibleScore(QMetaType());
 }
 
 static int numDefinedArguments(CallData *callArgs)
diff --git a/src/qml/qml/qqml.cpp b/src/qml/qml/qqml.cpp
index 26846bb68c..51a9c0816b 100644
--- a/src/qml/qml/qqml.cpp
+++ b/src/qml/qml/qqml.cpp
@@ -1915,169 +1915,38 @@ bool AOTCompiledContext::callQmlContextPropertyLookup(uint index, void **args, i
     return false;
 }
 
-enum MatchScore {
-    NoMatch           = 0x0,
-    VariantMatch      = 0x1,
-    VariantExactMatch = 0x2,
-    ExactMatch        = 0x4,
-
-    // VariantMatch and ExactMatch for different arguments are incompatible because the ExactMatch
-    // tells us that the variant was not meant as a generic argument but rather as a concrete one.
-    IncompatibleMatch = VariantMatch | ExactMatch,
-
-    // If we're calling a scope method we know that it cannot be shadowed. Therefore an all-variant
-    // method matched by an all-variant call is fine.
-    ScopeAccepted     = ExactMatch | VariantExactMatch,
-
-    // If we're calling an object method it may be shadowed. We cannot nail down an all-variant
-    // call to an all-variant method.
-    ObjectAccepted    = ExactMatch,
-
-};
-
-Q_DECLARE_FLAGS(MatchScores, MatchScore);
-
-static MatchScore overloadTypeMatch(QMetaType passed, QMetaType expected)
-{
-    const bool isVariant = (passed == QMetaType::fromType<QVariant>());
-    if (isTypeCompatible(passed, expected))
-        return isVariant ? VariantExactMatch : ExactMatch;
-    if (isVariant)
-        return VariantMatch;
-    return NoMatch;
-}
+enum MatchScore { NoMatch, VariantMatch, ExactMatch, };
 
 static MatchScore resolveQObjectMethodOverload(
-        QV4::QObjectMethod *method, QV4::Lookup *lookup, const QMetaType *types, int argc,
-        MatchScore acceptedScores)
+        QV4::QObjectMethod *method, QV4::Lookup *lookup, int relativeMethodIndex)
 {
     Q_ASSERT(lookup->qobjectMethodLookup.method.get() == method->d());
 
     const auto *d = method->d();
-    for (int i = 0, end = d->methodCount; i != end; ++i) {
-        const QMetaMethod metaMethod = d->methods[i].metaMethod();
-        if (metaMethod.parameterCount() != argc)
-            continue;
-
-        MatchScores finalScore = NoMatch;
-
-        if (!types[0].isValid()) {
-            if (argc == 0) {
-                // No arguments given and we're not interested in the return value:
-                // The overload with 0 arguments matches (but it may still be shadowable).
-                finalScore = VariantExactMatch;
-            }
-        } else {
-            const MatchScore score = overloadTypeMatch(types[0], metaMethod.returnMetaType());
-            if (score == NoMatch)
-                continue;
-            finalScore = score;
-        }
-
-        for (int j = 0; j < argc; ++j) {
-            const MatchScore score
-                    = overloadTypeMatch(types[j + 1], metaMethod.parameterMetaType(j));
-
-            if (score == NoMatch) {
-                finalScore = NoMatch;
-                break;
-            }
-
-            finalScore.setFlag(score);
-            if (finalScore.testFlags(IncompatibleMatch)) {
-                finalScore = NoMatch;
-                break;
-            }
-        }
+    const int methodCount = d->methodCount;
 
-        if (finalScore == NoMatch)
-            continue;
-
-        if (finalScore.testAnyFlags(acceptedScores)) {
-            lookup->qobjectMethodLookup.propertyData = d->methods + i;
-            return ExactMatch;
-        }
+    if (relativeMethodIndex == -1 && methodCount == 1) {
+        // QML-declared signals do not have a meaningful method index and cannot be overloaded.
+        // They still show up as QObjectMethod rather than ArrowFunction. If they didn't, we
+        // wouldn't have to care.
+        Q_ASSERT(d->methods[0].metaMethod().methodType() == QMetaMethod::Signal);
+        lookup->qobjectMethodLookup.propertyData = d->methods;
+        return ExactMatch;
     }
 
-    // No adjusting of the lookup's propertyData here. We re-fetch the method on every call.
-    // Furthermore, the first propertyData of the collection of possible overloads has the
-    // isOverridden flag we use to determine whether to invalidate a lookup. Therefore, we
-    // have to store that one if the method can be overridden (or shadowed).
-    return VariantMatch;
-}
-
-static inline bool allTypesAreVariant(const QMetaType *types, int argc)
-{
-    for (int i = 0; i <= argc; ++i) { // Yes, i <= argc, because of return type
-        if (types[i] != QMetaType::fromType<QVariant>())
-            return false;
-    }
-    return true;
-}
-
-static bool isArrowFunctionVariantCall(
-        QV4::ArrowFunction *function, const QMetaType *types, int argc)
-{
-    QV4::Function *v4Function = function->function();
-    Q_ASSERT(v4Function);
-
-    switch (v4Function->kind) {
-    case QV4::Function::AotCompiled: {
-        Q_ASSERT(argc + 1 == v4Function->aotCompiledFunction.types.size());
-        const QMetaType *parameterTypes = v4Function->aotCompiledFunction.types.data();
-
-        if (types[0].isValid() && !isTypeCompatible(types[0], parameterTypes[0])) {
-            Q_ASSERT(allTypesAreVariant(types, argc));
-            return true;
-        }
-
-        for (int i = 1; i <= argc; ++i) { // Yes, i <= argc, because of return type
-            if (!isTypeCompatible(types[i], parameterTypes[i])) {
-                Q_ASSERT(allTypesAreVariant(types, argc));
-                return true;
-            }
-        }
-
-        return false;
-    }
-    case QV4::Function::JsTyped: {
-        const auto *compiledFunction = v4Function->compiledFunction;
-        const QV4::CompiledData::Parameter *formals
-                = v4Function->compiledFunction->formalsTable();
-
-        if (types[0].isValid()
-                && !isTypeCompatible(types[0], jsTypedFunctionArgument(
-                       v4Function->jsTypedFunction.types[0], compiledFunction->returnType))) {
-            Q_ASSERT(allTypesAreVariant(types, argc));
-            return true;
-        }
-
-        for (int i = 1; i <= argc; ++i) { // Yes, i <= argc, because of return type
-            if (!isTypeCompatible(types[i], jsTypedFunctionArgument(
-                        v4Function->jsTypedFunction.types[i], formals[i - 1].type))) {
-                Q_ASSERT(allTypesAreVariant(types, argc));
-                return true;
-            }
-        }
+    for (int i = 0, end = d->methodCount; i != end; ++i) {
+        const QMetaMethod metaMethod = d->methods[i].metaMethod();
+        if (metaMethod.relativeMethodIndex() != relativeMethodIndex)
+            continue;
 
-        return false;
-    }
-    case QV4::Function::JsUntyped: {
-        // We can call untyped functions if we're not expecting a specific return value and don't
-        // have to pass any arguments. The compiler verifies this.
-        Q_ASSERT(v4Function->nFormals == 0);
-        Q_ASSERT(!types[0].isValid() || types[0] == QMetaType::fromType<QVariant>());
-        return types[0] == QMetaType::fromType<QVariant>();
-    }
-    case QV4::Function::Eval:
-        break;
+        lookup->qobjectMethodLookup.propertyData = d->methods + i;
+        return ExactMatch;
     }
 
-    Q_UNREACHABLE_RETURN(false);
+    return NoMatch;
 }
 
-void AOTCompiledContext::initCallQmlContextPropertyLookup(
-        uint index, const QMetaType *types, int argc) const
+void AOTCompiledContext::initCallQmlContextPropertyLookup(uint index, int relativeMethodIndex) const
 {
     if (engine->hasError()) {
         engine->handle()->amendException();
@@ -2092,7 +1961,7 @@ void AOTCompiledContext::initCallQmlContextPropertyLookup(
     if (auto *method = function->as<QV4::QObjectMethod>()) {
         Q_ASSERT(lookup->call == QV4::Lookup::Call::ContextGetterScopeObjectMethod);
         method->d()->ensureMethodsCache(qmlScopeObject->metaObject());
-        const auto match = resolveQObjectMethodOverload(method, lookup, types, argc, ScopeAccepted);
+        const auto match = resolveQObjectMethodOverload(method, lookup, relativeMethodIndex);
         Q_ASSERT(match == ExactMatch);
         return;
     }
@@ -2199,8 +2068,36 @@ bool AOTCompiledContext::callObjectPropertyLookup(
     return false;
 }
 
+void AOTCompiledContext::initCallObjectPropertyLookupAsVariant(uint index, QObject *object) const
+{
+    if (engine->hasError()) {
+        engine->handle()->amendException();
+        return;
+    }
+
+    QV4::Lookup *lookup = compilationUnit->runtimeLookups + index;
+    QV4::Scope scope(engine->handle());
+    QV4::ScopedValue thisObject(scope, QV4::QObjectWrapper::wrap(scope.engine, object));
+    QV4::ScopedFunctionObject function(scope, lookup->getter(scope.engine, thisObject));
+    if (auto *method = function->as<QV4::QObjectMethod>()) {
+        method->d()->ensureMethodsCache(object->metaObject());
+        lookup->asVariant = true;
+        return;
+    }
+
+    if (function->as<QV4::ArrowFunction>()) {
+        // Can't have overloads of JavaScript functions.
+        lookup->asVariant = true;
+        return;
+    }
+
+    scope.engine->throwTypeError(
+            QStringLiteral("Property '%1' of object [object Object] is not a function")
+                    .arg(compilationUnit->runtimeStrings[lookup->nameIndex]->toQString()));
+}
+
 void AOTCompiledContext::initCallObjectPropertyLookup(
-        uint index, QObject *object, const QMetaType *types, int argc) const
+        uint index, QObject *object, int relativeMethodIndex) const
 {
     if (engine->hasError()) {
         engine->handle()->amendException();
@@ -2213,15 +2110,13 @@ void AOTCompiledContext::initCallObjectPropertyLookup(
     QV4::ScopedFunctionObject function(scope, lookup->getter(scope.engine, thisObject));
     if (auto *method = function->as<QV4::QObjectMethod>()) {
         method->d()->ensureMethodsCache(object->metaObject());
-        if (resolveQObjectMethodOverload(method, lookup, types, argc, ObjectAccepted) == VariantMatch)
-            lookup->asVariant = true;
+        const auto match = resolveQObjectMethodOverload(method, lookup, relativeMethodIndex);
+        Q_ASSERT(match == ExactMatch);
         return;
     }
 
-    if (QV4::ArrowFunction *arrowFunction = function->as<QV4::ArrowFunction>()) {
+    if (function->as<QV4::ArrowFunction>()) {
         // Can't have overloads of JavaScript functions.
-        if (isArrowFunctionVariantCall(arrowFunction, types, argc))
-            lookup->asVariant = true;
         return;
     }
 
diff --git a/src/qml/qml/qqmlprivate.h b/src/qml/qml/qqmlprivate.h
index 8da2cb298c..8f512f7a93 100644
--- a/src/qml/qml/qqmlprivate.h
+++ b/src/qml/qml/qqmlprivate.h
@@ -693,7 +693,7 @@ namespace QQmlPrivate
         // the exception should be propagated. If not, the original lookup can be tried again.
 
         bool callQmlContextPropertyLookup(uint index, void **args, int argc) const;
-        void initCallQmlContextPropertyLookup(uint index, const QMetaType *types, int argc) const;
+        void initCallQmlContextPropertyLookup(uint index, int relativeMethodIndex) const;
 
 #if QT_QML_REMOVED_SINCE(6, 9)
         bool callQmlContextPropertyLookup(
@@ -706,7 +706,8 @@ namespace QQmlPrivate
 
         bool callObjectPropertyLookup(uint index, QObject *object, void **args, int argc) const;
         void initCallObjectPropertyLookup(
-                uint index, QObject *object, const QMetaType *types, int argc) const;
+                uint index, QObject *object, int relativeMethodIndex) const;
+        void initCallObjectPropertyLookupAsVariant(uint index, QObject *object) const;
 
 #if QT_QML_REMOVED_SINCE(6, 9)
         bool callObjectPropertyLookup(
diff --git a/src/qmlcompiler/qqmljscodegenerator.cpp b/src/qmlcompiler/qqmljscodegenerator.cpp
index 063140f92d..4d9c5811db 100644
--- a/src/qmlcompiler/qqmljscodegenerator.cpp
+++ b/src/qmlcompiler/qqmljscodegenerator.cpp
@@ -1738,13 +1738,11 @@ QString QQmlJSCodeGenerator::initAndCall(
         int argc, int argv, const QString &callMethodTemplate, const QString &initMethodTemplate,
         QString *outVar)
 {
-    QString types;
     QString args;
 
     if (m_state.changedRegisterIndex() == InvalidRegister ||
             m_typeResolver->registerContains(
                 m_state.accumulatorOut(), m_typeResolver->voidType())) {
-        types = u"QMetaType()"_s;
         args = u"nullptr"_s;
     } else {
         *outVar = u"callResult"_s;
@@ -1753,14 +1751,25 @@ QString QQmlJSCodeGenerator::initAndCall(
         m_body += u";\n";
 
         args = contentPointer(m_state.accumulatorOut(), *outVar);
-        types = contentType(m_state.accumulatorOut(), *outVar);
     }
 
     for (int i = 0; i < argc; ++i) {
         const QQmlJSRegisterContent content = registerType(argv + i);
         const QString var = registerVariable(argv + i);
         args += u", "_s + contentPointer(content, var);
-        types += u", "_s + contentType(content, var);
+    }
+
+    QString initMethod;
+
+    if (m_state.isShadowable()) {
+        initMethod = initMethodTemplate;
+    } else {
+        const QQmlJSMetaMethod method = m_state.accumulatorOut().methodCall();
+        Q_ASSERT(!method.isConstructor());
+
+        const QQmlJSMetaMethod::RelativeFunctionIndex relativeMethodIndex =
+                method.isJavaScriptFunction() ? method.jsFunctionIndex() : method.methodIndex();
+        initMethod = initMethodTemplate.arg(int(relativeMethodIndex));
     }
 
     return u"const auto doCall = [&]() {\n"_s
@@ -1768,8 +1777,7 @@ QString QQmlJSCodeGenerator::initAndCall(
             + u"    return aotContext->"_s + callMethodTemplate.arg(u"args"_s).arg(argc) + u";\n"
             + u"};\n"_s
             + u"const auto doInit = [&]() {\n"_s
-            + u"    QMetaType types[] = {" + types + u"};\n"_s
-            + u"    aotContext->"_s + initMethodTemplate.arg(u"types"_s).arg(argc) + u";\n"
+            + u"    aotContext->"_s + initMethod + u";\n"
             + u"};\n"_s;
 }
 
@@ -2282,10 +2290,14 @@ void QQmlJSCodeGenerator::generate_CallPropertyLookup(int index, int base, int a
 
     m_body += u"{\n"_s;
     QString outVar;
+
+    const QString initMethodTemplate = m_state.isShadowable()
+            ? u"initCallObjectPropertyLookupAsVariant(%1, %2)"_s
+            : u"initCallObjectPropertyLookup(%1, %2, %3)"_s;
+
     m_body += initAndCall(
             argc, argv, u"callObjectPropertyLookup(%1, %2, %3, %4)"_s.arg(index).arg(inputPointer),
-            u"initCallObjectPropertyLookup(%1, %2, %3, %4)"_s.arg(index).arg(inputPointer),
-            &outVar);
+            initMethodTemplate.arg(index).arg(inputPointer), &outVar);
 
     const QString lookup = u"doCall()"_s;
     const QString initialization = u"doInit()"_s;
@@ -2331,8 +2343,10 @@ void QQmlJSCodeGenerator::generate_CallQmlContextPropertyLookup(int index, int a
             return;
     }
 
-    if (m_state.accumulatorOut().isJavaScriptReturnValue())
+    if (m_state.accumulatorOut().isJavaScriptReturnValue()) {
         reject(u"call to untyped JavaScript function"_s);
+        return;
+    }
 
     AccumulatorConverter registers(this);
 
@@ -2340,7 +2354,7 @@ void QQmlJSCodeGenerator::generate_CallQmlContextPropertyLookup(int index, int a
     QString outVar;
     m_body += initAndCall(
             argc, argv, u"callQmlContextPropertyLookup(%1, %2, %3)"_s.arg(index),
-            u"initCallQmlContextPropertyLookup(%1, %2, %3)"_s.arg(index), &outVar);
+            u"initCallQmlContextPropertyLookup(%1, %2)"_s.arg(index), &outVar);
 
     const QString lookup = u"doCall()"_s;
     const QString initialization = u"doInit()"_s;
diff --git a/src/qmlcompiler/qqmljscodegenerator_p.h b/src/qmlcompiler/qqmljscodegenerator_p.h
index 0e8925c95b..5ee7e5c834 100644
--- a/src/qmlcompiler/qqmljscodegenerator_p.h
+++ b/src/qmlcompiler/qqmljscodegenerator_p.h
@@ -318,6 +318,7 @@ private:
     QString initAndCall(
             int argc, int argv, const QString &callMethodTemplate,
             const QString &initMethodTemplate, QString *outVar);
+
     QString castTargetName(const QQmlJSScope::ConstPtr &type) const;
 
     bool inlineStringMethod(const QString &name, int base, int argc, int argv);
diff --git a/src/qmlcompiler/qqmljscompilepass_p.h b/src/qmlcompiler/qqmljscompilepass_p.h
index a7447e2c23..3244251075 100644
--- a/src/qmlcompiler/qqmljscompilepass_p.h
+++ b/src/qmlcompiler/qqmljscompilepass_p.h
@@ -79,6 +79,7 @@ public:
         int changedRegisterIndex = InvalidRegister;
         bool hasSideEffects = false;
         bool isRename = false;
+        bool isShadowable = false;
     };
 
     using InstructionAnnotations = QFlatMap<int, InstructionAnnotation>;
@@ -247,6 +248,9 @@ public:
         bool isRename() const { return m_isRename; }
         void setIsRename(bool isRename) { m_isRename = isRename; }
 
+        bool isShadowable() const { return m_isShadowable; }
+        void setIsShadowable(bool isShadowable) { m_isShadowable = isShadowable; }
+
         int renameSourceRegisterIndex() const
         {
             Q_ASSERT(m_isRename);
@@ -260,6 +264,7 @@ public:
         int m_changedRegisterIndex = InvalidRegister;
         bool m_hasSideEffects = false;
         bool m_isRename = false;
+        bool m_isShadowable = false;
     };
 
     QQmlJSCompilePass(const QV4::Compiler::JSUnitGenerator *jsUnitGenerator,
@@ -363,6 +368,7 @@ protected:
         newState.markSideEffects(instruction->second.hasSideEffects);
         newState.setReadRegisters(instruction->second.readRegisters);
         newState.setIsRename(instruction->second.isRename);
+        newState.setIsShadowable(instruction->second.isShadowable);
 
         for (auto it = instruction->second.typeConversions.begin(),
              end = instruction->second.typeConversions.end(); it != end; ++it) {
diff --git a/src/qmlcompiler/qqmljsmetatypes_p.h b/src/qmlcompiler/qqmljsmetatypes_p.h
index b58aff9847..9aa4081d35 100644
--- a/src/qmlcompiler/qqmljsmetatypes_p.h
+++ b/src/qmlcompiler/qqmljsmetatypes_p.h
@@ -306,6 +306,18 @@ public:
         return m_relativeFunctionIndex;
     }
 
+    void setMethodIndex(RelativeFunctionIndex index)
+    {
+        Q_ASSERT(!m_isConstructor);
+        m_relativeFunctionIndex = index;
+    }
+
+    RelativeFunctionIndex methodIndex() const
+    {
+        Q_ASSERT(!m_isConstructor);
+        return m_relativeFunctionIndex;
+    }
+
     friend bool operator==(const QQmlJSMetaMethod &a, const QQmlJSMetaMethod &b)
     {
         return a.m_name == b.m_name && a.m_sourceLocation == b.m_sourceLocation
diff --git a/src/qmlcompiler/qqmljsshadowcheck.cpp b/src/qmlcompiler/qqmljsshadowcheck.cpp
index 469e4efa4a..0ceac934e2 100644
--- a/src/qmlcompiler/qqmljsshadowcheck.cpp
+++ b/src/qmlcompiler/qqmljsshadowcheck.cpp
@@ -190,6 +190,7 @@ QQmlJSShadowCheck::Shadowability QQmlJSShadowCheck::checkShadowing(
         const QQmlJSScope::ConstPtr varType = m_typeResolver->varType();
         const QQmlJSRegisterContent varContent = m_typeResolver->conversionType(varType);
         InstructionAnnotation &currentAnnotation = m_annotations[currentInstructionOffset()];
+        currentAnnotation.isShadowable = true;
 
         if (currentAnnotation.changedRegisterIndex != InvalidRegister) {
             m_typeResolver->adjustOriginalType(
@@ -203,6 +204,7 @@ QQmlJSShadowCheck::Shadowability QQmlJSShadowCheck::checkShadowing(
             if (it.key() != baseRegister)
                 it->second.content = m_typeResolver->convert(it->second.content, varContent);
         }
+
         return Shadowable;
     }
     default:
diff --git a/src/qmlcompiler/qqmljstypedescriptionreader.cpp b/src/qmlcompiler/qqmljstypedescriptionreader.cpp
index 6a0a207043..980a1448e4 100644
--- a/src/qmlcompiler/qqmljstypedescriptionreader.cpp
+++ b/src/qmlcompiler/qqmljstypedescriptionreader.cpp
@@ -167,6 +167,7 @@ void QQmlJSTypeDescriptionReader::readDependencies(UiScriptBinding *ast)
 void QQmlJSTypeDescriptionReader::readComponent(UiObjectDefinition *ast)
 {
     m_currentCtorIndex = 0;
+    m_currentMethodIndex = 0;
     QQmlJSScope::Ptr scope = QQmlJSScope::create();
     QList<QQmlJSScope::Export> exports;
 
@@ -354,6 +355,12 @@ void QQmlJSTypeDescriptionReader::readSignalOrMethod(
         return;
     }
 
+    // Signals, slots and method share one index space. Constructors are separate.
+    // We also assume that the order and therefore the indexing of all methods is retained from
+    // moc's JSON output.
+    if (!metaMethod.isConstructor())
+        metaMethod.setMethodIndex(QQmlJSMetaMethod::RelativeFunctionIndex(m_currentMethodIndex++));
+
     if (metaMethod.returnTypeName().isEmpty())
         metaMethod.setReturnTypeName(QLatin1String("void"));
 
diff --git a/src/qmlcompiler/qqmljstypedescriptionreader_p.h b/src/qmlcompiler/qqmljstypedescriptionreader_p.h
index 2bbae61fd6..592e9ad4c0 100644
--- a/src/qmlcompiler/qqmljstypedescriptionreader_p.h
+++ b/src/qmlcompiler/qqmljstypedescriptionreader_p.h
@@ -77,6 +77,7 @@ private:
     QList<QQmlJSExportedScope> *m_objects = nullptr;
     QStringList *m_dependencies = nullptr;
     int m_currentCtorIndex = 0;
+    int m_currentMethodIndex = 0;
 };
 
 QT_END_NAMESPACE
diff --git a/tests/auto/qml/qmlcppcodegen/data/qmlUsing.qml b/tests/auto/qml/qmlcppcodegen/data/qmlUsing.qml
index 8c7aca421e..29e66539f7 100644
--- a/tests/auto/qml/qmlcppcodegen/data/qmlUsing.qml
+++ b/tests/auto/qml/qmlcppcodegen/data/qmlUsing.qml
@@ -9,12 +9,18 @@ import TestTypes as T
 T.UsingUserObject {
     id: self
     property int valA: val.a
+    // property int valB: val.getB()
     property int myA: a
+    property int myB: getB()
     property int myA2: self.a
+    property int myB2: self.getB()
 
     function twiddle() {
         val.a = 55
+        // val.setB(56)
         a = 57
+        setB(58)
         self.a = 59
+        self.setB(60)
     }
 }
diff --git a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp
index 0ea7346746..6b6283b854 100644
--- a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp
+++ b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp
@@ -4235,24 +4235,39 @@ void tst_QmlCppCodegen::qmlUsing()
     QVERIFY(u);
 
     QCOMPARE(u->a(), 7);
+    QCOMPARE(u->getB(), 5);
     QCOMPARE(u->val().a(), 24);
+    QCOMPARE(u->val().getB(), 25);
     QCOMPARE(u->property("valA").toInt(), 24);
     QCOMPARE(u->property("myA").toInt(), 7);
+    QCOMPARE(u->property("myB").toInt(), 5);
     QCOMPARE(u->property("myA2").toInt(), 7);
+    QCOMPARE(u->property("myB2").toInt(), 5);
 
     QList<int> as;
-    QObject::connect(u, &UsingUserObject::aChanged, this, [&]() { as.append(u->a()); });
+    QList<int> bs;
+    QObject::connect(u, &UsingUserObject::aChanged, this, [&]() {
+        as.append(u->a());
+        bs.append(u->getB());
+    });
 
     QMetaObject::invokeMethod(object.data(), "twiddle");
 
-    const QList<int> expected = { 57, 59 };
-    QCOMPARE(as, expected);
+    const QList<int> expectedA = { 57, 59 };
+    QCOMPARE(as, expectedA);
+
+    const QList<int> expectedB = { 5, 58 };
+    QCOMPARE(bs, expectedB);
 
     QCOMPARE(u->a(), 59);
+    QCOMPARE(u->getB(), 60);
     QCOMPARE(u->val().a(), 55);
+    QCOMPARE(u->val().getB(), 25);
     QCOMPARE(u->property("valA").toInt(), 55);
     QCOMPARE(u->property("myA").toInt(), 59);
+    QCOMPARE(u->property("myB").toInt(), 5);  // Remains 5, due to lack of signaling
     QCOMPARE(u->property("myA2").toInt(), 59);
+    QCOMPARE(u->property("myB2").toInt(), 5); // Remains 5, due to lack of signaling
 }
 
 void tst_QmlCppCodegen::qtfont()
author	Ulf Hermann <ulf.hermann@qt.io>	2024-11-08 17:00:38 +0100
committer	Ulf Hermann <ulf.hermann@qt.io>	2024-11-13 11:58:50 +0100
commit	12fb7d27431a7db2a19e1954c85d5dc03264b356 (patch)
tree	5e998ab686c48c3c2d49084cb1eafd5e26c28e97
parent	9c2fc83be55647e0519cdbd6b19701fa022743e7 (diff)