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 62dad9be9e172597c56a970202299563aa04918e (for removeIf()) and
5b7c3e31b538376f2b4733bd868b5875b504cdb3 (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.9 6.8 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>

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

@@ -991,12 +991,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

@@ -402,17 +402,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));