From 162015e9c6f469951d9212ef655cff16dcace071 Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@qt.io>
Date: Thu, 2 Jan 2025 08:31:21 +0100
Subject: [PATCH] Unbreak QSet::intersect()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The selection of which set to iterate over and which one to remove
from based on their relative size violates the function's
documentation, which clearly states that items are removed from *this,
and not from `other`, so the result must never contain any elements
from `other`.

Amends 4f2c96eaa8bfa4d8a6dfb92096e4e4030d0cdea7. Instead of reverting
to the gruesome old code with the forced detach-just-to-remove copies,
distinguish four cases:

- if the two sets are shallow copies of each other, then their
  intersection is *this

- otherwise, if either set is empty, clear() *this. This is required
  for one of the tests that 29017f1395b1bc52e60760fa58c92f6fa4ee4f60
  added to succeed.

- otherwise, if *this is detached, perform the operation in-place,
  using removeIf()

- otherwise, create a new set and move-assign to *this to avoid
  detaching just to remove something again. In this case, we can
  continue to iterate over the smaller set, but we need to keep
  picking elements from LHS into the result.

[ChangeLog][QtCore][QSet] Fixed a regression (introduced for Qt 5.2)
in intersect() that caused equivalent elements of `*this` to be
overwritten by elements of `other` if `other.size()` was larger than
`this->size()`.

Not picking to 5.15, as users will have likely adjusted their code to
the buggy behavior, and because removeIf() isn't available there.

Pick-to: 6.9 6.8 6.5
Fixes: QTBUG-132536
Task-number: QTBUG-106179
Change-Id: Idfa17c3b3589c4eacec27259fc01df6aeaa6c45f
Reviewed-by: Øystein Heskestad <oystein.heskestad@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
 src/corelib/tools/qset.h                   | 54 +++++++++++++++++-----
 tests/auto/corelib/tools/qset/tst_qset.cpp |  2 -
 2 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/src/corelib/tools/qset.h b/src/corelib/tools/qset.h
index 3cba1a2b15d..d8857925f7d 100644
--- a/src/corelib/tools/qset.h
+++ b/src/corelib/tools/qset.h
@@ -209,6 +209,8 @@ public:
     QList<T> values() const;
 
 private:
+    static inline QSet intersected_helper(const QSet &lhs, const QSet &rhs);
+
     Hash q_hash;
 };
 
@@ -242,23 +244,51 @@ Q_INLINE_TEMPLATE QSet<T> &QSet<T>::unite(const QSet<T> &other)
 template <class T>
 Q_INLINE_TEMPLATE QSet<T> &QSet<T>::intersect(const QSet<T> &other)
 {
-    QSet<T> copy1;
-    QSet<T> copy2;
-    if (size() <= other.size()) {
-        copy1 = *this;
-        copy2 = other;
+    if (q_hash.isSharedWith(other.q_hash)) {
+        // nothing to do
+    } else if (isEmpty() || other.isEmpty()) {
+        // any set intersected with the empty set is the empty set
+        clear();
+    } else if (q_hash.isDetached()) {
+        // do it in-place:
+        removeIf([&other] (const T &e) { return !other.contains(e); });
     } else {
-        copy1 = other;
-        copy2 = *this;
-        *this = copy1;
-    }
-    for (const auto &e : std::as_const(copy1)) {
-        if (!copy2.contains(e))
-            remove(e);
+        // don't detach *this just to remove some items; create a new set
+        *this = intersected_helper(*this, other);
     }
     return *this;
 }
 
+template <class T>
+// static
+auto QSet<T>::intersected_helper(const QSet &lhs, const QSet &rhs) -> QSet
+{
+    QSet r;
+
+    const auto l_size = lhs.size();
+    const auto r_size = rhs.size();
+    r.reserve((std::min)(l_size, r_size));
+
+    // Iterate the smaller of the two sets, but always take from lhs, for
+    // consistency with insert():
+
+    if (l_size <= r_size) {
+        // lhs is not larger
+        for (const auto &e : lhs) {
+            if (rhs.contains(e))
+                r.insert(e);
+        }
+    } else {
+        // rhs is smaller
+        for (const auto &e : rhs) {
+            if (const auto it = lhs.find(e); it != lhs.end())
+                r.insert(*it);
+        }
+    }
+
+    return r;
+}
+
 template <class T>
 Q_INLINE_TEMPLATE bool QSet<T>::intersects(const QSet<T> &other) const
 {
diff --git a/tests/auto/corelib/tools/qset/tst_qset.cpp b/tests/auto/corelib/tools/qset/tst_qset.cpp
index 4e6dbab9f12..d28d9c43a5e 100644
--- a/tests/auto/corelib/tools/qset/tst_qset.cpp
+++ b/tests/auto/corelib/tools/qset/tst_qset.cpp
@@ -1304,11 +1304,9 @@ void tst_QSet::setOperationsPickEquivalentElementsFromLHSContainer_impl()
 
 
         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));