Port QPersistentModelIndex to new comparison helper macros

Decided to keep the strong_ordering semantics, as user code is very
much guaranteed to rely on it.

Update the docs.

[ChangeLog][Potentially Source-Incompatible Changes] Made several
operators (relational, QDebug streaming, etc) hidden friends. This may
break code that calls these operators a) with types that require
implicit conversion on the arguments, or b) using member-function
syntax (lhs.operator<(rhs)). The backwards-compatible fix for (a) is
to make at least one of the argument's implicit conversions
explicit. For (b), use infix notation (lhs < rhs).

[ChangeLog][EDITORIAL] Replace all "Potentially Source-Incompatible
Changes" items that mention making operators hidden friends with the
catch-all one from the QPersistentModelIndex change.

Fixes: QTBUG-117660
Change-Id: I8175af5e2cb3c77e7486e972630b9410b3a0896c
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>

src/corelib/compat/removed_api.cpp

@@ -928,6 +928,28 @@ QUrl QUrl::fromEncoded(const QByteArray &input, ParsingMode mode)
 #endif // QT_CORE_REMOVED_SINCE(6, 7)
 
 #if QT_CORE_REMOVED_SINCE(6, 8)
+#include "qabstractitemmodel.h"
+
+bool QPersistentModelIndex::operator<(const QPersistentModelIndex &other) const noexcept
+{
+    return is_lt(compareThreeWay(*this, other));
+}
+
+bool QPersistentModelIndex::operator==(const QPersistentModelIndex &other) const noexcept
+{
+    return comparesEqual(*this, other);
+}
+
+bool QPersistentModelIndex::operator==(const QModelIndex &other) const noexcept
+{
+    return comparesEqual(*this, other);
+}
+
+bool QPersistentModelIndex::operator!=(const QModelIndex &other) const noexcept
+{
+    return !comparesEqual(*this, other);
+}
+
 #include "qbitarray.h" // inlined API
 
 #include "qbytearray.h" // inlined API

src/corelib/itemmodels/qabstractitemmodel.cpp

@@ -288,6 +288,9 @@ void QPersistentModelIndexData::destroy(QPersistentModelIndexData *data)
   \brief The QPersistentModelIndex class is used to locate data in a data model.
 
   \ingroup model-view
+  \compares strong
+  \compareswith strong QModelIndex
+  \endcompareswith
 
   A QPersistentModelIndex is a model index that can be stored by an
   application, and later used to access information in a model.
@@ -375,45 +378,53 @@ QPersistentModelIndex::~QPersistentModelIndex()
 }
 
 /*!
-  Returns \c{true} if this persistent model index is equal to the \a other
+    \fn bool QPersistentModelIndex::operator==(const QPersistentModelIndex &lhs, const QPersistentModelIndex &rhs)
-  persistent model index; otherwise returns \c{false}.
+    Returns \c{true} if \a lhs persistent model index is equal to the \a rhs
-
-  The internal data pointer, row, column, and model values in the persistent
-  model index are used when comparing with another persistent model index.
-*/
-
-bool QPersistentModelIndex::operator==(const QPersistentModelIndex &other) const noexcept
-{
-    if (d && other.d)
-        return d->index == other.d->index;
-    return d == other.d;
-}
-
-/*!
-    \since 4.1
-
-    Returns \c{true} if this persistent model index is smaller than the \a other
     persistent model index; otherwise returns \c{false}.
 
     The internal data pointer, row, column, and model values in the persistent
     model index are used when comparing with another persistent model index.
 */
 
-bool QPersistentModelIndex::operator<(const QPersistentModelIndex &other) const noexcept
+/*!
-{
+    \fn bool QPersistentModelIndex::operator!=(const QPersistentModelIndex &lhs, const QPersistentModelIndex &rhs)
-    if (d && other.d)
+    \since 4.2
-        return d->index < other.d->index;
 
-    return std::less<>{}(d, other.d);
+    Returns \c{true} if \a lhs persistent model index is not equal to the \a rhs
+    persistent model index; otherwise returns \c{false}.
+*/
+bool comparesEqual(const QPersistentModelIndex &lhs, const QPersistentModelIndex &rhs) noexcept
+{
+    if (lhs.d && rhs.d)
+        return lhs.d->index == rhs.d->index;
+    return lhs.d == rhs.d;
 }
 
 /*!
-    \fn bool QPersistentModelIndex::operator!=(const QPersistentModelIndex &other) const
+    \fn bool QPersistentModelIndex::operator<(const QPersistentModelIndex &lhs, const QPersistentModelIndex &rhs)
-    \since 4.2
+    \since 4.1
 
-    Returns \c{true} if this persistent model index is not equal to the \a
+    Returns \c{true} if \a lhs persistent model index is smaller than the \a rhs
-    other persistent model index; otherwise returns \c{false}.
+    persistent model index; otherwise returns \c{false}.
+
+    The internal data pointer, row, column, and model values in the persistent
+    model index are used when comparing with another persistent model index.
 */
