Simplify the creation of APIs that take a callback

Functions in Qt that take a callback need to support callables with or
without context objects, and member functions of an object. The
implementation of those overloads follows a pattern that ultimately
results in a QSlotObjectBase implementation being created and
passed to an implementation helper that takes care of the logic.

Factor that common pattern into a new helper template in QtPrivate
that returns a suitable QSlotObjectBase after checking that the
functor is compatible with the specified argument types.

Use that new helper template in the implementation of
QCoreApplication::requestPermission and QHostInfo::lookupHost.

The only disadvantage of centralizing this logic is that we cannot print
a more detailed error message indicating which argument types the
caller expects. However, that information is visible from the detailed
compiler errors anyway.

Change-Id: I24cf0b2442217857b96ffc4d2d6c997c4fae34e0
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

4 files changed, 126 insertions, 88 deletions

diff --git a/src/corelib/kernel/qcoreapplication.h b/src/corelib/kernel/qcoreapplication.h
index 82580ceb34..5a5ddf4837 100644
--- a/src/corelib/kernel/qcoreapplication.h
+++ b/src/corelib/kernel/qcoreapplication.h
@@ -116,66 +116,34 @@ public:
 
 # ifdef Q_QDOC
     template <typename Functor>
-    void requestPermission(const QPermission &permission, Functor functor);
-    template <typename Functor>
     void requestPermission(const QPermission &permission, const QObject *context, Functor functor);
 # else
-    template <typename Slot> // requestPermission to a QObject slot
+    // requestPermission with context or receiver object; need to require here that receiver is the
+    // right type to avoid ambiguity with the private implementation function.
+    template <typename Functor>
     void requestPermission(const QPermission &permission,
-        const typename QtPrivate::FunctionPointer<Slot>::Object *receiver, Slot slot)
-    {
-        using CallbackSignature = QtPrivate::FunctionPointer<void (*)(QPermission)>;
-        using SlotSignature = QtPrivate::FunctionPointer<Slot>;
-
-        static_assert(int(SlotSignature::ArgumentCount) <= int(CallbackSignature::ArgumentCount),
-            "Slot requires more arguments than what can be provided.");
-        static_assert((QtPrivate::CheckCompatibleArguments<typename CallbackSignature::Arguments, typename SlotSignature::Arguments>::value),
-            "Slot arguments are not compatible (must be QPermission)");
-
-        auto slotObj = new QtPrivate::QSlotObject<Slot, typename SlotSignature::Arguments, void>(slot);
-        requestPermission(permission, slotObj, receiver);
-    }
-
-    // requestPermission to a functor or function pointer (with context)
-    template <typename Func, std::enable_if_t<
-        !QtPrivate::FunctionPointer<Func>::IsPointerToMemberFunction
-        && !std::is_same<const char *, Func>::value, bool> = true>
-    void requestPermission(const QPermission &permission, const QObject *context, Func func)
+                           const typename QtPrivate::ContextTypeForFunctor<Functor>::ContextType *receiver,
+                           Functor func)
     {
-        using CallbackSignature = QtPrivate::FunctionPointer<void (*)(QPermission)>;
-        constexpr int MatchingArgumentCount = QtPrivate::ComputeFunctorArgumentCount<
-            Func, CallbackSignature::Arguments>::Value;
-
-        static_assert(MatchingArgumentCount == 0
-            || MatchingArgumentCount == CallbackSignature::ArgumentCount,
-           "Functor arguments are not compatible (must be QPermission)");
-
-        QtPrivate::QSlotObjectBase *slotObj = nullptr;
-        if constexpr (MatchingArgumentCount == CallbackSignature::ArgumentCount) {
-            slotObj = new QtPrivate::QFunctorSlotObject<Func, 1,
-                typename CallbackSignature::Arguments, void>(std::move(func));
-        } else {
-            slotObj = new QtPrivate::QFunctorSlotObject<Func, 0,
-                typename QtPrivate::List_Left<void, 0>::Value, void>(std::move(func));
-        }
-
-        requestPermission(permission, slotObj, context);
+        using Prototype = void(*)(QPermission);
+        requestPermission(permission,
+                          QtPrivate::makeSlotObject<Prototype>(std::move(func)),
+                          receiver);
     }
