From 0e17856f6355541b7692efce77d4ceee8ea1f1eb Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 2 Jan 2025 08:31:21 +0100 Subject: [PATCH] Revert "Optimize QSet::unite" This reverts commit 92acc94fa98d19929d28feb5d543acf8c50c2290. The change broke QSet ordering guarantees: The documentation clearly states that each item from `other` that isn't already in `*this` is inserted ("STL insertion behavior"). Swapping *this and other breaks this. Independent of STL vs. Qt insertion behavior, making the picking of elements from containers "random" in the sense that the size of the container is now important, and not merely LHS vs. RHS, is a bad idea. [ChangeLog][QtCore][QSet] Fixed a regression in unite() that caused equivalent elements of `*this` to be overwritten by elements of `other` if `other.size()` was larger than `this->size()`. Fixes: QTBUG-132500 Change-Id: Ia636b62325139d618b5467a643ff710716324296 Reviewed-by: Thiago Macieira Reviewed-by: Edward Welbourne (cherry picked from commit 2d1b3028673493cb144060cbec49b1b95f4188d2) Reviewed-by: Qt Cherry-pick Bot (cherry picked from commit 3dff1a3f7b22bc623b3c326630bfd94d2bd35d08) --- src/corelib/tools/qset.h | 11 ++++------- tests/auto/corelib/tools/qset/tst_qset.cpp | 4 +--- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/corelib/tools/qset.h b/src/corelib/tools/qset.h index d403a0416e..deccaaf387 100644 --- a/src/corelib/tools/qset.h +++ b/src/corelib/tools/qset.h @@ -229,13 +229,10 @@ Q_INLINE_TEMPLATE void QSet::reserve(qsizetype asize) { q_hash.reserve(asize) template Q_INLINE_TEMPLATE QSet &QSet::unite(const QSet &other) { - if (q_hash.isSharedWith(other.q_hash)) - return *this; - QSet tmp = other; - if (size() < other.size()) - swap(tmp); - for (const auto &e : std::as_const(tmp)) - insert(e); + if (!q_hash.isSharedWith(other.q_hash)) { + for (const T &e : other) + insert(e); + } return *this; } diff --git a/tests/auto/corelib/tools/qset/tst_qset.cpp b/tests/auto/corelib/tools/qset/tst_qset.cpp index 2bb300a0c5..34e9334296 100644 --- a/tests/auto/corelib/tools/qset/tst_qset.cpp +++ b/tests/auto/corelib/tools/qset/tst_qset.cpp @@ -898,7 +898,7 @@ void tst_QSet::setOperationsOnEmptySet() empty.unite(nonEmpty); QCOMPARE(empty, nonEmpty); - QVERIFY(!empty.isDetached()); + QVERIFY(empty.isDetached()); } } @@ -1312,11 +1312,9 @@ void tst_QSet::setOperationsPickEquivalentElementsFromLHSContainer_impl() // (unlike other Qt containers, QSet's insertion behavior is STL-compliant): // QVERIFY(lhsCopy.contains(OneL)); - QEXPECT_FAIL("", "QTBUG-132500", Continue); QCOMPARE(lhsCopy.find(OneL)->id, OneL.id); QVERIFY(lhsCopy.contains(TwoL)); - QEXPECT_FAIL("", "QTBUG-132500", Continue); QCOMPARE(lhsCopy.find(TwoL)->id, TwoL.id); QVERIFY(lhsCopy.contains(ThreeL));