+Qt::strong_ordering compareThreeWay(const QPersistentModelIndex &lhs,
+                                    const QPersistentModelIndex &rhs) noexcept
+{
+    if (lhs.d && rhs.d)
+        return compareThreeWay(lhs.d->index, rhs.d->index);
+
+    using Qt::totally_ordered_wrapper;
+    return compareThreeWay(totally_ordered_wrapper{lhs.d}, totally_ordered_wrapper{rhs.d});
+}
+
+Qt::strong_ordering compareThreeWay(const QPersistentModelIndex &lhs,
+                                    const QModelIndex &rhs) noexcept
+{
+    return compareThreeWay(lhs.d ? lhs.d->index : QModelIndex{}, rhs);
+}
 
 /*!
     Sets the persistent model index to refer to the same item in a model
@@ -470,32 +481,26 @@ QPersistentModelIndex::operator QModelIndex() const
 }
 
 /*!
-    Returns \c{true} if this persistent model index refers to the same location as
+    \fn bool QPersistentModelIndex::operator==(const QPersistentModelIndex &lhs, const QModelIndex &rhs)
-    the \a other model index; otherwise returns \c{false}.
+    Returns \c{true} if \a lhs persistent model index refers to the same location as
+    the \a rhs model index; otherwise returns \c{false}.
 
     The internal data pointer, row, column, and model values in the persistent
     model index are used when comparing with another model index.
-*/
+ */
-
-bool QPersistentModelIndex::operator==(const QModelIndex &other) const noexcept
-{
-    if (d)
-        return d->index == other;
-    return !other.isValid();
-}
 
 /*!
-    \fn bool QPersistentModelIndex::operator!=(const QModelIndex &other) const
+    \fn bool QPersistentModelIndex::operator!=(const QPersistentModelIndex &lhs, const QModelIndex &rhs)
 
-    Returns \c{true} if this persistent model index does not refer to the same
+    Returns \c{true} if \a lhs persistent model index does not refer to the same
-    location as the \a other model index; otherwise returns \c{false}.
+    location as the \a rhs model index; otherwise returns \c{false}.
 */
 
