From bfc56e32fdb438bdb8b4dc6be312337581ba4761 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Tue, 24 Sep 2024 11:47:38 +0200 Subject: [PATCH] Revert "QmlCompiler: Use lookupResultMetaType() also for calls" This reverts commit 1d59171d19453fb67df2b2bb70b847b6d3300574. There are a number of places where we pass invalid metatypes to the various lookup methods in place of metatypes for QObject-derived pointers. Miraculously this actually works because we can determine the type of QObject pointers via run time inspection. We cannot replace the "not initialized" type with an explicit one before all of those miracles are de-mystified. Fixes: QTBUG-129202 Change-Id: I84e4d769dc00ef6ebdfcdc9717e30c17e30e811e Reviewed-by: Jarkko Koivikko Reviewed-by: Fabian Kosmale --- src/qml/qml/qqml.cpp | 26 +++----- src/qmlcompiler/qqmljscodegenerator.cpp | 79 +++++++++++++------------ src/qmlcompiler/qqmljscodegenerator_p.h | 11 +--- 3 files changed, 53 insertions(+), 63 deletions(-) diff --git a/src/qml/qml/qqml.cpp b/src/qml/qml/qqml.cpp index bf04081336..0642cb28e3 100644 --- a/src/qml/qml/qqml.cpp +++ b/src/qml/qml/qqml.cpp @@ -974,8 +974,6 @@ void qmlRegisterTypeAndRevisions( qmlregister(TypeAndRevisionsRegistration, &type); } -struct LookupNotInitialized {}; - QObject *AOTCompiledContext::thisObject() const { return static_cast(engine->handle()->currentStackFrame) @@ -1285,8 +1283,8 @@ static ObjectPropertyResult resetFallbackProperty( static bool isTypeCompatible(QMetaType lookupType, QMetaType propertyType) { - if (lookupType == QMetaType::fromType()) { - // If lookup is not initialized, then the calling code depends on the lookup + if (!lookupType.isValid()) { + // If type is invalid, then the calling code depends on the lookup // to be set up in order to query the type, via lookupResultMetaType. // We cannot verify the type in this case. } else if ((lookupType.flags() & QMetaType::IsQmlList) @@ -1599,10 +1597,8 @@ QMetaType AOTCompiledContext::lookupResultMetaType(uint index) const = reinterpret_cast(l->qobjectFallbackLookup.metaObject - 1); const int coreIndex = l->qobjectFallbackLookup.coreIndex; return metaObject->property(coreIndex).metaType(); - } else if (l->getter == QV4::Lookup::getterQObjectMethod) { - return l->qobjectMethodLookup.propertyData->propType(); } - return QMetaType::fromType(); + return QMetaType(); } static bool isUndefined(const void *value, QMetaType type) @@ -1629,7 +1625,7 @@ void AOTCompiledContext::storeNameSloppy(uint nameIndex, void *value, QMetaType l.nameIndex = nameIndex; l.forCall = false; ObjectPropertyResult storeResult = ObjectPropertyResult::NeedsInit; - switch (initObjectLookup(this, &l, qmlScopeObject, QMetaType::fromType())) { + switch (initObjectLookup(this, &l, qmlScopeObject, QMetaType())) { case ObjectLookupResult::ObjectAsVariant: case ObjectLookupResult::Object: { const QMetaType propType = l.qobjectLookup.propertyData->propType(); @@ -1784,9 +1780,6 @@ bool AOTCompiledContext::callQmlContextPropertyLookup( return false; } - if (types[0] == QMetaType::fromType()) - return false; - function->call(qmlScopeObject, args, types, argc); return !scope.hasException(); } @@ -1794,8 +1787,8 @@ bool AOTCompiledContext::callQmlContextPropertyLookup( void AOTCompiledContext::initCallQmlContextPropertyLookup(uint index) const { Q_UNUSED(index); - if (engine->hasError()) - amendException(engine->handle()); + Q_ASSERT(engine->hasError()); + amendException(engine->handle()); } bool AOTCompiledContext::loadContextIdLookup(uint index, void *target) const @@ -1869,9 +1862,6 @@ bool AOTCompiledContext::callObjectPropertyLookup( return false; } - if (types[0] == QMetaType::fromType()) - return false; - function->call(object, args, types, argc); return !scope.hasException(); } @@ -1879,8 +1869,8 @@ bool AOTCompiledContext::callObjectPropertyLookup( void AOTCompiledContext::initCallObjectPropertyLookup(uint index) const { Q_UNUSED(index); - if (engine->hasError()) - amendException(engine->handle()); + Q_ASSERT(engine->hasError()); + amendException(engine->handle()); } bool AOTCompiledContext::callGlobalLookup( diff --git a/src/qmlcompiler/qqmljscodegenerator.cpp b/src/qmlcompiler/qqmljscodegenerator.cpp index 18c596e93d..dba9cfbd2f 100644 --- a/src/qmlcompiler/qqmljscodegenerator.cpp +++ b/src/qmlcompiler/qqmljscodegenerator.cpp @@ -1551,6 +1551,22 @@ void QQmlJSCodeGenerator::generate_StoreProperty(int nameIndex, int baseReg) reject(u"StoreProperty"_s); } +QString QQmlJSCodeGenerator::setLookupPreparation( + const QQmlJSRegisterContent &content, const QString &arg, int lookup) +{ + if (m_typeResolver->registerContains(content, content.storedType())) + return QString(); + + if (registerIsStoredIn(content, m_typeResolver->varType())) { + return u"const QMetaType argType = aotContext->lookupResultMetaType("_s + + QString::number(lookup) + u");\n"_s + + u"if (argType.isValid())\n "_s + arg + u".convert(argType)"; + } + // TODO: We could make sure they're compatible, for example QObject pointers. + return QString(); +} + + void QQmlJSCodeGenerator::generate_SetLookup(int index, int baseReg) { INJECT_TRACE_INFO(generate_SetLookup); @@ -1589,6 +1605,7 @@ void QQmlJSCodeGenerator::generate_SetLookup(int index, int baseReg) m_body += u"{\n"_s; QString variableIn; QString variableInType; + QString preparation; QString argType; if (!m_typeResolver->registerContains( m_state.accumulatorIn(), property.containedType())) { @@ -1597,7 +1614,11 @@ void QQmlJSCodeGenerator::generate_SetLookup(int index, int baseReg) + u";\n"_s; variableIn = contentPointer(property, u"converted"_s); variableInType = contentType(property, u"converted"_s); - argType = contentType(property, u"converted"_s); + preparation = setLookupPreparation(property, u"converted"_s, index); + if (preparation.isEmpty()) + argType = contentType(property, u"converted"_s); + else + argType = u"argType"_s; } else { variableIn = contentPointer(property, m_state.accumulatorVariableIn); variableInType = contentType(property, m_state.accumulatorVariableIn); @@ -1614,7 +1635,7 @@ void QQmlJSCodeGenerator::generate_SetLookup(int index, int baseReg) + u", "_s + basePointer + u", "_s + variableIn + u')'; const QString initialization = u"aotContext->initSetObjectLookup("_s + indexString + u", "_s + basePointer + u", "_s + argType + u')'; - generateLookup(lookup, initialization); + generateLookup(lookup, initialization, preparation); break; } case QQmlJSScope::AccessSemantics::Sequence: { @@ -1654,7 +1675,7 @@ void QQmlJSCodeGenerator::generate_SetLookup(int index, int baseReg) + indexString + u", "_s + metaObject(originalScope) + u", "_s + argType + u')'; - generateLookup(lookup, initialization); + generateLookup(lookup, initialization, preparation); generateWriteBack(baseReg); break; @@ -1694,8 +1715,7 @@ void QQmlJSCodeGenerator::generate_Resume(int) BYTECODE_UNIMPLEMENTED(); } -QQmlJSCodeGenerator::ArgumentsAndTypes QQmlJSCodeGenerator::argumentsList( - int argc, int argv, QString *outVar) +QString QQmlJSCodeGenerator::argumentsList(int argc, int argv, QString *outVar) { QString types; QString args; @@ -1709,6 +1729,12 @@ QQmlJSCodeGenerator::ArgumentsAndTypes QQmlJSCodeGenerator::argumentsList( *outVar = u"callResult"_s; const QQmlJSScope::ConstPtr outType = m_state.accumulatorOut().storedType(); m_body += outType->augmentedInternalName() + u' ' + *outVar; + if (!m_typeResolver->registerContains(m_state.accumulatorOut(), outType)) { + if (m_typeResolver->equals(outType, m_typeResolver->varType()) + || m_typeResolver->equals(outType, m_typeResolver->jsPrimitiveType())) { + m_body += u'(' + metaType(m_state.accumulatorOut().containedType()) + u')'; + } + } m_body += u";\n"; args = contentPointer(m_state.accumulatorOut(), *outVar); @@ -1722,7 +1748,8 @@ QQmlJSCodeGenerator::ArgumentsAndTypes QQmlJSCodeGenerator::argumentsList( types += u", "_s + contentType(content, var); } - return {args, types}; + return u"void *args[] = { "_s + args + u" };\n"_s + + u"const QMetaType types[] = { "_s + types + u" };\n"_s; } void QQmlJSCodeGenerator::generateMoveOutVar(const QString &outVar) @@ -2237,22 +2264,13 @@ void QQmlJSCodeGenerator::generate_CallPropertyLookup(int index, int base, int a m_body += u"{\n"_s; QString outVar; - const ArgumentsAndTypes argsAndTypes = argumentsList(argc, argv, &outVar); - - m_body += u"const auto doCall = [&]() {\n"_s - + u" void *args[] = {" + argsAndTypes.arguments + u"};\n"_s - + u" QMetaType types[] = {" + argsAndTypes.types + u"};\n"_s - + u" return aotContext->callObjectPropertyLookup("_s - + indexString + u", "_s + inputPointer + u", args, types, " - + QString::number(argc) + u");\n" - + u"};\n"_s; - - const QString lookup = u"doCall()"_s; + m_body += argumentsList(argc, argv, &outVar); + const QString lookup = u"aotContext->callObjectPropertyLookup("_s + indexString + + u", "_s + inputPointer + + u", args, types, "_s + QString::number(argc) + u')'; const QString initialization = u"aotContext->initCallObjectPropertyLookup("_s + indexString + u')'; - const QString preparation = getLookupPreparation(m_state.accumulatorOut(), outVar, index); - - generateLookup(lookup, initialization, preparation); + generateLookup(lookup, initialization); generateMoveOutVar(outVar); m_body += u"}\n"_s; @@ -2302,19 +2320,12 @@ void QQmlJSCodeGenerator::generate_CallQmlContextPropertyLookup(int index, int a m_body += u"{\n"_s; QString outVar; - const ArgumentsAndTypes argsAndTypes = argumentsList(argc, argv, &outVar); - m_body += u"const auto doCall = [&]() {\n"_s - + u" void *args[] = {" + argsAndTypes.arguments + u"};\n"_s - + u" QMetaType types[] = {" + argsAndTypes.types + u"};\n"_s - + u" return aotContext->callQmlContextPropertyLookup("_s - + indexString + u", args, types, " + QString::number(argc) + u");\n" - + u"};\n"_s; - - const QString lookup = u"doCall()"_s; + m_body += argumentsList(argc, argv, &outVar); + const QString lookup = u"aotContext->callQmlContextPropertyLookup("_s + indexString + + u", args, types, "_s + QString::number(argc) + u')'; const QString initialization = u"aotContext->initCallQmlContextPropertyLookup("_s + indexString + u')'; - const QString preparation = getLookupPreparation(m_state.accumulatorOut(), outVar, index); - generateLookup(lookup, initialization, preparation); + generateLookup(lookup, initialization); generateMoveOutVar(outVar); m_body += u"}\n"_s; @@ -2909,12 +2920,6 @@ QString QQmlJSCodeGenerator::getLookupPreparation( return var + u" = QVariant(aotContext->lookupResultMetaType("_s + QString::number(lookup) + u"))"_s; } - - if (registerIsStoredIn(content, m_typeResolver->jsPrimitiveType())) { - return var + u" = QJSPrimitiveValue(aotContext->lookupResultMetaType("_s - + QString::number(lookup) + u"))"_s; - } - // TODO: We could make sure they're compatible, for example QObject pointers. return QString(); } diff --git a/src/qmlcompiler/qqmljscodegenerator_p.h b/src/qmlcompiler/qqmljscodegenerator_p.h index 72b91f0c90..27bf2b1116 100644 --- a/src/qmlcompiler/qqmljscodegenerator_p.h +++ b/src/qmlcompiler/qqmljscodegenerator_p.h @@ -260,6 +260,8 @@ protected: const QString &resultPreparation = QString()); QString getLookupPreparation( const QQmlJSRegisterContent &content, const QString &var, int lookup); + QString setLookupPreparation( + const QQmlJSRegisterContent &content, const QString &arg, int lookup); void generateEnumLookup(int index); QString registerVariable(int index) const; @@ -314,14 +316,7 @@ private: QString eqIntExpression(int lhsConst); - - struct ArgumentsAndTypes - { - QString arguments; - QString types; - }; - - ArgumentsAndTypes argumentsList(int argc, int argv, QString *outVar); + QString argumentsList(int argc, int argv, QString *outVar); QString castTargetName(const QQmlJSScope::ConstPtr &type) const; bool inlineStringMethod(const QString &name, int base, int argc, int argv);