QSet: check that unite() and intersect() prefer elements from *this

In Qt versions 5.0..6.7, at least, this used to be the implemented
behavior for unite(). For intersect(), it was implemented up to and
excl. 5.2. Even the documentation states as much:

intersect(): Removes all items from _this set_ that are not
             contained in the other set.

unite(): Each item in the other set that isn't already in this set
         is inserted into this set.

Add checks that the functions behave as documented (hint: they no
longer do, since 6.8 (unite()) and 5.2 (intersect()), resp.), this
being the only correct way to implement these functions (items in
sets may be equivalent, but not identical; it is important that the
set operations work in a manner consistent with insert(), to meet
user's expectation of how these functions work (unite() just
inserts() the rhs, intersect() removes if !rhs.contains). Note that
QSet, unlike other Qt associative containers, actually has STL-style
insertion behavior (insert() doesn't overwrite).

subtract() is the only one that's still true to its docs. A test for
this function will be added in a follow-up commit - eventually.

In anticipation of adding rvalue-other overloads of at least
unite(), add a test with that, too.

Pick-to: 6.8 6.5 6.2 5.15
Task-number: QTBUG-132500
Task-number: QTBUG-132536
Change-Id: Id309ab5192e6d1c9bbeef496cbd7116d306eaae8
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
(cherry picked from commit 0d4fd5c545b47966c56ed0b9eb9b5c0a8f75c02a)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Marc Mutz 2025-01-02 09:22:23 +01:00 committed by Qt Cherry-pick Bot
parent 0048e8de0e
commit 12f39bb2ab

View File

@ -52,6 +52,12 @@ private slots:
void intersects(); void intersects();
void find(); void find();
void values(); void values();
void setOperationsPickEquivalentElementsFromLHSContainer_data();
void setOperationsPickEquivalentElementsFromLHSContainer();
private:
template <bool RhsRValue>
void setOperationsPickEquivalentElementsFromLHSContainer_impl();
}; };
struct IdentityTracker { struct IdentityTracker {
@ -1217,6 +1223,139 @@ void tst_QSet::values()
QCOMPARE(sorted(set.values()), QList<int>({ 1, 2, 10 })); QCOMPARE(sorted(set.values()), QList<int>({ 1, 2, 10 }));
} }
void tst_QSet::setOperationsPickEquivalentElementsFromLHSContainer_data()
{
QTest::addColumn<bool>("lhsShared");
QTest::addColumn<bool>("rhsShared");
QTest::addColumn<bool>("rhsRValue");
for (bool lhs : {true, false}) {
for (bool rhs : {true, false}) {
for (bool rv : {true, false}) {
QTest::addRow("lhs %sshared, rhs %sshared %cvalue",
lhs ? "" : "not ", rhs ? "" : "not ", rv ? 'r' : 'l')
<< lhs << rhs << rv;
}
}
}
}
void tst_QSet::setOperationsPickEquivalentElementsFromLHSContainer()
{
QFETCH(const bool, rhsRValue);
if (rhsRValue)
setOperationsPickEquivalentElementsFromLHSContainer_impl<true>();
else
setOperationsPickEquivalentElementsFromLHSContainer_impl<false>();
}
template <bool RhsRValue>
void tst_QSet::setOperationsPickEquivalentElementsFromLHSContainer_impl()
{
QFETCH(const bool, lhsShared);
QFETCH(const bool, rhsShared);
//
// GIVEN: Two equivalent QSets:
//
constexpr IdentityTracker OneL{1, 1}, TwoL{2, 1}, ThreeL{3, 1};
constexpr IdentityTracker OneR{1, 2}, TwoR{2, 2}, ThreeR{3, 2};
// by construction:
QCOMPARE(OneL, OneR);
QCOMPARE(qHash(OneL), qHash(OneR));
const QSet<IdentityTracker> lhs = {OneL, TwoL};
const QSet<IdentityTracker> rhs = {OneR, TwoR, ThreeR};
auto maybeDetach = [](auto set, bool shared) {
if (!shared)
set.detach();
return set;
};
{
//
// WHEN: intersect()ing the two
//
auto lhsCopy = maybeDetach(lhs, lhsShared);
auto rhsCopy = maybeDetach(rhs, rhsShared);
if constexpr (RhsRValue) {
lhsCopy.intersect(maybeDetach(rhs, rhsShared));
rhsCopy.intersect(maybeDetach(lhs, lhsShared));
} else {
lhsCopy.intersect(rhs);
rhsCopy.intersect(lhs);
}
//
// THEN: equivalent elements are picked from the LHS container:
// (unlike other Qt containers, QSet's insertion behavior is STL-compliant):
//
QVERIFY(lhsCopy.contains(OneL));
QCOMPARE(lhsCopy.find(OneL)->id, OneL.id);
QVERIFY(lhsCopy.contains(TwoL));
QCOMPARE(lhsCopy.find(TwoL)->id, TwoL.id);
QVERIFY(!lhsCopy.contains(ThreeR));
QVERIFY(rhsCopy.contains(OneR));
QEXPECT_FAIL("", "QTBUG-132536", Continue);
QCOMPARE(rhsCopy.find(OneR)->id, OneR.id);
QVERIFY(rhsCopy.contains(TwoR));
QEXPECT_FAIL("", "QTBUG-132536", Continue);
QCOMPARE(rhsCopy.find(TwoR)->id, TwoR.id);
QVERIFY(!rhsCopy.contains(ThreeR));
}
{
//
// WHEN: unite()ing the two
//
auto lhsCopy = maybeDetach(lhs, lhsShared);
auto rhsCopy = maybeDetach(rhs, rhsShared);
if constexpr (RhsRValue) {
lhsCopy.unite(maybeDetach(rhs, rhsShared));
rhsCopy.unite(maybeDetach(lhs, lhsShared));
} else {
lhsCopy.unite(rhs);
rhsCopy.unite(lhs);
}
//
// THEN: equivalent elements are picked from the LHS container:
// (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));
QCOMPARE(lhsCopy.find(ThreeL)->id, ThreeR.id); // element was not contained in lhs
QVERIFY(rhsCopy.contains(OneR));
QCOMPARE(rhsCopy.find(OneR)->id, OneR.id);
QVERIFY(rhsCopy.contains(TwoR));
QCOMPARE(rhsCopy.find(TwoR)->id, TwoR.id);
QVERIFY(rhsCopy.contains(ThreeR));
QCOMPARE(rhsCopy.find(ThreeR)->id, ThreeR.id);
}
}
QTEST_APPLESS_MAIN(tst_QSet) QTEST_APPLESS_MAIN(tst_QSet)
#include "tst_qset.moc" #include "tst_qset.moc"