QTest: rip out qxp::function_ref from compare_helper()

[ChangeLog][QtTest] The QCOMPARE_xx macros can now only find
QTest::toString() expansions that are either found via Argument
Dependent Lookup on the type in question or are an instatiation of the
QTest::toString<T>() template. This matches the behavior of the
QCOMPARE() macro.

This changes the way how the toString() overloads are selected, so
we need to explicilty constraint the main QTest::toString() template
in order to pick the free functions when they exist.

Change-Id: Ie28eadac333c4bcd8c08fffd17c54e768c5cffd0
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>

6 files changed, 100 insertions, 28 deletions

diff --git a/src/testlib/qtestcase.cpp b/src/testlib/qtestcase.cpp
index 75c36066af..2ab55cac36 100644
--- a/src/testlib/qtestcase.cpp
+++ b/src/testlib/qtestcase.cpp
@@ -2748,6 +2748,7 @@ bool QTest::compare_helper(bool success, const char *failureMsg,
 }
 #endif // QT_DEPRECATED_SINCE(6, 4)
 
+#if QT_DEPRECATED_SINCE(6, 8)
 /*! \internal
     \since 6.4
     This function is called by various specializations of QTest::qCompare
@@ -2767,12 +2768,34 @@ bool QTest::compare_helper(bool success, const char *failureMsg,
                            const char *actual, const char *expected,
                            const char *file, int line)
 {
-    auto functionRefFormatter = [](const void *f) {
-        auto formatter = static_cast<const qxp::function_ref<const char *()> *>(f);
-        return (*formatter)();
-    };
-    return QTestResult::reportResult(success, &actualVal, &expectedVal, functionRefFormatter,
-                                     functionRefFormatter, actual, expected,
+    return QTestResult::reportResult(success, &actualVal, &expectedVal,
+                                     QTest::functionRefFormatter,
+                                     QTest::functionRefFormatter, actual, expected,
+                                     QTest::ComparisonOperation::CustomCompare,
+                                     file, line, failureMsg);
+}
+#endif // QT_DEPRECATED_SINCE(6, 8)
+
+/*! \internal
+    \since 6.8
+    This function is called by various specializations of QTest::qCompare
+    to decide whether to report a failure and to produce verbose test output.
+
+    The \a failureMsg parameter can be \c {nullptr}, in which case a default
+    message will be output if the compare fails. If the comparison succeeds,
+    \a failureMsg will not be output.
+*/
+
+bool QTest::compare_helper(bool success, const char *failureMsg,
+                           const void *actualPtr, const void *expectedPtr,
+                           const char *(*actualFormatter)(const void *),
+                           const char *(*expectedFormatter)(const void *),
+                           const char *actual, const char *expected,
+                           const char *file, int line)
+{
+    return QTestResult::reportResult(success, actualPtr, expectedPtr,
+                                     actualFormatter, expectedFormatter,
+                                     actual, expected,
                                      QTest::ComparisonOperation::CustomCompare,
                                      file, line, failureMsg);
 }
@@ -2818,9 +2841,10 @@ static bool floatingCompare(const T &actual, const T &expected)
 bool QTest::qCompare(qfloat16 const &t1, qfloat16 const &t2, const char *actual, const char *expected,
                      const char *file, int line)
 {
+    auto formatter = Internal::genericToString<qfloat16>;
     return compare_helper(floatingCompare(t1, t2),
                           "Compared qfloat16s are not the same (fuzzy compare)",
-                          [&t1] { return toString(t1); }, [&t2] { return toString(t2); },
+                          &t1, &t2, formatter, formatter,
                           actual, expected, file, line);
 }
 
@@ -3137,8 +3161,9 @@ char *QTest::toString(const volatile QObject *vo)
 bool QTest::compare_string_helper(const char *t1, const char *t2, const char *actual,
                                   const char *expected, const char *file, int line)
 {
+    auto formatter = Internal::genericToString<const char *>;
     return compare_helper(qstrcmp(t1, t2) == 0, "Compared strings are not the same",
-                          [t1] { return toString(t1); }, [t2] { return toString(t2); },
+                          &t1, &t2, formatter, formatter,
                           actual, expected, file, line);
 }
 
diff --git a/src/testlib/qtestcase.h b/src/testlib/qtestcase.h
index 6df20f8121..6dddcb5045 100644
--- a/src/testlib/qtestcase.h
+++ b/src/testlib/qtestcase.h
@@ -341,6 +341,12 @@ namespace QTest
         return toString(static_cast<const char *>(arg));
     }
 
+    template <typename T> const char *pointerToString(const void *arg)
+    {
+        using QTest::toString;
+        return toString(static_cast<const T *>(arg));
+    }
+
     // Exported so Qt Quick Test can also use it for generating backtraces upon crashes.
     Q_TESTLIB_EXPORT extern bool noCrashHandler;
 
@@ -437,7 +443,7 @@ namespace QTest
     Q_TESTLIB_EXPORT char keyToAscii(Qt::Key key);
 
 #if QT_DEPRECATED_SINCE(6, 4)
