From c2f4705f23ddccf075010edb0532fd73145b8b15 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Mon, 11 Jul 2016 11:14:34 +0200 Subject: [PATCH] Add qHash(QHash) and qHash(QMultiHash) The hash function is carefully designed to give the same result as the straight-forward implementation of qHash(unordered_map), which we'll probably add at some point, namely: std::accumulate over a container of std::pair. This is one reason to use std:: and not QPair in the implemen- tation of qHash(QHash). The other is that qHash(QPair) uses a bad hash combiner, which may xor out the 'seed' from the result. We can't fix that until Qt 6, but the qHash(std::pair) overload uses the well-known boost::hash_combine algorithm (implemented in Qt as QtPrivate::QHashCombine), so we can use that. I also trust std::pair to work without problems with reference template arguments, while QPair only very recently gained a very basic auto-test for reference parameters. [ChangeLog][QtCore] Added qHash() overloads for QHash, QMultiHash. Change-Id: I90879d8a99cf1aadb6e84ecc0c3704f52f3691da Reviewed-by: Thiago Macieira --- src/corelib/tools/qhash.cpp | 20 ++++++ src/corelib/tools/qhash.h | 21 +++++++ tests/auto/corelib/tools/qhash/tst_qhash.cpp | 64 ++++++++++++++++++++ 3 files changed, 105 insertions(+) diff --git a/src/corelib/tools/qhash.cpp b/src/corelib/tools/qhash.cpp index 593a87e65d..abec9ebb79 100644 --- a/src/corelib/tools/qhash.cpp +++ b/src/corelib/tools/qhash.cpp @@ -2644,4 +2644,24 @@ uint qHash(long double key, uint seed) Q_DECL_NOTHROW \sa QHash::constFind() */ +/*! + \fn uint qHash(const QHash &key, uint seed = 0) + \since 5.8 + \relates QHash + + Returns the hash value for the \a key, using \a seed to seed the calculation. + + Type \c T must be supported by qHash(). +*/ + +/*! + \fn uint qHash(const QMultiHash &key, uint seed = 0) + \since 5.8 + \relates QMultiHash + + Returns the hash value for the \a key, using \a seed to seed the calculation. + + Type \c T must be supported by qHash(). +*/ + QT_END_NAMESPACE diff --git a/src/corelib/tools/qhash.h b/src/corelib/tools/qhash.h index 7abbeabeae..6a2d7bdd11 100644 --- a/src/corelib/tools/qhash.h +++ b/src/corelib/tools/qhash.h @@ -1095,6 +1095,27 @@ Q_INLINE_TEMPLATE int QMultiHash::count(const Key &key, const T &value) Q_DECLARE_ASSOCIATIVE_ITERATOR(Hash) Q_DECLARE_MUTABLE_ASSOCIATIVE_ITERATOR(Hash) +template +uint qHash(const QHash &key, uint seed = 0) + Q_DECL_NOEXCEPT_EXPR(noexcept(qHash(std::declval())) && noexcept(qHash(std::declval()))) +{ + QtPrivate::QHashCombineCommutative hash; + for (auto it = key.begin(), end = key.end(); it != end; ++it) { + const Key &k = it.key(); + const T &v = it.value(); + seed = hash(seed, std::pair(k, v)); + } + return seed; +} + +template +inline uint qHash(const QMultiHash &key, uint seed = 0) + Q_DECL_NOEXCEPT_EXPR(noexcept(qHash(std::declval())) && noexcept(qHash(std::declval()))) +{ + const QHash &key2 = key; + return qHash(key2, seed); +} + QT_END_NAMESPACE #if defined(Q_CC_MSVC) diff --git a/tests/auto/corelib/tools/qhash/tst_qhash.cpp b/tests/auto/corelib/tools/qhash/tst_qhash.cpp index cfd2bdc6f7..0b864e71d4 100644 --- a/tests/auto/corelib/tools/qhash/tst_qhash.cpp +++ b/tests/auto/corelib/tools/qhash/tst_qhash.cpp @@ -49,6 +49,7 @@ private slots: void find(); // copied from tst_QMap void constFind(); // copied from tst_QMap void contains(); // copied from tst_QMap + void qhash(); void take(); // copied from tst_QMap void operator_eq(); // copied from tst_QMap void rehash_isnt_quadratic(); @@ -695,6 +696,69 @@ void tst_QHash::contains() QVERIFY(!map1.contains(43)); } +namespace { +class QGlobalQHashSeedResetter +{ + int oldSeed; +public: + // not entirely correct (may lost changes made by another thread between the query + // of the old and the setting of the new seed), but qSetGlobalQHashSeed doesn't + // return the old value, so this is the best we can do: + explicit QGlobalQHashSeedResetter(int newSeed) + : oldSeed(qGlobalQHashSeed()) + { + qSetGlobalQHashSeed(newSeed); + } + ~QGlobalQHashSeedResetter() + { + qSetGlobalQHashSeed(oldSeed); + } +}; + +template +QHash inverted(const QHash &in) +{ + QHash result; + for (auto it = in.begin(), end = in.end(); it != end; ++it) + result[it.value()] = it.key(); + return result; +} + +template +void make_test_data(AssociativeContainer &c) +{ + c["one"] = "1"; + c["two"] = "2"; +} + +} + +void tst_QHash::qhash() +{ + const QGlobalQHashSeedResetter seed1(0); + + QHash hash1; + make_test_data(hash1); + const QHash hsah1 = inverted(hash1); + + const QGlobalQHashSeedResetter seed2(1); + + QHash hash2; + make_test_data(hash2); + const QHash hsah2 = inverted(hash2); + + QCOMPARE(hash1, hash2); + QCOMPARE(hsah1, hsah2); + QCOMPARE(qHash(hash1), qHash(hash2)); + QCOMPARE(qHash(hsah1), qHash(hsah2)); + + // by construction this is almost impossible to cause false collisions: + QVERIFY(hash1 != hsah1); + QVERIFY(hash2 != hsah2); + QVERIFY(qHash(hash1) != qHash(hsah1)); + QVERIFY(qHash(hash2) != qHash(hsah2)); +} + //copied from tst_QMap void tst_QHash::take() {