Do QMetaType-style call in QQmlPropertyBinding::evaluate

We already have a void* and metatype available. There is no need to
convert, unless we have bound arguments. The call() itself will convert
as necessary anyway.

However, we do need to figure out whether the returned value was
undefined. Pass this information up from the actual call.

This reverts commit 8ac705247430ff6fbbc25a9db20c0e7dc572abe7. The
original commit 3a4e013f0058952c94ed3414aafbf96216efff8d was correct. We
were just missing the value type conversions in metaTypeFromJS().

Change-Id: Ic4b2ebf1eb3fb2e5a50a045be774dd02d0fed7c6
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

5 files changed, 82 insertions, 30 deletions

diff --git a/src/qml/jsruntime/qv4function.cpp b/src/qml/jsruntime/qv4function.cpp
index 348837c3a5..f6001da1f8 100644
--- a/src/qml/jsruntime/qv4function.cpp
+++ b/src/qml/jsruntime/qv4function.cpp
@@ -57,15 +57,15 @@ QT_BEGIN_NAMESPACE
 
 using namespace QV4;
 
-void Function::call(const Value *thisObject, void **a, const QMetaType *types, int argc,
+bool Function::call(const Value *thisObject, void **a, const QMetaType *types, int argc,
                     const ExecutionContext *context)
 {
     if (!aotFunction) {
-        QV4::convertAndCall(context->engine(), thisObject, a, types, argc,
-                            [this, context](const Value *thisObject, const Value *argv, int argc) {
+        return QV4::convertAndCall(
+                    context->engine(), thisObject, a, types, argc,
+                    [this, context](const Value *thisObject, const Value *argv, int argc) {
             return call(thisObject, argv, argc, context);
         });
-        return;
     }
 
     ExecutionEngine *engine = context->engine();
@@ -77,6 +77,7 @@ void Function::call(const Value *thisObject, void **a, const QMetaType *types, i
     engine->jsStackTop += frame.requiredJSStackFrameSize();
     Moth::VME::exec(&frame, engine);
     frame.pop(engine);
+    return true;
 }
 
 ReturnedValue Function::call(const Value *thisObject, const Value *argv, int argc, const ExecutionContext *context) {
diff --git a/src/qml/jsruntime/qv4function_p.h b/src/qml/jsruntime/qv4function_p.h
index a15344dacd..7577161c01 100644
--- a/src/qml/jsruntime/qv4function_p.h
+++ b/src/qml/jsruntime/qv4function_p.h
@@ -100,7 +100,7 @@ public:
         return compilationUnit->runtimeStrings[i];
     }
 
-    void call(const Value *thisObject, void **a, const QMetaType *types, int argc,
+    bool call(const Value *thisObject, void **a, const QMetaType *types, int argc,
               const ExecutionContext *context);
     ReturnedValue call(const Value *thisObject, const Value *argv, int argc, const ExecutionContext *context);
 
diff --git a/src/qml/jsruntime/qv4jscall_p.h b/src/qml/jsruntime/qv4jscall_p.h
index 3f0c8ee06d..c6d320ac20 100644
--- a/src/qml/jsruntime/qv4jscall_p.h
+++ b/src/qml/jsruntime/qv4jscall_p.h
@@ -206,7 +206,7 @@ ReturnedValue convertAndCall(
 }
 
 template<typename Callable>
-void convertAndCall(ExecutionEngine *engine, const Value *thisObject,
+bool convertAndCall(ExecutionEngine *engine, const Value *thisObject,
                     void **a, const QMetaType *types, int argc, Callable call)
 {
     Scope scope(engine);
@@ -215,13 +215,10 @@ void convertAndCall(ExecutionEngine *engine, const Value *thisObject,
     for (int ii = 0; ii < argc; ++ii)
         jsCallData.args[ii] = engine->metaTypeToJS(types[ii + 1], a[ii + 1]);
 
-    void *result = a[0];
-    if (!result) {
-        call(thisObject, jsCallData.args, argc);
-        return;
-    }
-
     ScopedValue jsResult(scope, call(thisObject, jsCallData.args, argc));
+    void *result = a[0];
+    if (!result)
+        return !jsResult->isUndefined();
 
     const QMetaType resultType = types[0];
     if (scope.hasException()) {
@@ -237,6 +234,7 @@ void convertAndCall(ExecutionEngine *engine, const Value *thisObject,
             scope.engine->metaTypeFromJS(jsResult, resultType, result);
         }
     }
+    return !jsResult->isUndefined();
 }
 
 }
diff --git a/src/qml/qml/qqmljavascriptexpression.cpp b/src/qml/qml/qqmljavascriptexpression.cpp
index 95d7d89c99..95db27019c 100644
--- a/src/qml/qml/qqmljavascriptexpression.cpp
+++ b/src/qml/qml/qqmljavascriptexpression.cpp
@@ -303,10 +303,10 @@ bool QQmlJavaScriptExpression::evaluate(void **a, const QMetaType *types, int ar
 
     Q_ASSERT(m_qmlScope.valueRef());
     Q_ASSERT(function());
-    function()->call(self, a, types, argc,
-                     static_cast<QV4::ExecutionContext *>(m_qmlScope.valueRef()));
+    const bool isUndefined = !function()->call(
+                self, a, types, argc, static_cast<QV4::ExecutionContext *>(m_qmlScope.valueRef()));
 
-    return !capture.catchException(scope);
+    return !capture.catchException(scope) && !isUndefined;
 }
 
 void QQmlPropertyCapture::captureProperty(QQmlNotifier *n)
diff --git a/src/qml/qml/qqmlpropertybinding.cpp b/src/qml/qml/qqmlpropertybinding.cpp
index 647b0f7b59..7729198146 100644
--- a/src/qml/qml/qqmlpropertybinding.cpp
+++ b/src/qml/qml/qqmlpropertybinding.cpp
@@ -146,6 +146,15 @@ QQmlPropertyBinding::QQmlPropertyBinding(QMetaType mt, QObject *target, QQmlProp
     errorCallBack = bindingErrorCallback;
 }
 
+template<typename T>
+bool compareAndAssign(void *dataPtr, const void *result)
+{
+    if (*static_cast<const T *>(result) == *static_cast<const T *>(dataPtr))
+        return false;
+    *static_cast<T *>(dataPtr) = *static_cast<const T *>(result);
+    return true;
+}
+
 bool QQmlPropertyBinding::evaluate(QMetaType metaType, void *dataPtr)
 {
     const auto ctxt = jsExpression()->context();
@@ -159,17 +168,16 @@ bool QQmlPropertyBinding::evaluate(QMetaType metaType, void *dataPtr)
     QQmlEnginePrivate *ep = QQmlEnginePrivate::get(engine);
     ep->referenceScarceResources();
 
-    bool evaluatedToUndefined = false;
-
-    QV4::Scope scope(engine->handle());
-    QV4::ScopedValue result(scope, hasBoundFunction()
-                            ? static_cast<QQmlPropertyBindingJSForBoundFunction *>(jsExpression())->evaluate(&evaluatedToUndefined)
-                            : jsExpression()->evaluate(&evaluatedToUndefined));
-
-    ep->dereferenceScarceResources();
+    const auto handleErrorAndUndefined = [&](bool evaluatedToUndefined) {
+        ep->dereferenceScarceResources();
+        if (jsExpression()->hasError()) {
+            QPropertyBindingError error(QPropertyBindingError::UnknownError,
+                                        jsExpression()->delayedError()->error().description());
+            QPropertyBindingPrivate::currentlyEvaluatingBinding()->setError(std::move(error));
+            bindingErrorCallback(this);
+            return false;
+        }
 
-    bool hadError = jsExpression()->hasError();
-    if (!hadError) {
         if (evaluatedToUndefined) {
             handleUndefinedAssignment(ep, dataPtr);
             // if property has been changed due to reset, reset is responsible for
@@ -178,13 +186,58 @@ bool QQmlPropertyBinding::evaluate(QMetaType metaType, void *dataPtr)
         } else if (isUndefined()) {
             setIsUndefined(false);
         }
-    } else {
-        QPropertyBindingError error(QPropertyBindingError::UnknownError, jsExpression()->delayedError()->error().description());
-        QPropertyBindingPrivate::currentlyEvaluatingBinding()->setError(std::move(error));
-        bindingErrorCallback(this);
-        return false;
+
+        return true;
+    };
+
+    if (!hasBoundFunction()) {
+        Q_ASSERT(metaType.sizeOf() > 0);
+
+        // No need to construct here. evaluate() expects uninitialized memory.
+        Q_ALLOCA_VAR(void, result, metaType.sizeOf());
+
+        const bool evaluatedToUndefined = !jsExpression()->evaluate(&result, &metaType, 0);
+        if (!handleErrorAndUndefined(evaluatedToUndefined))
+            return false;
+
+        if (metaType.flags() & QMetaType::PointerToQObject)
+            return compareAndAssign<QObject *>(dataPtr, result);
+
+        switch (metaType.id()) {
+        case QMetaType::Bool:
+            return compareAndAssign<bool>(dataPtr, result);
+        case QMetaType::Int:
+            return compareAndAssign<int>(dataPtr, result);
+        case QMetaType::Double:
+            return compareAndAssign<double>(dataPtr, result);
+        case QMetaType::Float:
+            return compareAndAssign<float>(dataPtr, result);
+        case QMetaType::QString: {
+            const bool hasChanged = compareAndAssign<QString>(dataPtr, result);
+            static_cast<QString *>(result)->~QString();
+            return hasChanged;
+        }
+        default:
+            break;
+        }
+
+        const bool hasChanged = !metaType.equals(result, dataPtr);
+        if (hasChanged) {
+            metaType.destruct(dataPtr);
+            metaType.construct(dataPtr, result);
+        }
+        metaType.destruct(result);
+        return hasChanged;
     }
 
+    bool evaluatedToUndefined = false;
+    QV4::Scope scope(engine->handle());
+    QV4::ScopedValue result(scope, static_cast<QQmlPropertyBindingJSForBoundFunction *>(
+                                jsExpression())->evaluate(&evaluatedToUndefined));
+
+    if (!handleErrorAndUndefined(evaluatedToUndefined))
+        return false;
+
     int propertyType = metaType.id();
 
     switch (propertyType) {