QRunnable: fix Coverity dead-code warnings in create()

Coverity complained about the call to warnNullCallable() being dead
code for most instantiations of the create() function template. It's
right, of course, even though warning in a template that clearly has
instantations where it's not dead code isn't exactly helpful.

Nonetheless, take the opportinity to avoid the dead code warning at
the expense of a bit or un-DRY-ing:

- because we now use them more than once, cache the result of
  is_*function_* predicates in constexpr variables

- then scope the is_null variable such that its use is not subject to
  dead-code removal anymore (at instantiation time; it may still be
  removed by the optimizer)

As drive-bys, add a comment about the reinterpret_cast, and make the
custom template predicates variable- instead of class templates.

Coverity-Id: 407640
Change-Id: I272223042c2aae9d814e82c466e1d29e1c42bfa1
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

1 files changed, 20 insertions, 12 deletions

diff --git a/src/corelib/thread/qrunnable.h b/src/corelib/thread/qrunnable.h
index 80945351ee..756156e583 100644
--- a/src/corelib/thread/qrunnable.h
+++ b/src/corelib/thread/qrunnable.h
@@ -101,27 +101,35 @@ public:
 namespace QtPrivate {
 
 template <typename T>
-using is_function_pointer = std::conjunction<std::is_pointer<T>, std::is_function<std::remove_pointer_t<T>>>;
+constexpr inline bool is_function_pointer_v = std::conjunction_v<
+        std::is_pointer<T>,
+        std::is_function<std::remove_pointer_t<T>>
+    >;
 template <typename T>
-struct is_std_function : std::false_type {};
+constexpr inline bool is_std_function_v = false;
 template <typename T>
-struct is_std_function<std::function<T>> : std::true_type {};
+constexpr inline bool is_std_function_v<std::function<T>> = true;
 
 } // namespace QtPrivate
 
 template <typename Callable, QRunnable::if_callable<Callable>>
 QRunnable *QRunnable::create(Callable &&functionToRun)
 {
-    bool is_null = false;
-    if constexpr(QtPrivate::is_std_function<std::decay_t<Callable>>::value)
-        is_null = !functionToRun;
-
-    if constexpr(QtPrivate::is_function_pointer<std::decay_t<Callable>>::value) {
-        const void *functionPtr = reinterpret_cast<void *>(functionToRun);
-        is_null = !functionPtr;
+    using F = std::decay_t<Callable>;
+    constexpr bool is_std_function = QtPrivate::is_std_function_v<F>;
+    constexpr bool is_function_pointer = QtPrivate::is_function_pointer_v<F>;
+    if constexpr (is_std_function || is_function_pointer) {
+        bool is_null;
+        if constexpr (is_std_function) {
+            is_null = !functionToRun;
+        } else if constexpr (is_function_pointer) {
+            // shut up warnings about functions always having a non-null address:
+            const void *functionPtr = reinterpret_cast<void *>(functionToRun);
+            is_null = !functionPtr;
+        }
+        if (is_null)
+            return warnNullCallable();
     }
-    if (is_null)
-        return warnNullCallable();
 
     return new QGenericRunnable(std::forward<Callable>(functionToRun));
 }