+# endif // Q_QDOC
 
     // requestPermission to a functor or function pointer (without context)
-    template <typename Func, std::enable_if_t<
-        !QtPrivate::FunctionPointer<Func>::IsPointerToMemberFunction
-        && !std::is_same<const char *, Func>::value, bool> = true>
-    void requestPermission(const QPermission &permission, Func func)
+    template <typename Functor>
+    void requestPermission(const QPermission &permission, Functor func)
     {
         requestPermission(permission, nullptr, std::move(func));
     }
 
 private:
+    // ### Qt 7: rename to requestPermissionImpl to avoid ambiguity
     void requestPermission(const QPermission &permission,
         QtPrivate::QSlotObjectBase *slotObj, const QObject *context);
 public:
-# endif // Q_QDOC
 
 #endif // QT_CONFIG(permission)
 
diff --git a/src/corelib/kernel/qobjectdefs_impl.h b/src/corelib/kernel/qobjectdefs_impl.h
index 10f06609ec..fea9e0ac52 100644
--- a/src/corelib/kernel/qobjectdefs_impl.h
+++ b/src/corelib/kernel/qobjectdefs_impl.h
@@ -333,6 +333,27 @@ namespace QtPrivate {
         typedef decltype(dummy<Functor>().operator()((dummy<ArgList>())...)) Value;
     };
 
