From 68cac07d500851209eab93dec771c45ac4864c8a Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 14 Jan 2025 17:08:14 +0100 Subject: [PATCH] QSet: add an rvalue overload of unite() The old QSet::unite() overload cannot really be optimized, because the RHS set was passed by cref, so cannot be modified, even if it were already detached. If we could, we could insert-or-overwrite from the LHS into the RHS and then swap the two, if the RHS was larger than the LHS. This was the original idea of 92acc94fa98d19929d28feb5d543acf8c50c2290, which, however, ignored the implicit detach of the larger set if it was shared. So add a unite(QSet&&) overload that, when called, does have the ability to change the RHS, incl. using it as the base for the result. We just need a way to insert_or_assign into a QSet, which is only possible using private API, because QHash never overwrites a once-inserted key, and that's exactly what we need to perform this operation while staying consistent with the QSet insertion behavior. [ChangeLog][QtCore][QSet] Added unite(QSet&&) overload of unite(const QSet&). Ditto for the `+` and `|` operators. Task-number: QTBUG-132500 Change-Id: I31592d7bec3b0702e746b69e4f024708ad6a3cc4 Reviewed-by: Allan Sandfeld Jensen --- src/corelib/tools/qset.h | 52 +++++++++++++++++++++++++++++++++++++ src/corelib/tools/qset.qdoc | 7 +++++ 2 files changed, 59 insertions(+) diff --git a/src/corelib/tools/qset.h b/src/corelib/tools/qset.h index d8857925f7d..d8d6e39dfbd 100644 --- a/src/corelib/tools/qset.h +++ b/src/corelib/tools/qset.h @@ -164,6 +164,7 @@ public: const_iterator find(const T &value) const { return q_hash.find(value); } inline const_iterator constFind(const T &value) const { return find(value); } QSet &unite(const QSet &other); + QSet &unite(QSet &&other); QSet &intersect(const QSet &other); bool intersects(const QSet &other) const; QSet &subtract(const QSet &other); @@ -185,23 +186,29 @@ public: // comfort inline QSet &operator<<(const T &value) { insert(value); return *this; } inline QSet &operator|=(const QSet &other) { unite(other); return *this; } + QSet &operator|=(QSet &&other) { return unite(std::move(other)); } inline QSet &operator|=(const T &value) { insert(value); return *this; } inline QSet &operator&=(const QSet &other) { intersect(other); return *this; } inline QSet &operator&=(const T &value) { QSet result; if (contains(value)) result.insert(value); return (*this = result); } inline QSet &operator+=(const QSet &other) { unite(other); return *this; } + QSet &operator+=(QSet &&other) { return unite(std::move(other)); } inline QSet &operator+=(const T &value) { insert(value); return *this; } inline QSet &operator-=(const QSet &other) { subtract(other); return *this; } inline QSet &operator-=(const T &value) { remove(value); return *this; } friend QSet operator|(const QSet &lhs, const QSet &rhs) { return QSet(lhs) |= rhs; } friend QSet operator|(QSet &&lhs, const QSet &rhs) { lhs |= rhs; return std::move(lhs); } + friend QSet operator|(const QSet &lhs, QSet &&rhs) { return lhs |= std::move(rhs); } + friend QSet operator|(QSet &&lhs, QSet &&rhs) { return std::move(lhs) |= std::move(rhs); } friend QSet operator&(const QSet &lhs, const QSet &rhs) { return QSet(lhs) &= rhs; } friend QSet operator&(QSet &&lhs, const QSet &rhs) { lhs &= rhs; return std::move(lhs); } friend QSet operator+(const QSet &lhs, const QSet &rhs) { return QSet(lhs) += rhs; } friend QSet operator+(QSet &&lhs, const QSet &rhs) { lhs += rhs; return std::move(lhs); } + friend QSet operator+(const QSet &lhs, QSet &&rhs) { return lhs += std::move(rhs); } + friend QSet operator+(QSet &&lhs, QSet &&rhs) { return std::move(lhs) += std::move(rhs); } friend QSet operator-(const QSet &lhs, const QSet &rhs) { return QSet(lhs) -= rhs; } friend QSet operator-(QSet &&lhs, const QSet &rhs) { lhs -= rhs; return std::move(lhs); } @@ -211,6 +218,9 @@ public: private: static inline QSet intersected_helper(const QSet &lhs, const QSet &rhs); + template + void _emplace_or_overwrite(E &&e); + Hash q_hash; }; @@ -241,6 +251,48 @@ Q_INLINE_TEMPLATE QSet &QSet::unite(const QSet &other) return *this; } +template +Q_INLINE_TEMPLATE auto QSet::unite(QSet &&other) -> QSet& +{ + if (other.isDetached() && size() < other.size()) { + + // We can change the state of `other`, so take the smaller *this and + // insert it into the larger `other`, making sure we take equivalent + // elements from *this: + + swap(other); + + // Now: iterate over `other`, insert into *this, making sure we take + // equivalent elements from `other`: + + if (other.isDetached()) { // can move elements from `other` + for (auto &e : other) + _emplace_or_overwrite(std::move(e)); + } else { // need to copy elements from `other` + for (const auto &e : std::as_const(other)) + _emplace_or_overwrite(e); + } + + return *this; + } + + // in all other cases, the lvalue overload is not worse: + return unite(other); +} + +template +template +Q_INLINE_TEMPLATE void QSet::_emplace_or_overwrite(E &&e) +{ + const auto r = q_hash.tryEmplace(std::forward(e)); + if (!r.inserted) { + // QHash never overwrites the key, but that's what we need + // here, so do it using private QHash API: + // NB: `e` was _not_ moved from by tryEmplace()! + typename Hash::Data::Bucket(r.iterator.i).node()->key = std::forward(e); + } +} + template Q_INLINE_TEMPLATE QSet &QSet::intersect(const QSet &other) { diff --git a/src/corelib/tools/qset.qdoc b/src/corelib/tools/qset.qdoc index 99958596c73..86328add1b6 100644 --- a/src/corelib/tools/qset.qdoc +++ b/src/corelib/tools/qset.qdoc @@ -445,6 +445,7 @@ /*! \fn template QSet &QSet::unite(const QSet &other) + \fn template QSet &QSet::unite(QSet &&other) Each item in the \a other set that isn't already in this set is inserted into this set. A reference to this set is returned. @@ -532,7 +533,9 @@ /*! \fn template QSet &QSet::operator|=(const QSet &other) + \fn template QSet &QSet::operator|=(QSet &&other) \fn template QSet &QSet::operator+=(const QSet &other) + \fn template QSet &QSet::operator+=(QSet &&other) Same as \l {unite()} {unite(\a other)}. @@ -567,9 +570,13 @@ /*! \fn template QSet QSet::operator|(const QSet &lhs, const QSet &rhs) + \fn template QSet QSet::operator|(const QSet &lhs, QSet &&rhs) \fn template QSet QSet::operator|(QSet &&lhs, const QSet &rhs) + \fn template QSet QSet::operator|(QSet &&lhs, QSet &&rhs) \fn template QSet QSet::operator+(const QSet &lhs, const QSet &rhs) + \fn template QSet QSet::operator+(const QSet &lhs, QSet &&rhs) \fn template QSet QSet::operator+(QSet &&lhs, const QSet &rhs) + \fn template QSet QSet::operator+(QSet &&lhs, QSet &&rhs) Returns a new QSet that is the union of sets \a lhs and \a rhs.