diff options
author | Simon Hausmann <simon.hausmann@qt.io> | 2018-07-25 11:40:18 +0200 |
---|---|---|
committer | Simon Hausmann <simon.hausmann@qt.io> | 2018-07-31 17:08:19 +0000 |
commit | a05d1796b88e628655afdba6063a186d3a0803bf (patch) | |
tree | f264bc7c52c1f1d62563ac3cb6444fecc537a4d7 | |
parent | c8f118d3a4ba53761d3dbc6e08b3454a2dae0a0a (diff) |
Simplify signal handler parameter handling
Unify the two QQmlBoundSignalExpression constructors and always call
updateInternalClass on the run-time function to set up the parameter
name -> argument mapping.
This streamlines the code, shares the error handling for unnamed
parameter clashes and allows getting rid of the code to extend the
formals of functions that become signal handlers in AOT generated cache
files. Either
onThatSignal: function(param1, param2) { ... }
syntax is used and the mapping is fixed and known at AOT/compile time.
Or alternatively the dynamic variant is used where the formals are
determined at signal handler installation time.
Saves a whopping KB of RAM on the QQC1 gallery.
Change-Id: I33a9afc06474143d7893f42366cb6553a07ce937
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
-rw-r--r-- | src/qml/compiler/qv4compileddata.cpp | 69 | ||||
-rw-r--r-- | src/qml/qml/qqmlboundsignal.cpp | 48 | ||||
-rw-r--r-- | src/qml/qml/qqmlboundsignal_p.h | 6 | ||||
-rw-r--r-- | tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp | 10 |
4 files changed, 23 insertions, 110 deletions
diff --git a/src/qml/compiler/qv4compileddata.cpp b/src/qml/compiler/qv4compileddata.cpp index e99c1ec842..f9216b5c4a 100644 --- a/src/qml/compiler/qv4compileddata.cpp +++ b/src/qml/compiler/qv4compileddata.cpp @@ -485,75 +485,6 @@ Unit *CompilationUnit::createUnitData(QmlIR::Document *irDocument) jsUnit->finalUrlIndex = stringTable.registerString(irDocument->jsModule.finalUrl); } - // Collect signals that have had a change in signature (from onClicked to onClicked(mouse) for example) - // and now need fixing in the QV4::CompiledData. Also register strings at the same time, to finalize - // the string table. - QVector<quint32> changedSignals; - QVector<QQmlJS::AST::FormalParameterList*> changedSignalParameters; - for (QmlIR::Object *o: qAsConst(irDocument->objects)) { - for (QmlIR::Binding *binding = o->firstBinding(); binding; binding = binding->next) { - if (!(binding->flags & QV4::CompiledData::Binding::IsSignalHandlerExpression)) - continue; - - quint32 functionIndex = binding->value.compiledScriptIndex; - QmlIR::CompiledFunctionOrExpression *foe = o->functionsAndExpressions->slowAt(functionIndex); - if (!foe) - continue; - - // save absolute index - changedSignals << o->runtimeFunctionIndices.at(functionIndex); - - Q_ASSERT(foe->node); - Q_ASSERT(QQmlJS::AST::cast<QQmlJS::AST::FunctionDeclaration*>(foe->node)); - - QQmlJS::AST::FormalParameterList *parameters = QQmlJS::AST::cast<QQmlJS::AST::FunctionDeclaration*>(foe->node)->formals; - changedSignalParameters << parameters; - - if (parameters) { - const QStringList formals = parameters->formals(); - for (const QString &arg : formals) - stringTable.registerString(arg); - } - } - } - - QVector<quint32> signalParameterNameTable; - quint32 signalParameterNameTableOffset = jsUnit->unitSize; - - // Update signal signatures - if (!changedSignals.isEmpty()) { - for (int i = 0; i < changedSignals.count(); ++i) { - const uint functionIndex = changedSignals.at(i); - // The data is now read-write due to the copy above, so the const_cast is ok. - QV4::CompiledData::Function *function = const_cast<QV4::CompiledData::Function *>(jsUnit->functionAt(functionIndex)); - Q_ASSERT(function->nFormals == quint32(0)); - - function->formalsOffset = signalParameterNameTableOffset - jsUnit->functionOffsetTable()[functionIndex]; - - if (QQmlJS::AST::FormalParameterList *parameters = changedSignalParameters.at(i)) { - const QStringList formals = parameters->formals(); - for (const QString &arg : formals) - signalParameterNameTable.append(stringTable.getStringId(arg)); - - function->nFormals = formals.size(); - } - function->length = function->nFormals; - - signalParameterNameTableOffset += function->nFormals * sizeof(quint32); - } - } - - if (!signalParameterNameTable.isEmpty()) { - Q_ASSERT(jsUnit != compilationUnit->data); - const uint signalParameterTableSize = signalParameterNameTable.count() * sizeof(quint32); - uint newSize = jsUnit->unitSize + signalParameterTableSize; - const uint oldSize = jsUnit->unitSize; - char *unitWithSignalParameters = (char*)realloc(jsUnit, newSize); - memcpy(unitWithSignalParameters + oldSize, signalParameterNameTable.constData(), signalParameterTableSize); - jsUnit = reinterpret_cast<QV4::CompiledData::Unit*>(unitWithSignalParameters); - jsUnit->unitSize = newSize; - } - return jsUnit; } diff --git a/src/qml/qml/qqmlboundsignal.cpp b/src/qml/qml/qqmlboundsignal.cpp index d5117c8cec..3d67b45fe8 100644 --- a/src/qml/qml/qqmlboundsignal.cpp +++ b/src/qml/qml/qqmlboundsignal.cpp @@ -110,46 +110,34 @@ QQmlBoundSignalExpression::QQmlBoundSignalExpression(QObject *target, int index, m_index(index), m_target(target) { - // If the function is marked as having a nested function, then the user wrote: - // onSomeSignal: function() { /*....*/ } - // So take that nested function: - if (auto closure = function->nestedFunction()) - function = closure; - - setupFunction(scope, function); + // It's important to call init first, because m_index gets remapped in case of cloned signals. init(ctxt, scopeObject); -} -QQmlBoundSignalExpression::QQmlBoundSignalExpression(QObject *target, int index, QQmlContextData *ctxt, QObject *scope, QV4::Function *runtimeFunction) - : QQmlJavaScriptExpression(), - m_index(index), - m_target(target) -{ - // It's important to call init first, because m_index gets remapped in case of cloned signals. - init(ctxt, scope); + QV4::ExecutionEngine *engine = ctxt->engine->handle(); // If the function is marked as having a nested function, then the user wrote: // onSomeSignal: function() { /*....*/ } // So take that nested function: - if (auto closure = runtimeFunction->nestedFunction()) - runtimeFunction = closure; - - QV4::ExecutionEngine *engine = ctxt->engine->handle(); - - QList<QByteArray> signalParameters = QMetaObjectPrivate::signal(m_target->metaObject(), m_index).parameterNames(); - if (!signalParameters.isEmpty()) { - QString error; - QQmlPropertyCache::signalParameterStringForJS(engine, signalParameters, &error); - if (!error.isEmpty()) { - qmlWarning(scopeObject()) << error; - return; + if (auto closure = function->nestedFunction()) { + function = closure; + } else { + QList<QByteArray> signalParameters = QMetaObjectPrivate::signal(m_target->metaObject(), m_index).parameterNames(); + if (!signalParameters.isEmpty()) { + QString error; + QQmlPropertyCache::signalParameterStringForJS(engine, signalParameters, &error); + if (!error.isEmpty()) { + qmlWarning(scopeObject) << error; + return; + } + function->updateInternalClass(engine, signalParameters); } - runtimeFunction->updateInternalClass(engine, signalParameters); } QV4::Scope valueScope(engine); - QV4::Scoped<QV4::QmlContext> qmlContext(valueScope, QV4::QmlContext::create(engine->rootContext(), ctxt, scope)); - setupFunction(qmlContext, runtimeFunction); + QV4::Scoped<QV4::QmlContext> qmlContext(valueScope, scope); + if (!qmlContext) + qmlContext = QV4::QmlContext::create(engine->rootContext(), ctxt, scopeObject); + setupFunction(qmlContext, function); } void QQmlBoundSignalExpression::init(QQmlContextData *ctxt, QObject *scope) diff --git a/src/qml/qml/qqmlboundsignal_p.h b/src/qml/qml/qqmlboundsignal_p.h index 01094a11f7..d1ec67210e 100644 --- a/src/qml/qml/qqmlboundsignal_p.h +++ b/src/qml/qml/qqmlboundsignal_p.h @@ -73,10 +73,8 @@ public: const QString ¶meterString = QString()); QQmlBoundSignalExpression(QObject *target, int index, - QQmlContextData *ctxt, QObject *scopeObject, QV4::Function *function, QV4::ExecutionContext *scope); - - QQmlBoundSignalExpression(QObject *target, int index, - QQmlContextData *ctxt, QObject *scope, QV4::Function *runtimeFunction); + QQmlContextData *ctxt, QObject *scopeObject, QV4::Function *function, + QV4::ExecutionContext *scope = nullptr); // inherited from QQmlJavaScriptExpression. QString expressionIdentifier() const override; diff --git a/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp b/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp index 71e661e507..41315fd5f0 100644 --- a/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp +++ b/tests/auto/qml/qmlcachegen/tst_qmlcachegen.cpp @@ -259,13 +259,9 @@ void tst_qmlcachegen::signalHandlerParameters() QVERIFY(compilationUnit); QVERIFY(compilationUnit->unitData()); - // The determination of the signal parameters for onTestMe by extending the - // formals of the CompiledData::Function of the signal handler implies adding new - // strings to the compilation unit. That means discarding the old string table as well as QML - // fields (to be newly written) to save memory. That means the first QML specific table - // (offsetToImports) should be the same _plus_ one entry in the newly added formals table. - const quint32 sizeOfNewFormalsTable = 1 * sizeof(quint32); - QCOMPARE(quint32(compilationUnit->unitData()->offsetToImports), oldImportsOffset + sizeOfNewFormalsTable); + // Verify that the JS unit is used unchanged, no tables were added, by checking the + // offset of the first QML specific table. + QCOMPARE(quint32(compilationUnit->unitData()->offsetToImports), oldImportsOffset); // Typically the final file name is one of those strings that is not in the original // pre-compiled qml file's string table, while for example the signal parameter |