-    QT_DEPRECATED_VERSION_X_6_4("use an overload that takes function_ref as parameters, "
+    QT_DEPRECATED_VERSION_X_6_4("use an overload that takes a formatter callback, "
                                 "or an overload that takes only failure message, if you "
                                 "do not need to stringify the values")
     Q_TESTLIB_EXPORT bool compare_helper(bool success, const char *failureMsg,
@@ -445,11 +451,22 @@ namespace QTest
                                          const char *actual, const char *expected,
                                          const char *file, int line);
 #endif // QT_DEPRECATED_SINCE(6, 4)
+#if QT_DEPRECATED_SINCE(6, 8)
+    QT_DEPRECATED_VERSION_X_6_8("use an overload that takes a formatter callback, "
+                                "or an overload that takes only failure message, if you "
+                                "do not need to stringify the values")
     Q_TESTLIB_EXPORT bool compare_helper(bool success, const char *failureMsg,
                                          qxp::function_ref<const char*()> actualVal,
                                          qxp::function_ref<const char*()> expectedVal,
                                          const char *actual, const char *expected,
                                          const char *file, int line);
+#endif // QT_DEPRECATED_SINCE(6, 8)
+    Q_TESTLIB_EXPORT bool compare_helper(bool success, const char *failureMsg,
+                                         const void *actualPtr, const void *expectedPtr,
+                                         const char *(*actualFormatter)(const void *),
+                                         const char *(*expectedFormatter)(const void *),
+                                         const char *actual, const char *expected,
+                                         const char *file, int line);
     Q_TESTLIB_EXPORT bool compare_helper(bool success, const char *failureMsg,
                                          const char *actual, const char *expected,
                                          const char *file, int line);
@@ -517,57 +534,71 @@ namespace QTest
     inline bool compare_ptr_helper(const volatile void *t1, const volatile void *t2, const char *actual,
                                    const char *expected, const char *file, int line)
     {
+        auto formatter = Internal::pointerToString<void>;
         return compare_helper(t1 == t2, "Compared pointers are not the same",
-                              [t1] { return toString(t1); }, [t2] { return toString(t2); },
-                              actual, expected, file, line);
+                              const_cast<const void *>(t1), const_cast<const void *>(t2),
+                              formatter, formatter, actual, expected, file, line);
     }
 
     inline bool compare_ptr_helper(const volatile QObject *t1, const volatile QObject *t2, const char *actual,
                                    const char *expected, const char *file, int line)
     {
+        auto formatter = Internal::pointerToString<QObject>;
         return compare_helper(t1 == t2, "Compared QObject pointers are not the same",
-                              [t1] { return toString(t1); }, [t2] { return toString(t2); },
-                              actual, expected, file, line);
+                              const_cast<const QObject *>(t1), const_cast<const QObject *>(t2),
+                              formatter, formatter, actual, expected, file, line);
     }
 
     inline bool compare_ptr_helper(const volatile QObject *t1, std::nullptr_t, const char *actual,
                                    const char *expected, const char *file, int line)
     {
+        auto lhsFormatter = Internal::pointerToString<QObject>;
+        auto rhsFormatter = Internal::genericToString<std::nullptr_t>;
         return compare_helper(t1 == nullptr, "Compared QObject pointers are not the same",
-                              [t1] { return toString(t1); }, [] { return toString(nullptr); },
-                              actual, expected, file, line);
+                              const_cast<const QObject *>(t1), nullptr,
+                              lhsFormatter, rhsFormatter, actual, expected, file, line);
     }
 
     inline bool compare_ptr_helper(std::nullptr_t, const volatile QObject *t2, const char *actual,
                                    const char *expected, const char *file, int line)
     {
+        auto lhsFormatter = Internal::genericToString<std::nullptr_t>;
+        auto rhsFormatter = Internal::pointerToString<QObject>;
         return compare_helper(nullptr == t2, "Compared QObject pointers are not the same",
-                              [] { return toString(nullptr); }, [t2] { return toString(t2); },
-                              actual, expected, file, line);
+                              nullptr, const_cast<const QObject *>(t2),
+                              lhsFormatter, rhsFormatter, actual, expected, file, line);
     }
 
     inline bool compare_ptr_helper(const volatile void *t1, std::nullptr_t, const char *actual,
                                    const char *expected, const char *file, int line)
     {
+        auto lhsFormatter = Internal::pointerToString<void>;
+        auto rhsFormatter = Internal::genericToString<std::nullptr_t>;
         return compare_helper(t1 == nullptr, "Compared pointers are not the same",
-                              [t1] { return toString(t1); }, [] { return toString(nullptr); },
-                              actual, expected, file, line);
+                              const_cast<const void *>(t1), nullptr,
+                              lhsFormatter, rhsFormatter, actual, expected, file, line);
     }
 
     inline bool compare_ptr_helper(std::nullptr_t, const volatile void *t2, const char *actual,
                                    const char *expected, const char *file, int line)
     {
+        auto lhsFormatter = Internal::genericToString<std::nullptr_t>;
+        auto rhsFormatter = Internal::pointerToString<void>;
         return compare_helper(nullptr == t2, "Compared pointers are not the same",
-                              [] { return toString(nullptr); }, [t2] { return toString(t2); },
-                              actual, expected, file, line);
+                              nullptr, const_cast<const void *>(t2),
+                              lhsFormatter, rhsFormatter, actual, expected, file, line);
     }
 
     template <typename T1, typename T2 = T1>
     inline bool qCompare(const T1 &t1, const T2 &t2, const char *actual, const char *expected,
                          const char *file, int line)
     {
+        using D1 = std::decay_t<T1>;
+        using D2 = std::decay_t<T2>;
+        using Internal::genericToString;
         return compare_helper(t1 == t2, "Compared values are not the same",
-                              [&t1] { return toString(t1); }, [&t2] { return toString(t2); },
+                              std::addressof(t1), std::addressof(t2),
+                              genericToString<D1>, genericToString<D2>,
                               actual, expected, file, line);
     }
 
