From b786203f08075790e0ef7ae40ee5eebc336af9d5 Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Wed, 31 Jul 2024 16:41:02 +0200 Subject: [PATCH] Q(Multi)Hash: update (in)equality operators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use comparison helper macros for the inequality operators. Note that we cannot use the "public" macros, because we need to provide attributes for the comparison of the same type, so use the _HELPER macro. Convert the operators to hidden friends. We do not need to use QT_CORE_REMOVED_SINCE in this case, because the classes are templates, so the operators were not exported. Add the missing docs for the (in)equality operators of QMultiHash. gcc-10 complains about accessing const_iterator::i from a hidden friend function, claiming that it is private in that context: qtbase/src/corelib/tools/qhash.h:916:38: error: ‘QHash::const_iterator::piter QHash::const_iterator::i’ is private within this context if (i == lhs.end() || !i.i.node()->valuesEqual(it.i.node())) ~~^ Use a static helper method that does this comparison. Task-number: QTBUG-120305 Change-Id: I41b8889b659ae5dd858790a4ff629807e6666dc4 Reviewed-by: Thiago Macieira --- src/corelib/tools/qhash.cpp | 40 +++++++++-- src/corelib/tools/qhash.h | 69 +++++++++++-------- tests/auto/corelib/tools/qhash/CMakeLists.txt | 2 + tests/auto/corelib/tools/qhash/tst_qhash.cpp | 29 ++++++++ 4 files changed, 106 insertions(+), 34 deletions(-) diff --git a/src/corelib/tools/qhash.cpp b/src/corelib/tools/qhash.cpp index 438ac8da9ad..9456c423e2a 100644 --- a/src/corelib/tools/qhash.cpp +++ b/src/corelib/tools/qhash.cpp @@ -1510,6 +1510,7 @@ size_t qHash(long double key, size_t seed) noexcept \class QHash \inmodule QtCore \brief The QHash class is a template class that provides a hash-table-based dictionary. + \compares equality \ingroup tools \ingroup shared @@ -1806,10 +1807,10 @@ size_t qHash(long double key, size_t seed) noexcept fast and never fails. */ -/*! \fn template bool QHash::operator==(const QHash &other) const +/*! \fn template bool QHash::operator==(const QHash &lhs, const QHash &rhs) - Returns \c true if \a other is equal to this hash; otherwise returns - false. + Returns \c true if \a lhs hash is equal to \a rhs hash; otherwise returns + \c false. Two hashes are considered equal if they contain the same (key, value) pairs. @@ -1819,9 +1820,9 @@ size_t qHash(long double key, size_t seed) noexcept \sa operator!=() */ -/*! \fn template bool QHash::operator!=(const QHash &other) const +/*! \fn template bool QHash::operator!=(const QHash &lhs, const QHash &rhs) - Returns \c true if \a other is not equal to this hash; otherwise + Returns \c true if \a lhs hash is not equal to \a rhs hash; otherwise returns \c false. Two hashes are considered equal if they contain the same (key, @@ -2901,6 +2902,7 @@ size_t qHash(long double key, size_t seed) noexcept /*! \class QMultiHash \inmodule QtCore \brief The QMultiHash class is a convenience QHash subclass that provides multi-valued hashes. + \compares equality \ingroup tools \ingroup shared @@ -3125,6 +3127,34 @@ size_t qHash(long double key, size_t seed) noexcept \sa insert(), value() */ +/*! + \fn template bool QMultiHash::operator==(const QMultiHash &lhs, const QMultiHash &rhs) + + Returns \c true if \a lhs multihash equals to the \a rhs multihash; + otherwise returns \c false. + + Two multihashes are considered equal if they contain the same (key, value) + pairs. + + This function requires the value type to implement \c {operator==()}. + + \sa operator!=() +*/ + +/*! + \fn template bool QMultiHash::operator!=(const QMultiHash &lhs, const QMultiHash &rhs) + + Returns \c true if \a lhs multihash is not equal to the \a rhs multihash; + otherwise returns \c false. + + Two multihashes are considered equal if they contain the same (key, value) + pairs. + + This function requires the value type to implement \c {operator==()}. + + \sa operator==() +*/ + /*! \fn template QMultiHash &QMultiHash::operator+=(const QMultiHash &other) Inserts all the items in the \a other hash into this hash diff --git a/src/corelib/tools/qhash.h b/src/corelib/tools/qhash.h index 47bce5706b2..5fdab94eca9 100644 --- a/src/corelib/tools/qhash.h +++ b/src/corelib/tools/qhash.h @@ -900,29 +900,39 @@ public: #endif void swap(QHash &other) noexcept { qt_ptr_swap(d, other.d); } + class const_iterator; + #ifndef Q_QDOC - template - QTypeTraits::compare_eq_result_container operator==(const QHash &other) const noexcept +private: + static bool compareIterators(const const_iterator &lhs, const const_iterator &rhs) { - if (d == other.d) + return lhs.i.node()->valuesEqual(rhs.i.node()); + } + + template = true> + friend bool comparesEqual(const QHash &lhs, const QHash &rhs) noexcept + { + if (lhs.d == rhs.d) return true; - if (size() != other.size()) + if (lhs.size() != rhs.size()) return false; - for (const_iterator it = other.begin(); it != other.end(); ++it) { - const_iterator i = find(it.key()); - if (i == end() || !i.i.node()->valuesEqual(it.i.node())) + for (const_iterator it = rhs.begin(); it != rhs.end(); ++it) { + const_iterator i = lhs.find(it.key()); + if (i == lhs.end() || !compareIterators(i, it)) return false; } // all values must be the same as size is the same return true; } - template - QTypeTraits::compare_eq_result_container operator!=(const QHash &other) const noexcept - { return !(*this == other); } + QT_DECLARE_EQUALITY_OPERATORS_HELPER(QHash, QHash, /* non-constexpr */, noexcept, + template = true>) +public: #else - bool operator==(const QHash &other) const; - bool operator!=(const QHash &other) const; + friend bool operator==(const QHash &lhs, const QHash &rhs) noexcept; + friend bool operator!=(const QHash &lhs, const QHash &rhs) noexcept; #endif // Q_QDOC inline qsizetype size() const noexcept { return d ? qsizetype(d->size) : 0; } @@ -1098,8 +1108,6 @@ public: } QList values() const { return QList(begin(), end()); } - class const_iterator; - class iterator { using piter = typename QHashPrivate::iterator; @@ -1523,22 +1531,24 @@ public: } #ifndef Q_QDOC - template - QTypeTraits::compare_eq_result_container operator==(const QMultiHash &other) const noexcept +private: + template = true> + friend bool comparesEqual(const QMultiHash &lhs, const QMultiHash &rhs) noexcept { - if (d == other.d) + if (lhs.d == rhs.d) return true; - if (m_size != other.m_size) + if (lhs.m_size != rhs.m_size) return false; - if (m_size == 0) + if (lhs.m_size == 0) return true; // equal size, and both non-zero size => d pointers allocated for both - Q_ASSERT(d); - Q_ASSERT(other.d); - if (d->size != other.d->size) + Q_ASSERT(lhs.d); + Q_ASSERT(rhs.d); + if (lhs.d->size != rhs.d->size) return false; - for (auto it = other.d->begin(); it != other.d->end(); ++it) { - auto *n = d->findNode(it.node()->key); + for (auto it = rhs.d->begin(); it != rhs.d->end(); ++it) { + auto *n = lhs.d->findNode(it.node()->key); if (!n) return false; Chain *e = it.node()->value; @@ -1557,12 +1567,13 @@ public: // all values must be the same as size is the same return true; } - template - QTypeTraits::compare_eq_result_container operator!=(const QMultiHash &other) const noexcept - { return !(*this == other); } + QT_DECLARE_EQUALITY_OPERATORS_HELPER(QMultiHash, QMultiHash, /* non-constexpr */, noexcept, + template = true>) +public: #else - bool operator==(const QMultiHash &other) const; - bool operator!=(const QMultiHash &other) const; + friend bool operator==(const QMultiHash &lhs, const QMultiHash &rhs) noexcept; + friend bool operator!=(const QMultiHash &lhs, const QMultiHash &rhs) noexcept; #endif // Q_QDOC inline qsizetype size() const noexcept { return m_size; } diff --git a/tests/auto/corelib/tools/qhash/CMakeLists.txt b/tests/auto/corelib/tools/qhash/CMakeLists.txt index 8702b8bf23b..778f09286fc 100644 --- a/tests/auto/corelib/tools/qhash/CMakeLists.txt +++ b/tests/auto/corelib/tools/qhash/CMakeLists.txt @@ -14,6 +14,8 @@ endif() qt_internal_add_test(tst_qhash SOURCES tst_qhash.cpp + LIBRARIES + Qt::TestPrivate ) qt_internal_undefine_global_definition(tst_qhash QT_NO_JAVA_STYLE_ITERATORS) diff --git a/tests/auto/corelib/tools/qhash/tst_qhash.cpp b/tests/auto/corelib/tools/qhash/tst_qhash.cpp index b3dbdfa40c4..742eac7056c 100644 --- a/tests/auto/corelib/tools/qhash/tst_qhash.cpp +++ b/tests/auto/corelib/tools/qhash/tst_qhash.cpp @@ -16,6 +16,8 @@ #include +#include + using namespace Qt::StringLiterals; class tst_QHash : public QObject @@ -37,6 +39,7 @@ private slots: void contains(); // copied from tst_QMap void qhash(); void take(); // copied from tst_QMap + void comparisonCompiles(); void operator_eq(); // slightly modified from tst_QMap void heterogeneousSearch(); void heterogeneousSearchConstKey(); @@ -1025,6 +1028,16 @@ void tst_QHash::take() } } +void tst_QHash::comparisonCompiles() +{ + QTestPrivate::testEqualityOperatorsCompile>(); + QTestPrivate::testEqualityOperatorsCompile>(); + QTestPrivate::testEqualityOperatorsCompile>(); + QTestPrivate::testEqualityOperatorsCompile>(); + QTestPrivate::testEqualityOperatorsCompile>(); + QTestPrivate::testEqualityOperatorsCompile>(); +} + // slightly modified from tst_QMap void tst_QHash::operator_eq() { @@ -1035,29 +1048,35 @@ void tst_QHash::operator_eq() QVERIFY(a == b); QVERIFY(!(a != b)); + QT_TEST_EQUALITY_OPS(a, b, true); a.insert(1,1); b.insert(1,1); QVERIFY(a == b); QVERIFY(!(a != b)); + QT_TEST_EQUALITY_OPS(a, b, true); a.insert(0,1); b.insert(0,1); QVERIFY(a == b); QVERIFY(!(a != b)); + QT_TEST_EQUALITY_OPS(a, b, true); // compare for inequality: a.insert(42,0); QVERIFY(a != b); QVERIFY(!(a == b)); + QT_TEST_EQUALITY_OPS(a, b, false); a.insert(65, -1); QVERIFY(a != b); QVERIFY(!(a == b)); + QT_TEST_EQUALITY_OPS(a, b, false); b.insert(-1, -1); QVERIFY(a != b); QVERIFY(!(a == b)); + QT_TEST_EQUALITY_OPS(a, b, false); } { @@ -1067,18 +1086,22 @@ void tst_QHash::operator_eq() QVERIFY(a == b); QVERIFY(!(a != b)); + QT_TEST_EQUALITY_OPS(a, b, true); a.insert("Hello", "World"); QVERIFY(a != b); QVERIFY(!(a == b)); + QT_TEST_EQUALITY_OPS(a, b, false); b.insert("Hello", "World"); QVERIFY(a == b); QVERIFY(!(a != b)); + QT_TEST_EQUALITY_OPS(a, b, true); a.insert("Goodbye", "cruel world"); QVERIFY(a != b); QVERIFY(!(a == b)); + QT_TEST_EQUALITY_OPS(a, b, false); b.insert("Goodbye", "cruel world"); @@ -1086,11 +1109,13 @@ void tst_QHash::operator_eq() a.insert(QString(), QString()); QVERIFY(a != b); QVERIFY(!(a == b)); + QT_TEST_EQUALITY_OPS(a, b, false); // empty keys and null keys match: b.insert(QString(""), QString()); QVERIFY(a == b); QVERIFY(!(a != b)); + QT_TEST_EQUALITY_OPS(a, b, true); } { @@ -1101,6 +1126,7 @@ void tst_QHash::operator_eq() b.insert("willy", 1); QVERIFY(a != b); QVERIFY(!(a == b)); + QT_TEST_EQUALITY_OPS(a, b, false); } // unlike multi-maps, multi-hashes should be equal iff their contents are equal, @@ -1118,6 +1144,7 @@ void tst_QHash::operator_eq() QVERIFY(a == b); QVERIFY(!(a != b)); + QT_TEST_EQUALITY_OPS(a, b, true); } { @@ -1138,6 +1165,7 @@ void tst_QHash::operator_eq() QVERIFY(a == b); QVERIFY(!(a != b)); + QT_TEST_EQUALITY_OPS(a, b, true); } { @@ -1166,6 +1194,7 @@ void tst_QHash::operator_eq() QVERIFY(a == b); QVERIFY(!(a != b)); + QT_TEST_EQUALITY_OPS(a, b, true); } }