QExplicitlySharedDataPointerV2: Use new comparison helper macros

Provide the new comparesEqual() helper function as an implementation of
the (in)equality operators.
The smart pointers are totally ordered, so generalize
QExplicitlySharedDataPointerV2 to the full comparison set by adding
compareThreeWay() helper function.
Use Q_DECLARE_STRONGLY_ORDERED_LITERAL_TYPE to provide all relational
operators.

Add comparisons between QExplicitlySharedDataPointerV2 and
std::nullptr_t.

Use QT_TEST_ALL_COMPARISON_OPS macros in unit-tests.

Use the new Qt::totally_ordered_wrapper to wrap the "d" pointer to
avoid UB when performing comparisons of QExplicitlySharedDataPointerV2.

Task-number: QTBUG-120306
Change-Id: I177fdc1ff2363a5e84e65506468093b87c5c9c03
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>

src/corelib/tools/qshareddata_impl.h

@@ -9,6 +9,7 @@
 #ifndef QSHAREDDATA_IMPL_H
 #define QSHAREDDATA_IMPL_H
 
+#include <QtCore/qcompare.h>
 #include <QtCore/qglobal.h>
 #include <QtCore/qshareddata.h>
 
@@ -19,7 +20,7 @@ namespace QtPrivate {
 template <typename T>
 class QExplicitlySharedDataPointerV2
 {
-    T *d;
+    Qt::totally_ordered_wrapper<T *> d;
 
 public:
     constexpr QExplicitlySharedDataPointerV2() noexcept : d(nullptr) {}
@@ -65,14 +66,14 @@ public:
     ~QExplicitlySharedDataPointerV2()
     {
         if (d && !d->ref.deref())
-            delete d;
+            delete d.get();
     }
 
     void detach()
     {
         if (!d) {
             // should this codepath be here on in all user's detach()?
-            d = new T;
+            d.reset(new T);
             d->ref.ref();
         } else if (d->ref.loadRelaxed() != 1) {
             // TODO: qAtomicDetach here...?
@@ -84,15 +85,15 @@ public:
     void reset(T *t = nullptr) noexcept
     {
         if (d && !d->ref.deref())
-            delete d;
+            delete d.get();
-        d = t;
+        d.reset(t);
         if (d)
             d->ref.ref();
     }
 
     constexpr T *take() noexcept
     {
-        return std::exchange(d, nullptr);
+        return std::exchange(d, nullptr).get();
     }
 
     bool isShared() const noexcept
@@ -106,27 +107,33 @@ public:
     }
 
     // important change from QExplicitlySharedDataPointer: deep const
-    constexpr T &operator*() { return *d; }
+    constexpr T &operator*() { return *(d.get()); }
-    constexpr T *operator->() { return d; }
+    constexpr T *operator->() { return d.get(); }
-    constexpr const T &operator*() const { return *d; }
+    constexpr const T &operator*() const { return *(d.get()); }
-    constexpr const T *operator->() const { return d; }
+    constexpr const T *operator->() const { return d.get(); }
 
-    constexpr T *data() noexcept { return d; }
+    constexpr T *data() noexcept { return d.get(); }
-    constexpr const T *data() const noexcept { return d; }
+    constexpr const T *data() const noexcept { return d.get(); }
 
-    constexpr explicit operator bool() const noexcept { return d; }
+    constexpr explicit operator bool() const noexcept { return d.get(); }
 
-    constexpr friend bool operator==(const QExplicitlySharedDataPointerV2 &lhs,
+private:
-                                     const QExplicitlySharedDataPointerV2 &rhs) noexcept
+    constexpr friend bool comparesEqual(const QExplicitlySharedDataPointerV2 &lhs,
-    {
+                                        const QExplicitlySharedDataPointerV2 &rhs) noexcept
-        return lhs.d == rhs.d;
+    { return lhs.d == rhs.d; }
-    }
+    constexpr friend Qt::strong_ordering
+    compareThreeWay(const QExplicitlySharedDataPointerV2 &lhs,
+                    const QExplicitlySharedDataPointerV2 &rhs) noexcept
+    { return Qt::compareThreeWay(lhs.d, rhs.d); }
+    Q_DECLARE_STRONGLY_ORDERED_LITERAL_TYPE(QExplicitlySharedDataPointerV2)
 
-    constexpr friend bool operator!=(const QExplicitlySharedDataPointerV2 &lhs,
+    constexpr friend bool
-                                     const QExplicitlySharedDataPointerV2 &rhs) noexcept
+    comparesEqual(const QExplicitlySharedDataPointerV2 &lhs, std::nullptr_t) noexcept
-    {
+    { return lhs.d == nullptr; }
-        return lhs.d != rhs.d;
+    constexpr friend Qt::strong_ordering
-    }
+    compareThreeWay(const QExplicitlySharedDataPointerV2 &lhs, std::nullptr_t) noexcept
+    { return Qt::compareThreeWay(lhs.d, nullptr); }
+    Q_DECLARE_STRONGLY_ORDERED_LITERAL_TYPE(QExplicitlySharedDataPointerV2, std::nullptr_t)
 };
 
 template <typename T>

tests/auto/corelib/tools/qexplicitlyshareddatapointerv2/CMakeLists.txt

@@ -14,4 +14,6 @@ endif()
 qt_internal_add_test(tst_qexplicitlyshareddatapointerv2
     SOURCES
         tst_qexplicitlyshareddatapointerv2.cpp
+    LIBRARIES
+        Qt::TestPrivate
 )

tests/auto/corelib/tools/qexplicitlyshareddatapointerv2/tst_qexplicitlyshareddatapointerv2.cpp

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only
 
 #include <QTest>
+#include <QtTest/private/qcomparisontesthelper_p.h>
 
 #include <QtCore/qshareddata_impl.h>
 
@@ -16,6 +17,7 @@ private slots:
     void moveConstructor() const;
     void copyAssignment() const;
     void moveAssignment() const;
+    void compareCompiles() const;
     void compare() const;
     void mutability() const;
     void data() const;
@@ -46,8 +48,8 @@ void tst_QExplicitlySharedDataPointerv2::moveConstructor() const
 {
     QESDP_V2<const MyClass> pointer(new MyClass());
     const QESDP_V2<const MyClass> moved(std::move(pointer));
-    QCOMPARE_NE(moved.data(), static_cast<MyClass *>(0));
+    QCOMPARE_NE(moved, nullptr);
-    QCOMPARE_EQ(pointer.data(), static_cast<MyClass *>(0));
+    QCOMPARE_EQ(pointer, nullptr);
 }
 
 void tst_QExplicitlySharedDataPointerv2::copyAssignment() const
@@ -61,29 +63,35 @@ void tst_QExplicitlySharedDataPointerv2::moveAssignment() const
 {
     QESDP_V2<const MyClass> pointer(new MyClass());
     const QESDP_V2<const MyClass> moved = std::move(pointer);
-    QCOMPARE_NE(moved.data(), static_cast<MyClass *>(0));
+    QCOMPARE_NE(moved, nullptr);
-    QCOMPARE_EQ(pointer.data(), static_cast<MyClass *>(0));
+    QCOMPARE_EQ(pointer, nullptr);
+}
+
+void tst_QExplicitlySharedDataPointerv2::compareCompiles() const
+{
+    QTestPrivate::testAllComparisonOperatorsCompile<QESDP_V2<MyClass>>();
+    QTestPrivate::testAllComparisonOperatorsCompile<QESDP_V2<MyClass>, std::nullptr_t>();
 }
 
 void tst_QExplicitlySharedDataPointerv2::compare() const
 {
     const QESDP_V2<MyClass> ptr;
     const QESDP_V2<MyClass> ptr2;
-    QCOMPARE_EQ(ptr.data(), static_cast<MyClass *>(0));
+    QT_TEST_ALL_COMPARISON_OPS(ptr, nullptr, Qt::strong_ordering::equal);
-    QCOMPARE_EQ(ptr2.data(), static_cast<MyClass *>(0));
+    QT_TEST_ALL_COMPARISON_OPS(ptr2, nullptr, Qt::strong_ordering::equal);
-    QCOMPARE_EQ(ptr, ptr2);
+    QT_TEST_ALL_COMPARISON_OPS(ptr, ptr2, Qt::strong_ordering::equal);
 
     const QESDP_V2<MyClass> copy(ptr);
-    QCOMPARE_EQ(ptr, copy);
+    QT_TEST_ALL_COMPARISON_OPS(ptr, copy, Qt::strong_ordering::equal);
 
     const QESDP_V2<MyClass> new_ptr(new MyClass());
-    QCOMPARE_NE(new_ptr.data(), static_cast<MyClass *>(0));
+    QT_TEST_ALL_COMPARISON_OPS(new_ptr, nullptr, Qt::strong_ordering::greater);
-    QCOMPARE_NE(ptr, new_ptr);
+    QT_TEST_ALL_COMPARISON_OPS(new_ptr, ptr, Qt::strong_ordering::greater);
 
     std::array<MyClass, 3> myArray {MyClass(2), MyClass(1), MyClass(0)};
     const QESDP_V2<const MyClass> val0(&myArray[0]);
     const QESDP_V2<const MyClass> val1(&myArray[1]);
-    QCOMPARE_NE(val1, val0);
+    QT_TEST_ALL_COMPARISON_OPS(val0, val1, Qt::strong_ordering::less);
 }
 
 void tst_QExplicitlySharedDataPointerv2::mutability() const
@@ -121,12 +129,14 @@ void tst_QExplicitlySharedDataPointerv2::data() const
     {
         QESDP_V2<const MyClass> pointer;
         QCOMPARE_EQ(pointer.data(), static_cast<const MyClass *>(0));
+        QT_TEST_ALL_COMPARISON_OPS(pointer, nullptr, Qt::strong_ordering::equal);
     }
 
     {
         const QESDP_V2<const MyClass> pointer(new MyClass());
         /* Check that this cast is possible. */
         Q_UNUSED(static_cast<const MyClass *>(pointer.data()));
+        QT_TEST_ALL_COMPARISON_OPS(pointer, nullptr, Qt::strong_ordering::greater);
     }
 
     {
@@ -154,31 +164,31 @@ void tst_QExplicitlySharedDataPointerv2::reset() const
     /* Reset a default constructed shared data object: reference count is equal to 0. */
     {
         QESDP_V2<MyClass> pointer;
-        QCOMPARE_EQ(pointer.data(), static_cast<MyClass *>(0));
+        QCOMPARE_EQ(pointer, nullptr);
 
         pointer.reset();
-        QCOMPARE_EQ(pointer.data(), static_cast<MyClass *>(0));
+        QCOMPARE_EQ(pointer, nullptr);
     }
 
     /* Reset a shared data object where the reference count is equal to 1. */
     {
         QESDP_V2<MyClass> pointer(new MyClass());
-        QCOMPARE_NE(pointer.data(), static_cast<MyClass *>(0));
+        QCOMPARE_NE(pointer, nullptr);
 
         pointer.reset();
-        QCOMPARE_EQ(pointer.data(), static_cast<MyClass *>(0));
+        QCOMPARE_EQ(pointer, nullptr);
     }
 
     /* Reset a shared data object where the reference count is greater than 1. */
     {
         QESDP_V2<MyClass> pointer(new MyClass());
-        QCOMPARE_NE(pointer.data(), static_cast<MyClass *>(0));
+        QCOMPARE_NE(pointer, nullptr);
         QESDP_V2<MyClass> pointer2(pointer);
-        QCOMPARE_NE(pointer2.data(), static_cast<MyClass *>(0));
+        QCOMPARE_NE(pointer2, nullptr);
 
         pointer.reset();
-        QCOMPARE_EQ(pointer.data(), static_cast<MyClass *>(0));
+        QCOMPARE_EQ(pointer, nullptr);
-        QCOMPARE_NE(pointer2.data(), static_cast<MyClass *>(0));
+        QCOMPARE_NE(pointer2, nullptr);
     }
 }
 
@@ -204,9 +214,9 @@ void tst_QExplicitlySharedDataPointerv2::swap() const
 void tst_QExplicitlySharedDataPointerv2::take() const
 {
     QESDP_V2<MyClass> pointer(new MyClass());
-    QCOMPARE_NE(pointer.data(), static_cast<MyClass *>(0));
+    QCOMPARE_NE(pointer, nullptr);
     delete pointer.take();
-    QCOMPARE_EQ(pointer.data(), static_cast<MyClass *>(0));
+    QCOMPARE_EQ(pointer, nullptr);
 }
 
 QTEST_MAIN(tst_QExplicitlySharedDataPointerv2)