QSet: don't detach in remove()/removeIf() if nothing is being removed

QSet::remove() calls QHash::remove(), which unconditionally detaches,
so fixing the detach there, by pulling the isUnused() check to before
detach(), fixes it for both.

For removeIf(), the old code used begin()/end() for iteration,
detaching unconditionally, even if nothing was removed in the end.

In this case, fix by using cbegin()/cend(). That delays the detach()
until the first erase(), where it belongs. The end() iterator of QHash
(and therefore QSet) are stateless sentinels in all but name, so we can
continue to cache end(). Add a code comment to that effect.

Amends 62dad9be9e (for removeIf()) and
5b7c3e31b5 (for remove()).

[ChangeLog][QtCore][QSet] remove() and removeIf() no longer
unconditionally detach, but only if something is actually being
removed.

[ChangeLog][QtCore][QHash] remove() no longer unconditionally
detaches, but only if something is actually being removed.

Pick-to: 6.5
Task-number: QTBUG-106179
Fixes: QTBUG-132831
Change-Id: I807577eafa1be478b0a2da45cf8c44936d5e48ed
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
(cherry picked from commit 3a284dc19d1c77692f2faddedc674cc293d51a00)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 605a20b2624dac1bfc4a4cb683700db5026bdbe8)

src/corelib/tools/qcontainertools_impl.h

@@ -398,12 +398,13 @@ template <typename T, typename Predicate>
 qsizetype qset_erase_if(QSet<T> &set, Predicate &pred)
 {
     qsizetype result = 0;
-    auto it = set.begin();
-    const auto e = set.end();
+    auto it = set.cbegin();
+    auto e = set.cend(); // stable across detach (QHash::end() is a stateless sentinel)...
     while (it != e) {
         if (pred(*it)) {
             ++result;
             it = set.erase(it);
+            e = set.cend(); // ...but re-set nonetheless, in case at some point it won't be
         } else {
             ++it;
         }

src/corelib/tools/qhash.h

@@ -975,12 +975,13 @@ private:
         if (isEmpty()) // prevents detaching shared null
             return false;
         auto it = d->findBucket(key);
+        if (it.isUnused())
+            return false;
+
         size_t bucket = it.toBucketIndex(d);
         detach();
         it = typename Data::Bucket(d, bucket); // reattach in case of detach
 
-        if (it.isUnused())
-            return false;
         d->erase(it);
         return true;
     }

tests/auto/corelib/tools/qset/tst_qset.cpp

@@ -379,17 +379,9 @@ void tst_QSet::removeOnlyDetachesIfSomethingGetsRemoved()
     QVERIFY(!copy.isDetached());
 
     QVERIFY(!copy.remove(42));
-    QEXPECT_FAIL("", "QTBUG-132831", Continue);
-    QVERIFY(!copy.isDetached());
-
-    copy = set;
     QVERIFY(!copy.isDetached());
 
     QCOMPARE(copy.removeIf([] (auto) { return false; }), 0);
-    QEXPECT_FAIL("", "QTBUG-132831", Continue);
-    QVERIFY(!copy.isDetached());
-
-    copy = set;
     QVERIFY(!copy.isDetached());
 
     QVERIFY(copy.remove(4));