QList - fix QList::erase when the list is shared

Before calls to erase on a shared instance would in release mode
imply that items were removed from the shared data (i.e all instances).
In debug mode it would assert.

This patch improves the behavior to detach and erase items
specified by the iterator(s) (i.e same behavior as QVector)

Change-Id: I89b69446cb1ffd43a98402b7ab1ec9a59bceb8e6
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

2 files changed, 48 insertions, 0 deletions

diff --git a/src/corelib/tools/qlist.h b/src/corelib/tools/qlist.h
index 28d35745a5..419e0b4966 100644
--- a/src/corelib/tools/qlist.h
+++ b/src/corelib/tools/qlist.h
@@ -454,6 +454,11 @@ template <typename T>
 inline typename QList<T>::iterator QList<T>::erase(iterator it)
 {
     Q_ASSERT_X(isValidIterator(it), "QList::erase", "The specified iterator argument 'it' is invalid");
+    if (d->ref.isShared()) {
+        int offset = int(it.i - reinterpret_cast<Node *>(p.begin()));
+        it = begin(); // implies detach()
+        it += offset;
+    }
     node_destruct(it.i);
     return reinterpret_cast<Node *>(p.erase(reinterpret_cast<void**>(it.i)));
 }
@@ -820,6 +825,16 @@ Q_OUTOFLINE_TEMPLATE typename QList<T>::iterator QList<T>::erase(typename QList<
     Q_ASSERT_X(isValidIterator(afirst), "QList::erase", "The specified iterator argument 'afirst' is invalid");
     Q_ASSERT_X(isValidIterator(alast), "QList::erase", "The specified iterator argument 'alast' is invalid");
 
+    if (d->ref.isShared()) {
+        // ### A block is erased and a detach is needed. We should shrink and only copy relevant items.
+        int offsetfirst = int(afirst.i - reinterpret_cast<Node *>(p.begin()));
+        int offsetlast = int(alast.i - reinterpret_cast<Node *>(p.begin()));
+        afirst = begin(); // implies detach()
+        alast = afirst;
+        afirst += offsetfirst;
+        alast += offsetlast;
+    }
+
     for (Node *n = afirst.i; n < alast.i; ++n)
         node_destruct(n);
     int idx = afirst - begin();
diff --git a/tests/auto/corelib/tools/qlist/tst_qlist.cpp b/tests/auto/corelib/tools/qlist/tst_qlist.cpp
index 787aa094a8..5ed85939ff 100644
--- a/tests/auto/corelib/tools/qlist/tst_qlist.cpp
+++ b/tests/auto/corelib/tools/qlist/tst_qlist.cpp
@@ -276,6 +276,7 @@ private slots:
     void setSharableInt() const;
     void setSharableComplex_data() const;
     void setSharableComplex() const;
+    void eraseValidIteratorsOnSharedList() const;
 private:
     template<typename T> void length() const;
     template<typename T> void append() const;
@@ -1620,5 +1621,37 @@ void tst_QList::setSharableComplex() const
     runSetSharableTest<Complex>();
 }
 
+void tst_QList::eraseValidIteratorsOnSharedList() const
+{
+    QList<int> a, b;
+    a.push_back(10);
+    a.push_back(20);
+    a.push_back(30);
+    QList<int>::iterator i = a.begin();
+    ++i;
+    b = a;
+    a.erase(i);
+    QCOMPARE(b.size(), 3);
+    QCOMPARE(a.size(), 2);
+    QCOMPARE(a.at(0), 10);
+    QCOMPARE(a.at(1), 30);
+
+    a.push_back(40);
+    a.push_back(50);
+    a.push_back(60);
+    QCOMPARE(a.size(), 5);
+    i = a.begin();
+    b = a;
+    ++i;
+    QList<int>::iterator j = i;
+    ++j;
+    ++j;
+    a.erase(i, j); // remove 3 elements
+    QCOMPARE(b.size(), 5);
+    QCOMPARE(a.size(), 3);
+    QCOMPARE(a.at(0), 10);
+    QCOMPARE(a.at(1), 50);
+}
+
 QTEST_APPLESS_MAIN(tst_QList)
 #include "tst_qlist.moc"