diff --git a/src/testlib/qtestcase.qdoc b/src/testlib/qtestcase.qdoc
index 2eb060b9af..6a067c351f 100644
--- a/src/testlib/qtestcase.qdoc
+++ b/src/testlib/qtestcase.qdoc
@@ -1423,7 +1423,7 @@
 */
 
 /*!
-    \fn template<typename T> char *QTest::toString(const T &value)
+    \fn template<typename T, QTest::Internal::is_suitable_type_v<T> = true> char *QTest::toString(const T &value)
 
     Returns a textual representation of \a value. This function is used by
     \l QCOMPARE() to output verbose information in case of a test failure.
diff --git a/src/testlib/qtestresult_p.h b/src/testlib/qtestresult_p.h
index 52607cc2ee..e94de64c06 100644
--- a/src/testlib/qtestresult_p.h
+++ b/src/testlib/qtestresult_p.h
@@ -102,8 +102,8 @@ public:
     static const char *currentAppName();
 
     static bool reportResult(bool success, const void *lhs, const void *rhs,
-                             const char *(*lhsFormatter)(const void*),
-                             const char *(*rhsFormatter)(const void*),
+                             const char *(*lhsFormatter)(const void *),
+                             const char *(*rhsFormatter)(const void *),
                              const char *lhsExpr, const char *rhsExpr,
                              QTest::ComparisonOperation op, const char *file, int line,
                              const char *failureMessage = nullptr);
diff --git a/src/testlib/qtesttostring.h b/src/testlib/qtesttostring.h
index 3e2a59c217..18262332ba 100644
--- a/src/testlib/qtesttostring.h
+++ b/src/testlib/qtesttostring.h
@@ -79,6 +79,19 @@ inline typename std::enable_if<!QtPrivate::IsQEnumHelper<F>::Value, char*>::type
     return msg;
 }
 
+template <typename T>
+constexpr bool is_suitable_type_helper_v = std::disjunction_v<std::is_same<T, char>,
+                                                              std::is_same<T, void>,
+                                                              std::is_same<T, QObject>
+                                                             >;
+
+template <typename T>
+using is_suitable_type_v =
+    std::enable_if_t<!(std::is_pointer_v<T>
+                        && is_suitable_type_helper_v<
+                                   std::remove_const_t<std::remove_pointer_t<T>>>),
+                    bool>;
+
 } // namespace Internal
 
 Q_TESTLIB_EXPORT bool compare_string_helper(const char *t1, const char *t2, const char *actual,
@@ -91,7 +104,7 @@ Q_TESTLIB_EXPORT char *toString(const char *);
 Q_TESTLIB_EXPORT char *toString(const volatile void *);
 Q_TESTLIB_EXPORT char *toString(const volatile QObject *);
 
-template<typename T>
+template<typename T, Internal::is_suitable_type_v<T> = true>
 inline char *toString(const T &t)
 {
     return Internal::toString(t);
diff --git a/tests/auto/corelib/io/qurlquery/tst_qurlquery.cpp b/tests/auto/corelib/io/qurlquery/tst_qurlquery.cpp
index 8a799fbf94..8360bdbe28 100644
--- a/tests/auto/corelib/io/qurlquery/tst_qurlquery.cpp
+++ b/tests/auto/corelib/io/qurlquery/tst_qurlquery.cpp
@@ -73,9 +73,12 @@ static QByteArray prettyList(const QueryItems &items)
 static bool compare(const QueryItems &actual, const QueryItems &expected,
                     const char *actualStr, const char *expectedStr, const char *file, int line)
 {
+    auto formatter = [](const void *val) -> const char * {
+        const QueryItems items = *static_cast<const QueryItems *>(val);
+        return qstrdup(prettyList(items).constData());
+    };
     return QTest::compare_helper(actual == expected, "Compared values are not the same",
-                                 [&actual] { return qstrdup(prettyList(actual).constData()); },
-                                 [&expected] { return qstrdup(prettyList(expected).constData()); },
+                                 &actual, &expected, formatter, formatter,
                                  actualStr, expectedStr, file, line);
 }