From 6e2bba71bb8e51cecaeeacdf9c2b0f45fe1543d4 Mon Sep 17 00:00:00 2001
From: Volker Hilsheimer <volker.hilsheimer@qt.io>
Date: Tue, 2 May 2023 17:16:05 +0200
Subject: [PATCH] Pass functor through as references until stored
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Amends 642f799fc6c36c88834a7e2a10904816a962c2cb to avoid unnecessary
copies in between the calls to the QFunctorSlotObject construcotr. We
can't use a univeral reference in the QFunctorSlotObject constructor
as the call is not deduced. So provide two overloads for lvalue and
rvalue references instead.

The compile check in the test now no longer fails as we delay the
storage until one level later, but that's acceptable.

Change-Id: Ide95b4a73c70f6f47698dd1e95138aa5d48ee95d
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
---
 src/corelib/kernel/qobjectdefs_impl.h         | 22 ++++++++++---------
 .../corelib/kernel/qobject/tst_qobject.cpp    |  2 --
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/corelib/kernel/qobjectdefs_impl.h b/src/corelib/kernel/qobjectdefs_impl.h
index 8d1564c4be8..41ef92b4aa4 100644
--- a/src/corelib/kernel/qobjectdefs_impl.h
+++ b/src/corelib/kernel/qobjectdefs_impl.h
@@ -405,11 +405,12 @@ namespace QtPrivate {
     // Args and R are the List of arguments and the return type of the signal to which the slot is connected.
     template<typename Func, typename Args, typename R> class QFunctorSlotObject : public QSlotObjectBase
     {
-        using FuncType = std::conditional_t<std::is_member_function_pointer_v<std::decay_t<Func>>,
-            QtPrivate::FunctionPointer<std::decay_t<Func>>,
-            QtPrivate::Functor<Func, Args::size>
+        using FunctorValue = std::decay_t<Func>;
+        using FuncType = std::conditional_t<std::is_member_function_pointer_v<FunctorValue>,
+            QtPrivate::FunctionPointer<FunctorValue>,
+            QtPrivate::Functor<FunctorValue, Args::size>
         >;
-        Func function;
+        FunctorValue function;
         static void impl(int which, QSlotObjectBase *this_, QObject *r, void **a, bool *ret)
         {
             switch (which) {
@@ -417,14 +418,14 @@ namespace QtPrivate {
                 delete static_cast<QFunctorSlotObject*>(this_);
                 break;
             case Call:
-                if constexpr (std::is_member_function_pointer_v<std::decay_t<Func>>)
+                if constexpr (std::is_member_function_pointer_v<FunctorValue>)
                     FuncType::template call<Args, R>(static_cast<QFunctorSlotObject*>(this_)->function, static_cast<typename FuncType::Object *>(r), a);
                 else
                     FuncType::template call<Args, R>(static_cast<QFunctorSlotObject*>(this_)->function, r, a);
                 break;
             case Compare:
-                if constexpr (std::is_member_function_pointer_v<std::decay_t<Func>>) {
-                    *ret = *reinterpret_cast<std::decay_t<Func> *>(a) == static_cast<QFunctorSlotObject*>(this_)->function;
+                if constexpr (std::is_member_function_pointer_v<FunctorValue>) {
+                    *ret = *reinterpret_cast<FunctorValue *>(a) == static_cast<QFunctorSlotObject*>(this_)->function;
                     break;
                 }
                 // not implemented otherwise
@@ -434,7 +435,8 @@ namespace QtPrivate {
             }
         }
     public:
-        explicit QFunctorSlotObject(Func f) : QSlotObjectBase(&impl), function(std::move(f)) {}
+        explicit QFunctorSlotObject(Func &&f) : QSlotObjectBase(&impl), function(std::move(f)) {}
+        explicit QFunctorSlotObject(const Func &f) : QSlotObjectBase(&impl), function(f) {}
     };
 
     // typedefs for readability for when there are no parameters
@@ -489,7 +491,7 @@ namespace QtPrivate {
     template <typename Prototype, typename Functor>
     static constexpr std::enable_if_t<QtPrivate::countMatchingArguments<Prototype, Functor>() >= 0,
         QtPrivate::QSlotObjectBase *>
-    makeSlotObject(Functor func)
+    makeSlotObject(Functor &&func)
     {
         using ExpectedSignature = QtPrivate::FunctionPointer<Prototype>;
         using ExpectedReturnType = typename ExpectedSignature::ReturnType;
@@ -502,7 +504,7 @@ namespace QtPrivate {
         static_assert(int(ActualSignature::ArgumentCount) <= int(ExpectedSignature::ArgumentCount),
             "Functor requires more arguments than what can be provided.");
 
-        return new QtPrivate::QFunctorSlotObject<Functor, ActualArguments, ExpectedReturnType>(std::forward<Functor>(func));
+        return new QtPrivate::QFunctorSlotObject<std::decay_t<Functor>, ActualArguments, ExpectedReturnType>(std::forward<Functor>(func));
     }
 
     template<typename Prototype, typename Functor, typename = void>
diff --git a/tests/auto/corelib/kernel/qobject/tst_qobject.cpp b/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
index c6db1a3a978..3d353a40448 100644
--- a/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
+++ b/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
@@ -8447,7 +8447,6 @@ void tst_QObject::asyncCallbackHelper()
     static_assert(compiles<AsyncCaller::Prototype0>(&AsyncCaller::callback0));
     static_assert(compiles<AsyncCaller::Prototype0>(&AsyncCaller::staticCallback0));
     static_assert(compiles<AsyncCaller::Prototype0>(lambda0));
-    static_assert(!compiles<AsyncCaller::Prototype0>(moveOnlyLambda));
     static_assert(compiles<AsyncCaller::Prototype0>(std::move(moveOnlyLambda)));
     static_assert(compiles<AsyncCaller::Prototype0>(freeFunction0));
     static_assert(compiles<AsyncCaller::Prototype0>(functor0));
@@ -8463,7 +8462,6 @@ void tst_QObject::asyncCallbackHelper()
     static_assert(compiles<AsyncCaller::Prototype1>(&AsyncCaller::callback1));
     static_assert(compiles<AsyncCaller::Prototype1>(&AsyncCaller::staticCallback1));
     static_assert(compiles<AsyncCaller::Prototype1>(lambda1));
-    static_assert(!compiles<AsyncCaller::Prototype1>(moveOnlyLambda1));
     static_assert(compiles<AsyncCaller::Prototype1>(std::move(moveOnlyLambda1)));
     static_assert(compiles<AsyncCaller::Prototype1>(constLambda));
     static_assert(compiles<AsyncCaller::Prototype1>(freeFunction1));