-bool QPersistentModelIndex::operator!=(const QModelIndex &other) const noexcept
+bool comparesEqual(const QPersistentModelIndex &lhs, const QModelIndex &rhs) noexcept
 {
-    if (d)
+    if (lhs.d)
-        return d->index != other;
+        return lhs.d->index == rhs;
-    return other.isValid();
+    return !rhs.isValid();
 }
 
 /*!

src/corelib/itemmodels/qabstractitemmodel.h

@@ -178,17 +178,21 @@ public:
     QPersistentModelIndex(const QModelIndex &index);
     QPersistentModelIndex(const QPersistentModelIndex &other);
     ~QPersistentModelIndex();
+#if QT_CORE_REMOVED_SINCE(6, 8)
     bool operator<(const QPersistentModelIndex &other) const noexcept;
     bool operator==(const QPersistentModelIndex &other) const noexcept;
     inline bool operator!=(const QPersistentModelIndex &other) const noexcept
     { return !operator==(other); }
+#endif
     QPersistentModelIndex &operator=(const QPersistentModelIndex &other);
     inline QPersistentModelIndex(QPersistentModelIndex &&other) noexcept
         : d(std::exchange(other.d, nullptr)) {}
     QT_MOVE_ASSIGNMENT_OPERATOR_IMPL_VIA_PURE_SWAP(QPersistentModelIndex)
     void swap(QPersistentModelIndex &other) noexcept { qt_ptr_swap(d, other.d); }
+#if QT_CORE_REMOVED_SINCE(6, 8)
     bool operator==(const QModelIndex &other) const noexcept;
     bool operator!=(const QModelIndex &other) const noexcept;
+#endif
     QPersistentModelIndex &operator=(const QModelIndex &other);
     operator QModelIndex() const;
     int row() const;
@@ -208,6 +212,18 @@ private:
     friend size_t qHash(const QPersistentModelIndex &, size_t seed) noexcept;
     friend bool qHashEquals(const QPersistentModelIndex &a, const QPersistentModelIndex &b) noexcept
     { return a.d == b.d; }
+    friend Q_CORE_EXPORT bool
+    comparesEqual(const QPersistentModelIndex &lhs, const QPersistentModelIndex &rhs) noexcept;
+    friend Q_CORE_EXPORT bool
+    comparesEqual(const QPersistentModelIndex &lhs, const QModelIndex &rhs) noexcept;
+    friend Q_CORE_EXPORT Qt::strong_ordering // ### Qt 7: partial_ordering?
+    compareThreeWay(const QPersistentModelIndex &lhs, const QPersistentModelIndex &rhs) noexcept;
+    friend Q_CORE_EXPORT Qt::strong_ordering // ### Qt 7: partial_ordering?
+    compareThreeWay(const QPersistentModelIndex &lhs, const QModelIndex &rhs) noexcept;
+#if !QT_CORE_REMOVED_SINCE(6, 8)
+    Q_DECLARE_STRONGLY_ORDERED(QPersistentModelIndex)
+    Q_DECLARE_STRONGLY_ORDERED(QPersistentModelIndex, QModelIndex)
+#endif
 #ifndef QT_NO_DEBUG_STREAM
     friend Q_CORE_EXPORT QDebug operator<<(QDebug, const QPersistentModelIndex &);
 #endif

src/corelib/itemmodels/qitemselectionmodel.h

@@ -77,7 +77,7 @@ private:
     friend bool comparesEqual(const QItemSelectionRange &lhs,
                               const QItemSelectionRange &rhs) noexcept
     {
-        return (lhs.tl == rhs.tl && lhs.br == rhs.br);
+        return comparesEqual(lhs.tl, rhs.tl) && comparesEqual(lhs.br, rhs.br);
     }
     Q_DECLARE_EQUALITY_COMPARABLE(QItemSelectionRange)
     QPersistentModelIndex tl, br;

tests/auto/corelib/itemmodels/qabstractitemmodel/tst_qabstractitemmodel.cpp

@@ -994,8 +994,8 @@ void tst_QAbstractItemModel::complexChangesWithPersistent()
 void tst_QAbstractItemModel::modelIndexComparisons()
 {
     QTestPrivate::testAllComparisonOperatorsCompile<QModelIndex>();
-    QTestPrivate::testEqualityOperatorsCompile<QPersistentModelIndex>();
+    QTestPrivate::testAllComparisonOperatorsCompile<QPersistentModelIndex>();
-    QTestPrivate::testEqualityOperatorsCompile<QPersistentModelIndex, QModelIndex>();
+    QTestPrivate::testAllComparisonOperatorsCompile<QPersistentModelIndex, QModelIndex>();
 
     QtTestModel model(3, 3);
 
@@ -1003,6 +1003,7 @@ void tst_QAbstractItemModel::modelIndexComparisons()
     QModelIndex mi22 = model.index(2, 2);
     QPersistentModelIndex pmi11 = mi11;
     QPersistentModelIndex pmi22 = mi22;
+    QPersistentModelIndex pmiU;
 
     QT_TEST_EQUALITY_OPS(mi11, mi11, true);
     QT_TEST_EQUALITY_OPS(mi11, mi22, false);
@@ -1013,6 +1014,17 @@ void tst_QAbstractItemModel::modelIndexComparisons()
     QT_TEST_EQUALITY_OPS(pmi11, pmi22, false);
     QT_TEST_EQUALITY_OPS(pmi11, mi11, true);
     QT_TEST_EQUALITY_OPS(pmi11, mi22, false);
+
+    QT_TEST_ALL_COMPARISON_OPS(pmi11, pmi11, Qt::strong_ordering::equal);
+    QT_TEST_ALL_COMPARISON_OPS(pmi11, pmi22, Qt::strong_ordering::less);
+    // Disengaged QPMIs are sorted randomly (based on address of their Private)
+    // So all we can check here is QPMIs with d == nullptr, which should reliably
+    // come before any others.
+    QT_TEST_ALL_COMPARISON_OPS(pmiU, pmiU, Qt::strong_ordering::equal);
+    QT_TEST_ALL_COMPARISON_OPS(pmi11, pmiU, Qt::strong_ordering::greater);
+    QT_TEST_ALL_COMPARISON_OPS(pmi11, mi11, Qt::strong_ordering::equal);
+    QT_TEST_ALL_COMPARISON_OPS(pmi11, mi22, Qt::strong_ordering::less);
+    QT_TEST_ALL_COMPARISON_OPS(pmiU, mi11, Qt::strong_ordering::less);
 }
 
 void tst_QAbstractItemModel::testMoveSameParentDown_data()