+    /*
+        Wrapper around ComputeFunctorArgumentCount and CheckCompatibleArgument,
+        depending on whether \a Functor is a PMF or not. Returns -1 if \a Func is
+        not compatible with the \a ExpectedArguments, otherwise returns >= 0.
+    */
+    template<typename Prototype, typename Functor>
+    constexpr int inline countMatchingArguments()
+    {
+        using ExpectedArguments = typename QtPrivate::FunctionPointer<Prototype>::Arguments;
+
+        if constexpr (QtPrivate::FunctionPointer<Functor>::IsPointerToMemberFunction) {
+            using ActualArguments = typename QtPrivate::FunctionPointer<Functor>::Arguments;
+            if constexpr (QtPrivate::CheckCompatibleArguments<ExpectedArguments, ActualArguments>::value)
+                return QtPrivate::FunctionPointer<Functor>::ArgumentCount;
+            else
+                return -1;
+        } else {
+            return QtPrivate::ComputeFunctorArgumentCount<Functor, ExpectedArguments>::Value;
+        }
+    }
+
     // internal base class (interface) containing functions required to call a slot managed by a pointer to function.
     class QSlotObjectBase {
         QAtomicInt m_ref;
@@ -428,6 +449,64 @@ namespace QtPrivate {
 
     template <typename Func>
     using QFunctorSlotObjectWithNoArgsImplicitReturn = QFunctorSlotObjectWithNoArgs<Func, typename QtPrivate::FunctionPointer<Func>::ReturnType>;
+
+
+    // Helper to detect the context object type based on the functor type:
+    // QObject for free functions and lambdas; the callee for member function
+    // pointers. The default declaration doesn't have the ContextType typedef,
+    // and so non-functor APIs (like old-style string-based slots) are removed
+    // from the overload set.
+    template <typename Func, typename = void>
+    struct ContextTypeForFunctor {};
+
+    template <typename Func>
+    struct ContextTypeForFunctor<Func,
+        std::enable_if_t<std::negation_v<std::disjunction<std::is_same<const char *, Func>,
+                                                          std::is_member_function_pointer<Func>>>
+        >
+    >
+    {
+        using ContextType = QObject;
+    };
+    template <typename Func>
+    struct ContextTypeForFunctor<Func,
+        std::enable_if_t<std::conjunction_v<std::negation<std::is_same<const char *, Func>>,
+                                            std::is_member_function_pointer<Func>,
+                                            std::is_convertible<typename QtPrivate::FunctionPointer<Func>::Object *, QObject *>>
+        >
+    >
+    {
+        using ContextType = typename QtPrivate::FunctionPointer<Func>::Object;
+    };
+
+    /*
+        Returns a suitable QSlotObjectBase object that holds \a func, if possible.
+
+        Not available (and thus produces compile-time errors) if the Functor provided is
+        not compatible with the expected Prototype.
+    */
+    template <typename Prototype, typename Functor>
+    static constexpr std::enable_if_t<QtPrivate::countMatchingArguments<Prototype, Functor>() >= 0,
+        QtPrivate::QSlotObjectBase *>
+    makeSlotObject(Functor &&func)
+    {
+        using ExpectedSignature = QtPrivate::FunctionPointer<Prototype>;
+        using ExpectedArguments = typename ExpectedSignature::Arguments;
+        using ActualSignature = QtPrivate::FunctionPointer<Functor>;
+
+        static_assert(int(ActualSignature::ArgumentCount) <= int(ExpectedSignature::ArgumentCount),
+            "Functor requires more arguments than what can be provided.");
+        constexpr int MatchingArgumentCount = QtPrivate::countMatchingArguments<Prototype, Functor>();
+
+        if constexpr (QtPrivate::FunctionPointer<Functor>::IsPointerToMemberFunction) {
+            using ActualArguments = typename ActualSignature::Arguments;
+            return  new QtPrivate::QSlotObject<Functor, ActualArguments, void>(func);
+        } else {
+            using ActualArguments = typename QtPrivate::List_Left<ExpectedArguments, MatchingArgumentCount>::Value;
+
+            return new QtPrivate::QFunctorSlotObject<Functor, MatchingArgumentCount, ActualArguments, void>(std::move(func));
+        }
+    }
 }
 
 QT_END_NAMESPACE
diff --git a/src/network/kernel/qhostinfo.h b/src/network/kernel/qhostinfo.h
index 47db075b44..d990af685a 100644
--- a/src/network/kernel/qhostinfo.h
+++ b/src/network/kernel/qhostinfo.h
@@ -57,60 +57,28 @@ public:
 
 #ifdef Q_QDOC
     template<typename Functor>
-    static int lookupHost(const QString &name, Functor functor);
-    template<typename Functor>
     static int lookupHost(const QString &name, const QObject *context, Functor functor);
 #else
-    // lookupHost to a QObject slot
-    template <typename Func>
+    // lookupHost to a callable (with context)
+    template <typename Functor>
     static inline int lookupHost(const QString &name,
-                                 const typename QtPrivate::FunctionPointer<Func>::Object *receiver,
-                                 Func slot)
+                                 const typename QtPrivate::ContextTypeForFunctor<Functor>::ContextType *receiver,
+                                 Functor func)
     {
-        typedef QtPrivate::FunctionPointer<Func> SlotType;
-
-        typedef QtPrivate::FunctionPointer<void (*)(QHostInfo)> SignalType;
-        static_assert(int(SignalType::ArgumentCount) >= int(SlotType::ArgumentCount),
-                          "The slot requires more arguments than the signal provides.");
-        static_assert((QtPrivate::CheckCompatibleArguments<typename SignalType::Arguments,
-                           typename SlotType::Arguments>::value),
-                          "Signal and slot arguments are not compatible.");
-        static_assert((QtPrivate::AreArgumentsCompatible<typename SlotType::ReturnType,
-                           typename SignalType::ReturnType>::value),
-                          "Return type of the slot is not compatible "
-                          "with the return type of the signal.");
-
-        auto slotObj = new QtPrivate::QSlotObject<Func, typename SlotType::Arguments, void>(slot);
-        return lookupHostImpl(name, receiver, slotObj, nullptr);
+        using Prototype = void(*)(QHostInfo);
+        return lookupHostImpl(name, receiver,
+                              QtPrivate::makeSlotObject<Prototype>(std::move(func)),
+                              nullptr);
     }
