diff options
author | Erik Verbruggen <erik.verbruggen@qt.io> | 2018-03-19 13:14:13 +0100 |
---|---|---|
committer | Simon Hausmann <simon.hausmann@qt.io> | 2018-03-28 12:26:48 +0000 |
commit | eb4f43a3f69c550213ed0b33cd35786a9a7cbc9f (patch) | |
tree | 0292e66b7cde966c9687cdfce62643e81748e3e4 | |
parent | 59cc1c7d58a0b20b2c079eefd057b93197d84b32 (diff) |
Handle function expressions as signal handlers
There are two ways to use function expressions on the right-hand side
of bindings:
property var somethingPressed
somethingPressed: function() { /* ..press something else.. */ }
signal buttonPressed
onButtonPressed: function() { /* ..handle buttonPress.. */ }
In the former case, it declares a property that holds a function. So on
initialization, the right-hand side of the binding returns a closure
that gets assigned to the property 'somethingPressed'.
In the latter case, the signal handler is explicitly marked as a
function for clarity. So, the handler should not be returning the
closure, but the handler should *be* the closure.
In general, it is not possible to detect if the left-hand side is a
property or a signal handler when generating QML cache files ahead of
time. So for this case, we mark the function as only returning a
closure. Then when instantiating the object, we check if it is a signal
handler, and if the handler is marked as only returning a closure. If
so, we set that closure to be the signal handler.
This patch also handles function expressions in SignalTransition and
function expressions as signal handlers.
Task-number: QTBUG-57043
Task-number: QTBUG-50328
Task-number: QTBUG-50328
(cherry picked from commit 22b13921f8067f8a93164875a4ad59bed85b0400)
(cherry picked from commit dc4d6293f9473c0f03c570430d08867d2d01c6e2)
(cherry picked from commit 21301c1dbb00f4a2cd991e520423ed039b297ffb)
Change-Id: I3008ddd847e30b7d0adef07344a326f84d85f1ba
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>
-rw-r--r-- | src/imports/statemachine/signaltransition.cpp | 7 | ||||
-rw-r--r-- | src/qml/compiler/qv4codegen.cpp | 9 | ||||
-rw-r--r-- | src/qml/compiler/qv4compileddata_p.h | 4 | ||||
-rw-r--r-- | src/qml/compiler/qv4compiler.cpp | 3 | ||||
-rw-r--r-- | src/qml/compiler/qv4jsir.cpp | 1 | ||||
-rw-r--r-- | src/qml/compiler/qv4jsir_p.h | 3 | ||||
-rw-r--r-- | src/qml/jsruntime/qv4function_p.h | 7 | ||||
-rw-r--r-- | src/qml/qml/qqmlboundsignal.cpp | 12 | ||||
-rw-r--r-- | src/qml/types/qqmlconnections.cpp | 7 | ||||
-rw-r--r-- | tests/auto/qmltest/statemachine/tst_triggeredArguments2.qml | 1 |
10 files changed, 45 insertions, 9 deletions
diff --git a/src/imports/statemachine/signaltransition.cpp b/src/imports/statemachine/signaltransition.cpp index 0f88ec641b..7057fd0621 100644 --- a/src/imports/statemachine/signaltransition.cpp +++ b/src/imports/statemachine/signaltransition.cpp @@ -176,9 +176,10 @@ void SignalTransition::connectTriggered() QMetaMethod metaMethod = target->metaObject()->method(qobjectSignal->methodIndex()); int signalIndex = QMetaObjectPrivate::signalIndex(metaMethod); - QQmlBoundSignalExpression *expression = ctxtdata ? - new QQmlBoundSignalExpression(target, signalIndex, - ctxtdata, this, m_compilationUnit->runtimeFunctions[binding->value.compiledScriptIndex]) : 0; + auto f = m_compilationUnit->runtimeFunctions[binding->value.compiledScriptIndex]; + QQmlBoundSignalExpression *expression = + ctxtdata ? new QQmlBoundSignalExpression(target, signalIndex, ctxtdata, this, f) + : nullptr; if (expression) expression->setNotifyOnValueChanged(false); m_signalExpression = expression; diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp index b07e9f55f5..b609a06382 100644 --- a/src/qml/compiler/qv4codegen.cpp +++ b/src/qml/compiler/qv4codegen.cpp @@ -2102,6 +2102,15 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast, IR::BasicBlock *exitBlock = function->newBasicBlock(0, IR::Function::DontInsertBlock); function->hasDirectEval = _env->hasDirectEval || _env->compilationMode == EvalCode || _module->debugMode; // Conditional breakpoints are like eval in the function + + // When a user writes the following QML signal binding: + // onSignal: function() { doSomethingUsefull } + // we will generate a binding function that just returns the closure. However, that's not useful + // at all, because if the onSignal is a signal handler, the user is actually making it explicit + // that the binding is a function, so we should execute that. However, we don't know that during + // AOT compilation, so mark the surrounding function as only-returning-a-closure. + function->returnsClosure = cast<ExpressionStatement *>(ast) && cast<FunctionExpression *>(cast<ExpressionStatement *>(ast)->expression); + function->usesArgumentsObject = _env->parent && (_env->usesArgumentsObject == Environment::ArgumentsObjectUsed); function->usesThis = _env->usesThis; function->maxNumberOfArguments = qMax(_env->maxNumberOfArguments, (int)QV4::Global::ReservedArgumentCount); diff --git a/src/qml/compiler/qv4compileddata_p.h b/src/qml/compiler/qv4compileddata_p.h index 188a571394..b15e4ade60 100644 --- a/src/qml/compiler/qv4compileddata_p.h +++ b/src/qml/compiler/qv4compileddata_p.h @@ -228,6 +228,7 @@ struct Function LEUInt32 nLocals; LEUInt32 localsOffset; LEUInt32 nInnerFunctions; + LEUInt32 nestedFunctionIndex; // for functions that only return a single closure, used in signal handlers Location location; // Qml Extensions Begin @@ -248,6 +249,7 @@ struct Function quint8 flags; quint8 padding1; LEUInt16 padding2; + LEUInt32 padding3; const LEUInt32 *formalsTable() const { return reinterpret_cast<const LEUInt32 *>(reinterpret_cast<const char *>(this) + formalsOffset); } const LEUInt32 *localsTable() const { return reinterpret_cast<const LEUInt32 *>(reinterpret_cast<const char *>(this) + localsOffset); } @@ -266,7 +268,7 @@ struct Function return (sizeof(Function) + (nFormals + nLocals + nInnerfunctions + nIdObjectDependencies + 2 * nPropertyDependencies) * sizeof(quint32) + 7) & ~0x7; } }; -static_assert(sizeof(Function) == 72, "Function structure needs to have the expected size to be binary compatible on disk when generated by host compiler and loaded by target"); +static_assert(sizeof(Function) == 80, "Function structure needs to have the expected size to be binary compatible on disk when generated by host compiler and loaded by target"); // Qml data structures diff --git a/src/qml/compiler/qv4compiler.cpp b/src/qml/compiler/qv4compiler.cpp index 0dc40d9698..e11e0bbcdd 100644 --- a/src/qml/compiler/qv4compiler.cpp +++ b/src/qml/compiler/qv4compiler.cpp @@ -298,6 +298,9 @@ void QV4::Compiler::JSUnitGenerator::writeFunction(char *f, QV4::IR::Function *i if (irFunction->hasTry || irFunction->hasWith) function->flags |= CompiledData::Function::HasCatchOrWith; function->nFormals = irFunction->formals.size(); + function->nestedFunctionIndex = + irFunction->returnsClosure ? quint32(irModule->functions.indexOf(irFunction->nestedFunctions.first())) + : std::numeric_limits<uint32_t>::max(); function->formalsOffset = currentOffset; currentOffset += function->nFormals * sizeof(quint32); diff --git a/src/qml/compiler/qv4jsir.cpp b/src/qml/compiler/qv4jsir.cpp index a4038f827a..7741a64667 100644 --- a/src/qml/compiler/qv4jsir.cpp +++ b/src/qml/compiler/qv4jsir.cpp @@ -374,6 +374,7 @@ Function::Function(Module *module, Function *outer, const QString &name) , hasTry(false) , hasWith(false) , isQmlBinding(false) + , returnsClosure(false) , unused(0) , line(0) , column(0) diff --git a/src/qml/compiler/qv4jsir_p.h b/src/qml/compiler/qv4jsir_p.h index 317abd09bb..35b6abaa58 100644 --- a/src/qml/compiler/qv4jsir_p.h +++ b/src/qml/compiler/qv4jsir_p.h @@ -1313,7 +1313,8 @@ struct Function { uint hasTry: 1; uint hasWith: 1; uint isQmlBinding: 1; - uint unused : 24; + uint returnsClosure: 1; + uint unused : 23; // Location of declaration in source code (0 if not specified) uint line; diff --git a/src/qml/jsruntime/qv4function_p.h b/src/qml/jsruntime/qv4function_p.h index b212b3d027..fb8f1400a2 100644 --- a/src/qml/jsruntime/qv4function_p.h +++ b/src/qml/jsruntime/qv4function_p.h @@ -50,6 +50,7 @@ // We mean it. // +#include <stdint.h> #include "qv4global_p.h" #include <private/qqmlglobal_p.h> #include <private/qv4compileddata_p.h> @@ -100,6 +101,12 @@ struct Q_QML_EXPORT Function { return QQmlSourceLocation(sourceFile(), compiledFunction->location.line, compiledFunction->location.column); } + Function *nestedFunction() const + { + if (compiledFunction->nestedFunctionIndex == std::numeric_limits<uint32_t>::max()) + return nullptr; + return compilationUnit->runtimeFunctions[compiledFunction->nestedFunctionIndex]; + } }; diff --git a/src/qml/qml/qqmlboundsignal.cpp b/src/qml/qml/qqmlboundsignal.cpp index 19ece44beb..efe0cccc8d 100644 --- a/src/qml/qml/qqmlboundsignal.cpp +++ b/src/qml/qml/qqmlboundsignal.cpp @@ -110,6 +110,12 @@ 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); init(ctxt, scopeObject); } @@ -122,6 +128,12 @@ QQmlBoundSignalExpression::QQmlBoundSignalExpression(QObject *target, int index, // It's important to call init first, because m_index gets remapped in case of cloned signals. init(ctxt, scope); + // 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 = QQmlEnginePrivate::getV4Engine(ctxt->engine); QList<QByteArray> signalParameters = QMetaObjectPrivate::signal(m_target->metaObject(), m_index).parameterNames(); diff --git a/src/qml/types/qqmlconnections.cpp b/src/qml/types/qqmlconnections.cpp index 7607c19374..2554dcf2b6 100644 --- a/src/qml/types/qqmlconnections.cpp +++ b/src/qml/types/qqmlconnections.cpp @@ -289,9 +289,10 @@ void QQmlConnections::connectSignals() new QQmlBoundSignal(target, signalIndex, this, qmlEngine(this)); signal->setEnabled(d->enabled); - QQmlBoundSignalExpression *expression = ctxtdata ? - new QQmlBoundSignalExpression(target, signalIndex, - ctxtdata, this, d->compilationUnit->runtimeFunctions[binding->value.compiledScriptIndex]) : 0; + auto f = d->compilationUnit->runtimeFunctions[binding->value.compiledScriptIndex]; + QQmlBoundSignalExpression *expression = + ctxtdata ? new QQmlBoundSignalExpression(target, signalIndex, ctxtdata, this, f) + : nullptr; signal->takeExpression(expression); d->boundsignals += signal; } else { diff --git a/tests/auto/qmltest/statemachine/tst_triggeredArguments2.qml b/tests/auto/qmltest/statemachine/tst_triggeredArguments2.qml index 73bc653404..f23cc14152 100644 --- a/tests/auto/qmltest/statemachine/tst_triggeredArguments2.qml +++ b/tests/auto/qmltest/statemachine/tst_triggeredArguments2.qml @@ -67,7 +67,6 @@ TestCase { // Emit the signalTrans.signal testCase.mysignal("test1", true, 2) - expectFail("", "QTBUG-50328") compare(testCase.mystr, "test1") compare(testCase.mybool, true) compare(testCase.myint, 2) |