QVariant: use comparison helper macros

The relational operators were removed in
8652c79df0a47264a2d525424484e15744e2462b with the argument that they
do not have a total order. Back than it was a valid argument, because Qt
did not support any of the C++20 ordering types.

Now Qt has its own implementation for all three ordering types, so we
could technically add relational operators and claim that QVariant
provides partial ordering.

However, that could potentially lead to many bugs and/or unexpected
results, if people start using QVariant as a key in std::map/QMap
containers.

We do not want that to happen, so we only use the helper macros to
implement (in)equality operators.

This commit also extends the unit tests, providing more extensive
testing of (in)equality operators and the pre-existing
QVariant::compare() method.

Fixes: QTBUG-113234
Change-Id: I783f3b5df552da782627f4ed0a5bb1b577753a23
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

2 files changed, 70 insertions, 34 deletions

diff --git a/tests/auto/corelib/kernel/qvariant/CMakeLists.txt b/tests/auto/corelib/kernel/qvariant/CMakeLists.txt
index 4a1eb18d4f..eae9c0d30e 100644
--- a/tests/auto/corelib/kernel/qvariant/CMakeLists.txt
+++ b/tests/auto/corelib/kernel/qvariant/CMakeLists.txt
@@ -25,6 +25,7 @@ qt_internal_add_test(tst_qvariant
     LIBRARIES
         Qt::CorePrivate
         Qt::Gui
+        Qt::TestPrivate
     TESTDATA ${qvariant_resource_files}
     BUILTIN_TESTDATA
 )
diff --git a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
index 84456a57b2..75907ec062 100644
--- a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
+++ b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
@@ -76,6 +76,7 @@ CHECK_GET(MyVariant, const &&);
 #include <QUrl>
 #include <QUuid>
 
+#include <private/qcomparisontesthelper_p.h>
 #include <private/qlocale_p.h>
 #include <private/qmetatype_p.h>
 #include "tst_qvariant_common.h"
@@ -290,6 +291,7 @@ private slots:
     void variantHash();
 
     void convertToQUint8() const;
+    void compareCompiles() const;
     void compareNumerics_data() const;
     void compareNumerics() const;
     void comparePointers() const;
@@ -305,6 +307,7 @@ private slots:
     void loadBrokenUserType();
 
     void invalidDate() const;
+    void compareCustomTypes_data() const;
     void compareCustomTypes() const;
     void timeToDateTime() const;
     void copyingUserTypes() const;
@@ -2007,7 +2010,7 @@ void tst_QVariant::operator_eq_eq()
     QFETCH( QVariant, left );
     QFETCH( QVariant, right );
     QFETCH( bool, equal );
-    QCOMPARE( left == right, equal );
+    QT_TEST_EQUALITY_OPS(left, right, equal);
 }
 
 #if QT_DEPRECATED_SINCE(6, 0)
@@ -2980,6 +2983,11 @@ void tst_QVariant::convertToQUint8() const
     }
 }
 