+#endif // Q_QDOC
 
     // lookupHost to a callable (without context)
-    template <typename Func>
-    static inline typename std::enable_if<!QtPrivate::FunctionPointer<Func>::IsPointerToMemberFunction &&
-                                          !std::is_same<const char *, Func>::value, int>::type
-        lookupHost(const QString &name, Func slot)
+    template <typename Functor>
+    static inline int lookupHost(const QString &name, Functor slot)
     {
         return lookupHost(name, nullptr, std::move(slot));
     }
 
-    // lookupHost to a functor or function pointer (with context)
-    template <typename Func1>
-    static inline typename std::enable_if<!QtPrivate::FunctionPointer<Func1>::IsPointerToMemberFunction &&
-                                          !std::is_same<const char*, Func1>::value, int>::type
-        lookupHost(const QString &name, QObject *context, Func1 slot)
-    {
-        typedef QtPrivate::FunctionPointer<Func1> SlotType;
-
-        static_assert(int(SlotType::ArgumentCount) <= 1,
-                          "The slot must not require more than one argument");
-
-        auto slotObj = new QtPrivate::QFunctorSlotObject<Func1, 1,
-                                                         typename QtPrivate::List<QHostInfo>,
-                                                         void>(std::move(slot));
-        return lookupHostImpl(name, context, slotObj, nullptr);
-    }
-#endif // Q_QDOC
-
 private:
     QHostInfoPrivate *d_ptr;
     Q_DECLARE_PRIVATE(QHostInfo)
diff --git a/tests/auto/corelib/kernel/qpermission/tst_qpermission.cpp b/tests/auto/corelib/kernel/qpermission/tst_qpermission.cpp
index b698969652..146c1b2660 100644
--- a/tests/auto/corelib/kernel/qpermission/tst_qpermission.cpp
+++ b/tests/auto/corelib/kernel/qpermission/tst_qpermission.cpp
@@ -26,6 +26,7 @@ private Q_SLOTS:
 
     void conversionMaintainsState() const;
 
+    void functorWithoutContext();
     void functorWithContextInThread();
     void receiverInThread();
     void destroyedContextObject();
@@ -145,6 +146,23 @@ void tst_QPermission::conversionMaintainsState() const
     }
 }
 
+// Compile test for context-less functor overloads
+void tst_QPermission::functorWithoutContext()
+{
+    int argc = 0;
+    char *argv = nullptr;
+    QCoreApplication app(argc, &argv);
+
+    DummyPermission dummy;
+#ifdef Q_OS_DARWIN
+    QTest::ignoreMessage(QtWarningMsg, QRegularExpression(".*Could not find permission plugin for DummyPermission.*"));
+#endif
+
+    qApp->requestPermission(dummy, [](const QPermission &permission){
+        QVERIFY(permission.value<DummyPermission>());
+    });
+}
+
 void tst_QPermission::functorWithContextInThread()
 {
     int argc = 0;
@@ -209,6 +227,11 @@ void tst_QPermission::receiverInThread()
 
     qApp->requestPermission(dummy, &receiver, &Receiver::handlePermission);
     QTRY_COMPARE(receiver.permissionReceiverThread, &receiverThread);
+
+    // compile tests: none of these work and the error output isn't horrible
+    // qApp->requestPermission(dummy, &receiver, "&tst_QPermission::receiverInThread");
+    // qApp->requestPermission(dummy, &receiver, &tst_QPermission::receiverInThread);
+    // qApp->requestPermission(dummy, &receiver, &QObject::destroyed);
 }
 
 void tst_QPermission::destroyedContextObject()