QVariant: use comparison helper macros

The relational operators were removed in
8652c79df0 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>

src/corelib/kernel/qvariant.cpp

@@ -324,6 +324,7 @@ static QVariant::Private clonePrivate(const QVariant::Private &other)
     \ingroup objectmodel
     \ingroup shared
 
+    \compares equality
 
     Because C++ forbids unions from including types that have
     non-default constructors or destructors, most interesting Qt
@@ -2152,9 +2153,9 @@ bool QVariant::view(int type, void *ptr)
 }
 
 /*!
-    \fn bool QVariant::operator==(const QVariant &v1, const QVariant &v2)
+    \fn bool QVariant::operator==(const QVariant &lhs, const QVariant &rhs)
 
-    Returns \c true if \a v1 and \a v2 are equal; otherwise returns \c false.
+    Returns \c true if \a lhs and \a rhs are equal; otherwise returns \c false.
 
     QVariant uses the equality operator of the type() contained to check for
     equality.
@@ -2178,9 +2179,9 @@ bool QVariant::view(int type, void *ptr)
 */
 
 /*!
-    \fn bool QVariant::operator!=(const QVariant &v1, const QVariant &v2)
+    \fn bool QVariant::operator!=(const QVariant &lhs, const QVariant &rhs)
 
-    Returns \c false if \a v1 and \a v2 are equal; otherwise returns \c true.
+    Returns \c false if \a lhs and \a rhs are equal; otherwise returns \c true.
 
     QVariant uses the equality operator of the type() contained to check for
     equality.

src/corelib/kernel/qvariant.h

@@ -5,6 +5,7 @@
 #define QVARIANT_H
 
 #include <QtCore/qatomic.h>
+#include <QtCore/qcompare.h>
 #include <QtCore/qcontainerfwd.h>
 #include <QtCore/qmetatype.h>
 #ifndef QT_NO_DEBUG_STREAM
@@ -614,10 +615,10 @@ private:
         return std::visit(visitor, std::forward<StdVariant>(v));
     }
 
-    friend inline bool operator==(const QVariant &a, const QVariant &b)
+    friend bool comparesEqual(const QVariant &a, const QVariant &b)
     { return a.equals(b); }
-    friend inline bool operator!=(const QVariant &a, const QVariant &b)
-    { return !a.equals(b); }
+    Q_DECLARE_EQUALITY_COMPARABLE(QVariant)
+
 #ifndef QT_NO_DEBUG_STREAM
     template <typename T>
     friend auto operator<<(const QDebug &debug, const T &variant) -> std::enable_if_t<std::is_same_v<T, QVariant>, QDebug> {

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
 )

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{} };
+
+    const QVariant nonObjV1 = QVariant::fromValue<const void*>(&arr[0]);
+    const QVariant nonObjV2 = QVariant::fromValue<const void*>(&arr[1]);
+
+    Qt::partial_ordering expectedOrdering = Qt::partial_ordering::equivalent;
+    QCOMPARE(QVariant::compare(nonObjV1, nonObjV1), expectedOrdering);
+    QT_TEST_EQUALITY_OPS(nonObjV1, nonObjV1, is_eq(expectedOrdering));
+
+    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;
 
-    MyClass myClass;
-
-    QVariant v  = QVariant::fromValue<void *>(&myClass);
-    QVariant v2 = QVariant::fromValue<void *>(&myClass);
-
-    QCOMPARE(v, v2);
+    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_data() const
+{
+    QTest::addColumn<QVariant>("v1");
+    QTest::addColumn<QVariant>("v2");
+    QTest::addColumn<Qt::partial_ordering>("expectedOrdering");
+
+    QTest::newRow("same_uncomparable")
+            << QVariant::fromValue(WontCompare{0})
+            << QVariant::fromValue(WontCompare{0})
+            << Qt::partial_ordering::unordered;
+
+    QTest::newRow("same_comparable")
+            << QVariant::fromValue(WillCompare{0})
+            << QVariant::fromValue(WillCompare{0})
+            << Qt::partial_ordering::equivalent;
+
+    QTest::newRow("different_comparable")
+            << QVariant::fromValue(WillCompare{1})
+            << QVariant::fromValue(WillCompare{0})
+            << Qt::partial_ordering::greater;
+
+    QTest::newRow("qdatetime_vs_comparable")
+            << QVariant::fromValue(QDateTime::currentDateTimeUtc())
+            << QVariant::fromValue(WillCompare{0})
+            << Qt::partial_ordering::unordered;
+}
+
 void tst_QVariant::compareCustomTypes() const
 {
-    {
-        WontCompare f1{0};
-        const QVariant variant1(QVariant::fromValue(f1));
+    QFETCH(const QVariant, v1);
+    QFETCH(const QVariant, v2);
+    QFETCH(const Qt::partial_ordering, expectedOrdering);
 
-        WontCompare f2{1};
-        const QVariant variant2(QVariant::fromValue(f2));
-
-        /* No comparison operator exists. */
-        QVERIFY(variant1 != variant2);
-        QVERIFY(variant1 != variant1);
-        QVERIFY(variant2 != variant2);
-    }
-    {
-        WillCompare f1{0};
-        const QVariant variant1(QVariant::fromValue(f1));
-
-        WillCompare f2 {1};
-        const QVariant variant2(QVariant::fromValue(f2));
-
-        QVERIFY(variant1 != variant2);
-        QCOMPARE(variant1, variant1);
-        QCOMPARE(variant2, variant2);
-    }
+    QCOMPARE(QVariant::compare(v1, v2), expectedOrdering);
+    QT_TEST_EQUALITY_OPS(v1, v2, is_eq(expectedOrdering));
 }
 void tst_QVariant::timeToDateTime() const
 {