From 22b13921f8067f8a93164875a4ad59bed85b0400 Mon Sep 17 00:00:00 2001 From: Erik Verbruggen Date: Mon, 19 Mar 2018 13:14:13 +0100 Subject: 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. Task-number: QTBUG-57043 Task-number: QTBUG-50328 Change-Id: I3008ddd847e30b7d0adef07344a326f84d85f1ba Reviewed-by: Simon Hausmann --- src/qml/compiler/qv4codegen.cpp | 8 ++++++++ src/qml/compiler/qv4compileddata_p.h | 2 +- src/qml/compiler/qv4compiler.cpp | 4 +++- src/qml/compiler/qv4compilercontext_p.h | 1 + src/qml/jsruntime/qv4function_p.h | 7 +++++++ src/qml/qml/qqmlobjectcreator.cpp | 8 ++++++++ src/qml/types/qqmlconnections.cpp | 14 +++++++++++--- 7 files changed, 39 insertions(+), 5 deletions(-) (limited to 'src/qml') diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp index bc4ca5d6f4..a262908960 100644 --- a/src/qml/compiler/qv4codegen.cpp +++ b/src/qml/compiler/qv4codegen.cpp @@ -2075,6 +2075,14 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast, _context->hasDirectEval |= (_context->compilationMode == EvalCode || _context->compilationMode == GlobalCode || _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. + _context->returnsClosure = cast(ast) && cast(cast(ast)->expression); + BytecodeGenerator bytecode(_context->line, _module->debugMode); BytecodeGenerator *savedBytecodeGenerator; savedBytecodeGenerator = bytecodeGenerator; diff --git a/src/qml/compiler/qv4compileddata_p.h b/src/qml/compiler/qv4compileddata_p.h index f1776f5772..e6c4225bbd 100644 --- a/src/qml/compiler/qv4compileddata_p.h +++ b/src/qml/compiler/qv4compileddata_p.h @@ -225,7 +225,7 @@ struct Function quint32_le localsOffset; quint32_le nLineNumbers; quint32_le lineNumberOffset; - quint32_le nInnerFunctions; + quint32_le nestedFunctionIndex; // for functions that only return a single closure, used in signal handlers quint32_le nRegisters; Location location; diff --git a/src/qml/compiler/qv4compiler.cpp b/src/qml/compiler/qv4compiler.cpp index f2e1f4a0de..ccc909c199 100644 --- a/src/qml/compiler/qv4compiler.cpp +++ b/src/qml/compiler/qv4compiler.cpp @@ -305,6 +305,9 @@ void QV4::Compiler::JSUnitGenerator::writeFunction(char *f, QV4::Compiler::Conte function->flags |= CompiledData::Function::IsStrict; if (irFunction->hasTry || irFunction->hasWith) function->flags |= CompiledData::Function::HasCatchOrWith; + function->nestedFunctionIndex = + irFunction->returnsClosure ? quint32(module->functions.indexOf(irFunction->nestedContexts.first())) + : std::numeric_limits::max(); function->nFormals = irFunction->arguments.size(); function->formalsOffset = currentOffset; currentOffset += function->nFormals * sizeof(quint32); @@ -317,7 +320,6 @@ void QV4::Compiler::JSUnitGenerator::writeFunction(char *f, QV4::Compiler::Conte function->lineNumberOffset = currentOffset; currentOffset += function->nLineNumbers * sizeof(CompiledData::CodeOffsetToLine); - function->nInnerFunctions = irFunction->nestedContexts.size(); function->nRegisters = irFunction->registerCount; diff --git a/src/qml/compiler/qv4compilercontext_p.h b/src/qml/compiler/qv4compilercontext_p.h index a78a66db52..8fabf41c40 100644 --- a/src/qml/compiler/qv4compilercontext_p.h +++ b/src/qml/compiler/qv4compilercontext_p.h @@ -144,6 +144,7 @@ struct Context { bool usesThis = false; bool hasTry = false; bool hasWith = false; + bool returnsClosure = false; mutable bool argumentsCanEscape = false; enum UsesArgumentsObject { diff --git a/src/qml/jsruntime/qv4function_p.h b/src/qml/jsruntime/qv4function_p.h index 4c8c790ca7..59a94e5dde 100644 --- a/src/qml/jsruntime/qv4function_p.h +++ b/src/qml/jsruntime/qv4function_p.h @@ -105,6 +105,13 @@ struct Q_QML_EXPORT Function { { return QQmlSourceLocation(sourceFile(), compiledFunction->location.line, compiledFunction->location.column); } + + Function *nestedFunction() const + { + if (compiledFunction->nestedFunctionIndex == std::numeric_limits::max()) + return nullptr; + return compilationUnit->runtimeFunctions[compiledFunction->nestedFunctionIndex]; + } }; } diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp index 7051fb51da..7c36f59035 100644 --- a/src/qml/qml/qqmlobjectcreator.cpp +++ b/src/qml/qml/qqmlobjectcreator.cpp @@ -879,6 +879,14 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *bindingProper if (binding->type == QV4::CompiledData::Binding::Type_Script || binding->containsTranslations()) { if (binding->flags & QV4::CompiledData::Binding::IsSignalHandlerExpression) { QV4::Function *runtimeFunction = compilationUnit->runtimeFunctions[binding->value.compiledScriptIndex]; + + // When a user writes the following: + // onSignal: function() { doSomethingUsefull } + // then do not run the binding that returns the closure, but run the closure + // instead. + if (auto closure = runtimeFunction->nestedFunction()) + runtimeFunction = closure; + int signalIndex = _propertyCache->methodIndexToSignalIndex(bindingProperty->coreIndex()); QQmlBoundSignal *bs = new QQmlBoundSignal(_bindingTarget, signalIndex, _scopeObject, engine); QQmlBoundSignalExpression *expr = new QQmlBoundSignalExpression(_bindingTarget, signalIndex, diff --git a/src/qml/types/qqmlconnections.cpp b/src/qml/types/qqmlconnections.cpp index a43562a7b8..d1a7aa9b6f 100644 --- a/src/qml/types/qqmlconnections.cpp +++ b/src/qml/types/qqmlconnections.cpp @@ -288,9 +288,17 @@ 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]) : nullptr; + auto f = d->compilationUnit->runtimeFunctions[binding->value.compiledScriptIndex]; + + // If the function is marked as having a nested function, then the user wrote: + // onSomeSignal: function() { /*....*/ } + // So take that nested function: + if (auto closure = f->nestedFunction()) + f = closure; + + QQmlBoundSignalExpression *expression = + ctxtdata ? new QQmlBoundSignalExpression(target, signalIndex, ctxtdata, this, f) + : nullptr; signal->takeExpression(expression); d->boundsignals += signal; } else { -- cgit v1.2.3