+void tst_QVariant::compareCompiles() const
+{
+    QTestPrivate::testEqualityOperatorsCompile<QVariant>();
+}
+
 void tst_QVariant::compareNumerics_data() const
 {
     QTest::addColumn<QVariant>("v1");
@@ -3225,25 +3233,38 @@ void tst_QVariant::compareNumerics() const
     QFETCH(QPartialOrdering, result);
     QCOMPARE(QVariant::compare(v1, v2), result);
 
-    QEXPECT_FAIL("invalid-invalid", "needs fixing", Continue);
-    if (result == QPartialOrdering::Equivalent)
-        QCOMPARE_EQ(v1, v2);
-    else
-        QCOMPARE_NE(v1, v2);
+    QEXPECT_FAIL("invalid-invalid", "needs fixing", Abort);
+    QT_TEST_EQUALITY_OPS(v1, v2, is_eq(result));
 }
 
 void tst_QVariant::comparePointers() const
 {
-    class MyClass
-    {
-    };
+    class NonQObjectClass {};
+    const std::array<NonQObjectClass, 2> arr{ NonQObjectClass{}, NonQObjectClass{} };
 
-    MyClass myClass;
+    const QVariant nonObjV1 = QVariant::fromValue<const void*>(&arr[0]);
+    const QVariant nonObjV2 = QVariant::fromValue<const void*>(&arr[1]);
 
-    QVariant v  = QVariant::fromValue<void *>(&myClass);
-    QVariant v2 = QVariant::fromValue<void *>(&myClass);
+    Qt::partial_ordering expectedOrdering = Qt::partial_ordering::equivalent;
+    QCOMPARE(QVariant::compare(nonObjV1, nonObjV1), expectedOrdering);
+    QT_TEST_EQUALITY_OPS(nonObjV1, nonObjV1, is_eq(expectedOrdering));
 
-    QCOMPARE(v, v2);
+    expectedOrdering = Qt::partial_ordering::less;
+    QCOMPARE(QVariant::compare(nonObjV1, nonObjV2), expectedOrdering);
+    QT_TEST_EQUALITY_OPS(nonObjV1, nonObjV2, is_eq(expectedOrdering));
+
+    class QObjectClass : public QObject
+    {
+    public:
+        QObjectClass(QObject *parent = nullptr) : QObject(parent) {}
+    };
+    const QObjectClass c1;
+    const QObjectClass c2;
+
+    const QVariant objV1 = QVariant::fromValue(&c1);
+    const QVariant objV2 = QVariant::fromValue(&c2);
+    QT_TEST_EQUALITY_OPS(objV1, objV1, true);
+    QT_TEST_EQUALITY_OPS(objV1, objV2, false);
 }
 
 struct Data {};
@@ -3442,35 +3463,49 @@ Q_DECLARE_METATYPE(WontCompare);
 struct WillCompare
 {
     int x;
+
+    friend bool operator==(const WillCompare &a, const WillCompare &b)
+    { return a.x == b.x; }
+    friend bool operator<(const WillCompare &a, const WillCompare &b)
+    { return a.x < b.x; }
 };
-bool operator==(const WillCompare &a, const WillCompare &b) { return a.x == b.x; }
 Q_DECLARE_METATYPE(WillCompare);
 
-void tst_QVariant::compareCustomTypes() const
+void tst_QVariant::compareCustomTypes_data() const
 {
-    {
-        WontCompare f1{0};
-        const QVariant variant1(QVariant::fromValue(f1));
+    QTest::addColumn<QVariant>("v1");
+    QTest::addColumn<QVariant>("v2");
+    QTest::addColumn<Qt::partial_ordering>("expectedOrdering");
 
-        WontCompare f2{1};
-        const QVariant variant2(QVariant::fromValue(f2));
+    QTest::newRow("same_uncomparable")
+            << QVariant::fromValue(WontCompare{0})
+            << QVariant::fromValue(WontCompare{0})
+            << Qt::partial_ordering::unordered;
 
-        /* No comparison operator exists. */
-        QVERIFY(variant1 != variant2);
-        QVERIFY(variant1 != variant1);
-        QVERIFY(variant2 != variant2);
-    }
-    {
-        WillCompare f1{0};
-        const QVariant variant1(QVariant::fromValue(f1));
+    QTest::newRow("same_comparable")
+            << QVariant::fromValue(WillCompare{0})
+            << QVariant::fromValue(WillCompare{0})
+            << Qt::partial_ordering::equivalent;
 
-        WillCompare f2 {1};
-        const QVariant variant2(QVariant::fromValue(f2));
+    QTest::newRow("different_comparable")
+            << QVariant::fromValue(WillCompare{1})
+            << QVariant::fromValue(WillCompare{0})
+            << Qt::partial_ordering::greater;
 
-        QVERIFY(variant1 != variant2);
-        QCOMPARE(variant1, variant1);
-        QCOMPARE(variant2, variant2);
-    }
+    QTest::newRow("qdatetime_vs_comparable")
+            << QVariant::fromValue(QDateTime::currentDateTimeUtc())
+            << QVariant::fromValue(WillCompare{0})
+            << Qt::partial_ordering::unordered;
+}
+
+void tst_QVariant::compareCustomTypes() const
+{
+    QFETCH(const QVariant, v1);
+    QFETCH(const QVariant, v2);
+    QFETCH(const Qt::partial_ordering, expectedOrdering);
+
+    QCOMPARE(QVariant::compare(v1, v2), expectedOrdering);
+    QT_TEST_EQUALITY_OPS(v1, v2, is_eq(expectedOrdering));
 }
 void tst_QVariant::timeToDateTime() const
 {