From cbda9583521633fa6e8a9274d2e47aa14c8bd175 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Wed, 22 Nov 2023 13:23:31 +0100 Subject: [PATCH] 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 --- src/corelib/compat/removed_api.cpp | 22 +++++ src/corelib/itemmodels/qabstractitemmodel.cpp | 93 ++++++++++--------- src/corelib/itemmodels/qabstractitemmodel.h | 16 ++++ src/corelib/itemmodels/qitemselectionmodel.h | 2 +- .../tst_qabstractitemmodel.cpp | 16 +++- 5 files changed, 102 insertions(+), 47 deletions(-) diff --git a/src/corelib/compat/removed_api.cpp b/src/corelib/compat/removed_api.cpp index 16f14c45790..2210cae848a 100644 --- a/src/corelib/compat/removed_api.cpp +++ b/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 diff --git a/src/corelib/itemmodels/qabstractitemmodel.cpp b/src/corelib/itemmodels/qabstractitemmodel.cpp index 629ee9582af..216db3b42a5 100644 --- a/src/corelib/itemmodels/qabstractitemmodel.cpp +++ b/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 - 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 -{ - 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 + \fn bool QPersistentModelIndex::operator==(const QPersistentModelIndex &lhs, const QPersistentModelIndex &rhs) + Returns \c{true} if \a lhs persistent model index is equal to the \a rhs 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 -{ - if (d && other.d) - return d->index < other.d->index; +/*! + \fn bool QPersistentModelIndex::operator!=(const QPersistentModelIndex &lhs, const QPersistentModelIndex &rhs) + \since 4.2 - 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 - \since 4.2 + \fn bool QPersistentModelIndex::operator<(const QPersistentModelIndex &lhs, const QPersistentModelIndex &rhs) + \since 4.1 - Returns \c{true} if this persistent model index is not equal to the \a - other persistent model index; otherwise returns \c{false}. + Returns \c{true} if \a lhs persistent model index is smaller than the \a rhs + 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 - the \a other model index; otherwise returns \c{false}. + \fn bool QPersistentModelIndex::operator==(const QPersistentModelIndex &lhs, const QModelIndex &rhs) + 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 - location as the \a other model index; otherwise returns \c{false}. + Returns \c{true} if \a lhs persistent model index does not refer to the same + 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) - return d->index != other; - return other.isValid(); + if (lhs.d) + return lhs.d->index == rhs; + return !rhs.isValid(); } /*! diff --git a/src/corelib/itemmodels/qabstractitemmodel.h b/src/corelib/itemmodels/qabstractitemmodel.h index 1e9c58334de..418fc6b864f 100644 --- a/src/corelib/itemmodels/qabstractitemmodel.h +++ b/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 diff --git a/src/corelib/itemmodels/qitemselectionmodel.h b/src/corelib/itemmodels/qitemselectionmodel.h index c4b8dadc97b..aaf32e68ef7 100644 --- a/src/corelib/itemmodels/qitemselectionmodel.h +++ b/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; diff --git a/tests/auto/corelib/itemmodels/qabstractitemmodel/tst_qabstractitemmodel.cpp b/tests/auto/corelib/itemmodels/qabstractitemmodel/tst_qabstractitemmodel.cpp index 3c46c519f88..c48b79b2600 100644 --- a/tests/auto/corelib/itemmodels/qabstractitemmodel/tst_qabstractitemmodel.cpp +++ b/tests/auto/corelib/itemmodels/qabstractitemmodel/tst_qabstractitemmodel.cpp @@ -994,8 +994,8 @@ void tst_QAbstractItemModel::complexChangesWithPersistent() void tst_QAbstractItemModel::modelIndexComparisons() { QTestPrivate::testAllComparisonOperatorsCompile(); - QTestPrivate::testEqualityOperatorsCompile(); - QTestPrivate::testEqualityOperatorsCompile(); + QTestPrivate::testAllComparisonOperatorsCompile(); + QTestPrivate::testAllComparisonOperatorsCompile(); 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()