Base QList::setSharable on RefCount::setSharable

Change-Id: I2acccdf9ee595a0eee33c9f7ddded9cc121412c1
Reviewed-by: Bradley T. Hughes <bradley.hughes@nokia.com>

diff --git a/src/corelib/tools/qlist.cpp b/src/corelib/tools/qlist.cpp
index dbd026e..1b6610a 100644 (file)
--- a/src/corelib/tools/qlist.cpp
+++ b/src/corelib/tools/qlist.cpp
@@ -59,7 +59,7 @@ QT_BEGIN_NAMESPACE
     the number of elements in the list.
 */
 
-const QListData::Data QListData::shared_null = { Q_REFCOUNT_INITIALIZE_STATIC, 0, 0, 0, true, { 0 } };
+const QListData::Data QListData::shared_null = { Q_REFCOUNT_INITIALIZE_STATIC, 0, 0, 0, { 0 } };
 
 static int grow(int size)
 {
@@ -88,7 +88,6 @@ QListData::Data *QListData::detach_grow(int *idx, int num)
     Q_CHECK_PTR(t);
 
     t->ref.initializeOwned();
-    t->sharable = true;
     t->alloc = alloc;
     // The space reservation algorithm's optimization is biased towards appending:
     // Something which looks like an append will put the data at the beginning,
@@ -130,7 +129,6 @@ QListData::Data *QListData::detach(int alloc)
     Q_CHECK_PTR(t);
 
     t->ref.initializeOwned();
-    t->sharable = true;
     t->alloc = alloc;
     if (!alloc) {
         t->begin = 0;

diff --git a/src/corelib/tools/qlist.h b/src/corelib/tools/qlist.h
index 798351c..bc3350f 100644 (file)
--- a/src/corelib/tools/qlist.h
+++ b/src/corelib/tools/qlist.h
@@ -71,7 +71,6 @@ struct Q_CORE_EXPORT QListData {
     struct Data {
         QtPrivate::RefCount ref;
         int alloc, begin, end;
-        uint sharable : 1;
         void *array[1];
     };
     enum { DataHeaderSize = sizeof(Data) - sizeof(void *) };
@@ -114,7 +113,7 @@ class QList
 
 public:
     inline QList() : d(const_cast<QListData::Data *>(&QListData::shared_null)) { }
-    inline QList(const QList<T> &l) : d(l.d) { d->ref.ref(); if (!d->sharable) detach_helper(); }
+    QList(const QList<T> &l);
     ~QList();
     QList<T> &operator=(const QList<T> &l);
 #ifdef Q_COMPILER_RVALUE_REFS
@@ -142,7 +141,15 @@ public:
     }
 
     inline bool isDetached() const { return !d->ref.isShared(); }
-    inline void setSharable(bool sharable) { if (!sharable) detach(); if (d != &QListData::shared_null) d->sharable = sharable; }
+    inline void setSharable(bool sharable)
+    {
+        if (sharable == d->ref.isSharable())
+            return;
+        if (!sharable)
+            detach();
+        if (d != &QListData::shared_null)
+            d->ref.setSharable(sharable);
+    }
     inline bool isSharedWith(const QList<T> &other) const { return d == other.d; }
 
     inline bool isEmpty() const { return p.isEmpty(); }
@@ -703,6 +710,28 @@ Q_OUTOFLINE_TEMPLATE void QList<T>::detach_helper()
 }
 
 template <typename T>
+Q_OUTOFLINE_TEMPLATE QList<T>::QList(const QList<T> &l)
+    : d(l.d)
+{
+    if (!d->ref.ref()) {
+        p.detach(d->alloc);
+
+        struct Cleanup
+        {
+            Cleanup(QListData::Data *d) : d_(d) {}
+            ~Cleanup() { if (d_) qFree(d_); }
+
+            QListData::Data *d_;
+        } tryCatch(d);
+
+        node_copy(reinterpret_cast<Node *>(p.begin()),
+                reinterpret_cast<Node *>(p.end()),
+                reinterpret_cast<Node *>(l.p.begin()));
+        tryCatch.d_ = 0;
+    }
+}
+
+template <typename T>
 Q_OUTOFLINE_TEMPLATE QList<T>::~QList()
 {
     if (!d->ref.deref())

diff --git a/tests/auto/corelib/tools/qlist/tst_qlist.cpp b/tests/auto/corelib/tools/qlist/tst_qlist.cpp
index fbb821c..3baa47f 100644 (file)
--- a/tests/auto/corelib/tools/qlist/tst_qlist.cpp
+++ b/tests/auto/corelib/tools/qlist/tst_qlist.cpp
@@ -54,6 +54,9 @@ class tst_QList : public QObject
     Q_OBJECT
 
 private slots:
+    void init();
+    void cleanup();
+
     void length() const;
     void lengthSignature() const;
     void append() const;
@@ -90,8 +93,100 @@ private slots:
     void initializeList() const;
 
     void const_shared_null() const;
+    void setSharable1_data() const;
+    void setSharable1() const;
+    void setSharable2_data() const;
+    void setSharable2() const;
+
+private:
+    int dummyForGuard;
+};
+
+struct Complex
+{
+    Complex(int val)
+        : value(val)
+        , checkSum(this)
+    {
+        ++liveCount;
+    }
+
+    Complex(Complex const &other)
+        : value(other.value)
+        , checkSum(this)
+    {
+        ++liveCount;
+    }
+
+    Complex &operator=(Complex const &other)
+    {
+        check(); other.check();
+
+        value = other.value;
+        return *this;
+    }
+
+    ~Complex()
+    {
+        --liveCount;
+        check();
+    }
+
+    operator int() const { return value; }
+
+    bool operator==(Complex const &other) const
+    {
+        check(); other.check();
+        return value == other.value;
+    }
+
+    bool check() const
+    {
+        if (this != checkSum) {
+            ++errorCount;
+            return false;
+        }
+        return true;
+    }
+
+    struct Guard
+    {
+        Guard() : initialLiveCount(liveCount) {}
+        ~Guard() { if (liveCount != initialLiveCount) ++errorCount; }
+
+    private:
+        Q_DISABLE_COPY(Guard);
+        int initialLiveCount;
+    };
+
+    static void resetErrors() { errorCount = 0; }
+    static int errors() { return errorCount; }
+
+private:
+    static int errorCount;
+    static int liveCount;
+
+    int value;
+    void *checkSum;
 };
 
+int Complex::errorCount = 0;
+int Complex::liveCount = 0;
+
+void tst_QList::init()
+{
+    Complex::resetErrors();
+    new (&dummyForGuard) Complex::Guard();
+}
+
+void tst_QList::cleanup()
+{
+    QCOMPARE(Complex::errors(), 0);
+
+    reinterpret_cast<Complex::Guard *>(&dummyForGuard)->~Guard();
+    QCOMPARE(Complex::errors(), 0);
+}
+
 void tst_QList::length() const
 {
     /* Empty list. */
@@ -696,5 +791,82 @@ void tst_QList::const_shared_null() const
     QVERIFY(!list2.isDetached());
 }
 
+Q_DECLARE_METATYPE(QList<int>);
+Q_DECLARE_METATYPE(QList<Complex>);
+
+template <class T>
+void generateSetSharableData()
+{
+    QTest::addColumn<QList<T> >("list");
+    QTest::addColumn<int>("size");
+
+    QTest::newRow("null") << QList<T>() << 0;
+    QTest::newRow("non-empty") << (QList<T>() << T(0) << T(1) << T(2) << T(3) << T(4)) << 5;
+}
+
+template <class T>
+void runSetSharableTest()
+{
+    QFETCH(QList<T>, list);
+    QFETCH(int, size);
+
+    QVERIFY(!list.isDetached()); // Shared with QTest
+
+    list.setSharable(true);
+
+    QCOMPARE(list.size(), size);
+
+    {
+        QList<T> copy(list);
+        QVERIFY(!copy.isDetached());
+        QVERIFY(copy.isSharedWith(list));
+    }
+
+    list.setSharable(false);
+    QVERIFY(list.isDetached() || list.isSharedWith(QList<T>()));
+
+    {
+        QList<T> copy(list);
+
+        QVERIFY(copy.isDetached() || copy.isSharedWith(QList<T>()));
+        QCOMPARE(copy.size(), size);
+        QCOMPARE(copy, list);
+    }
+
+    list.setSharable(true);
+
+    {
+        QList<T> copy(list);
+
+        QVERIFY(!copy.isDetached());
+        QVERIFY(copy.isSharedWith(list));
+    }
+
+    for (int i = 0; i < list.size(); ++i)
+        QCOMPARE(int(list[i]), i);
+
+    QCOMPARE(list.size(), size);
+}
+
+void tst_QList::setSharable1_data() const
+{
+    generateSetSharableData<int>();
+}
+
+void tst_QList::setSharable2_data() const
+{
+    generateSetSharableData<Complex>();
+}
+
+void tst_QList::setSharable1() const
+{
+    runSetSharableTest<int>();
+}
+
+void tst_QList::setSharable2() const
+{
+    runSetSharableTest<Complex>();
+}
+
 QTEST_APPLESS_MAIN(tst_QList)
 #include "tst_qlist.moc"