diff --git a/src/qml/qml/qqml.cpp b/src/qml/qml/qqml.cpp index 5628f14fa3..0486247d63 100644 --- a/src/qml/qml/qqml.cpp +++ b/src/qml/qml/qqml.cpp @@ -2139,8 +2139,13 @@ static bool callQObjectMethodAsVariant( QV4::Scope scope(engine); QV4::ScopedValue wrappedObject(scope, QV4::QObjectWrapper::wrap(scope.engine, thisObject)); QV4::ScopedFunctionObject function(scope, lookup->getter(scope.engine, wrappedObject)); - Q_ASSERT(function); - Q_ASSERT(lookup->asVariant); // The getter mustn't reset the isVariant flag + + // The getter mustn't reset the isVariant flag + Q_ASSERT(lookup->asVariant); + + // Since we have an asVariant lookup, the function may have been overridden in the mean time. + if (!function) + return false; Q_ALLOCA_VAR(QMetaType, types, (argc + 1) * sizeof(QMetaType)); std::fill(types, types + argc + 1, QMetaType::fromType()); @@ -2232,31 +2237,6 @@ static bool callArrowFunction( Q_UNREACHABLE_RETURN(false); } -static bool callArrowFunctionAsVariant( - QV4::ExecutionEngine *engine, QV4::ArrowFunction *function, - QObject *thisObject, void **args, int argc) -{ - QV4::Function *v4Function = function->function(); - Q_ASSERT(v4Function); - - switch (v4Function->kind) { - case QV4::Function::JsUntyped: - // We cannot assert anything here because the method can be shadowed. - // That's why we wrap everything in QVariant. - case QV4::Function::AotCompiled: - case QV4::Function::JsTyped: { - Q_ALLOCA_VAR(QMetaType, types, (argc + 1) * sizeof(QMetaType)); - std::fill(types, types + argc + 1, QMetaType::fromType()); - function->call(thisObject, args, types, argc); - return !engine->hasException; - } - case QV4::Function::Eval: - break; - } - - Q_UNREACHABLE_RETURN(false); -} - bool AOTCompiledContext::callQmlContextPropertyLookup(uint index, void **args, int argc) const { QV4::Lookup *lookup = compilationUnit->runtimeLookups + index; @@ -2436,16 +2416,25 @@ bool AOTCompiledContext::callObjectPropertyLookup( : callQObjectMethod(engine->handle(), lookup, object, args, argc); case QV4::Lookup::Call::GetterQObjectProperty: case QV4::Lookup::Call::GetterQObjectPropertyFallback: { - const bool asVariant = lookup->asVariant; - // Here we always retrieve a fresh method via the getter. No need to re-init. + if (lookup->asVariant) { + // If the method can be shadowed, the overridden method can be taken away, too. + // In that case we might end up with a QObjectMethod or random other values instead. + // callQObjectMethodAsVariant is flexible enough to handle that. + return callQObjectMethodAsVariant(engine->handle(), lookup, object, args, argc); + } + + // Here we always retrieve a fresh ArrowFunction via the getter. QV4::Scope scope(engine->handle()); QV4::ScopedValue thisObject(scope, QV4::QObjectWrapper::wrap(scope.engine, object)); QV4::Scoped function(scope, lookup->getter(scope.engine, thisObject)); + + // The getter mustn't touch the asVariant bit + Q_ASSERT(!lookup->asVariant); + + // If the method can't be shadowed, it has to stay the same. Q_ASSERT(function); - Q_ASSERT(lookup->asVariant == asVariant); // The getter mustn't touch the asVariant bit - return asVariant - ? callArrowFunctionAsVariant(scope.engine, function, qmlScopeObject, args, argc) - : callArrowFunction(scope.engine, function, qmlScopeObject, args, argc); + + return callArrowFunction(scope.engine, function, qmlScopeObject, args, argc); } default: break; @@ -2464,16 +2453,16 @@ void AOTCompiledContext::initCallObjectPropertyLookupAsVariant(uint index, QObje QV4::Lookup *lookup = compilationUnit->runtimeLookups + index; QV4::Scope scope(engine->handle()); - const auto throwInvalidObjectError = [&]() { + const auto throwInvalidObjectError = [&](const QString &object) { scope.engine->throwTypeError( - QStringLiteral("Property '%1' of object [object Object] is not a function") - .arg(compilationUnit->runtimeStrings[lookup->nameIndex]->toQString())); + QStringLiteral("Property '%1' of object %2 is not a function").arg( + compilationUnit->runtimeStrings[lookup->nameIndex]->toQString(), object)); }; const auto *ddata = QQmlData::get(object, false); if (ddata && ddata->hasVMEMetaObject && ddata->jsWrapper.isNullOrUndefined()) { // We cannot lookup functions on an object with VME metaobject but no QObjectWrapper - throwInvalidObjectError(); + throwInvalidObjectError(QStringLiteral("[object Object]")); return; } @@ -2491,7 +2480,7 @@ void AOTCompiledContext::initCallObjectPropertyLookupAsVariant(uint index, QObje return; } - throwInvalidObjectError(); + throwInvalidObjectError(thisObject->toQStringNoThrow()); } void AOTCompiledContext::initCallObjectPropertyLookup( diff --git a/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt b/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt index efd95ce64f..29b272c440 100644 --- a/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt +++ b/tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt @@ -151,6 +151,7 @@ set(qml_files detachedreferences.qml dialog.qml dialogButtonBox.qml + disappearingArrowFunction.qml dynamicscene.qml enforceSignature.qml enumConversion.qml diff --git a/tests/auto/qml/qmlcppcodegen/data/disappearingArrowFunction.qml b/tests/auto/qml/qmlcppcodegen/data/disappearingArrowFunction.qml new file mode 100644 index 0000000000..cb97ab5c02 --- /dev/null +++ b/tests/auto/qml/qmlcppcodegen/data/disappearingArrowFunction.qml @@ -0,0 +1,28 @@ +pragma Strict +import QtQml + +QtObject { + property Person inner: Person { + function getName() : int { return 5 } + } + + property Person none: Person {} + + property Person evil: Person { + property string getName: "not a function" + } + + onObjectNameChanged: console.log(inner.getName()) + + function swapNone() { + let t = inner; + inner = none; + none = t; + } + + function swapEvil() { + let t = inner; + inner = evil; + evil = t; + } +} diff --git a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp index 579f279e93..5780862b47 100644 --- a/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp +++ b/tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp @@ -103,6 +103,7 @@ private slots: void detachOnAssignment(); void detachedReferences(); void dialogButtonBox(); + void disappearingArrowFunction(); void enumConversion(); void enumFromBadSingleton(); void enumLookup(); @@ -1866,6 +1867,56 @@ void tst_QmlCppCodegen::dialogButtonBox() QPlatformDialogHelper::Ok | QPlatformDialogHelper::Cancel); } +void tst_QmlCppCodegen::disappearingArrowFunction() +{ + QQmlEngine engine; + const QUrl url(u"qrc:/qt/qml/TestTypes/disappearingArrowFunction.qml"_s); + QQmlComponent c(&engine, url); + QVERIFY2(c.isReady(), qPrintable(c.errorString())); + QScopedPointer o(c.create()); + QVERIFY(!o.isNull()); + + QTest::ignoreMessage(QtDebugMsg, "5"); + o->setObjectName("no"); + + QMetaObject::invokeMethod(o.data(), "swapNone"); + QTest::ignoreMessage(QtDebugMsg, "Bart"); + o->setObjectName("nono"); + + QMetaObject::invokeMethod(o.data(), "swapNone"); + QTest::ignoreMessage(QtDebugMsg, "5"); + o->setObjectName("nonono"); + + const QRegularExpression warning( + QRegularExpression::escape(url.toString()) + + u"\\:15\\: TypeError\\: Property 'getName' of object " + "Person_QML_[0-9]+\\(0x[0-9a-f]+\\) is not a function"_s); + + QMetaObject::invokeMethod(o.data(), "swapEvil"); + QTest::ignoreMessage(QtWarningMsg, warning); + o->setObjectName("nononono"); + + QMetaObject::invokeMethod(o.data(), "swapEvil"); + QTest::ignoreMessage(QtDebugMsg, "5"); + o->setObjectName("nonononono"); + + QMetaObject::invokeMethod(o.data(), "swapNone"); + QTest::ignoreMessage(QtDebugMsg, "Bart"); + o->setObjectName("nononononono"); + + QMetaObject::invokeMethod(o.data(), "swapEvil"); + QTest::ignoreMessage(QtWarningMsg, warning); + o->setObjectName("nonononononono"); + + QMetaObject::invokeMethod(o.data(), "swapEvil"); + QTest::ignoreMessage(QtDebugMsg, "Bart"); + o->setObjectName("nononononononono"); + + QMetaObject::invokeMethod(o.data(), "swapNone"); + QTest::ignoreMessage(QtDebugMsg, "5"); + o->setObjectName("nonononononononono"); +} + void tst_QmlCppCodegen::enumConversion() { QQmlEngine engine;