From 30597cfc0ef299356a2d200a5628612d4d0b222c Mon Sep 17 00:00:00 2001
From: Lars Knoll <lars.knoll@qt.io>
Date: Thu, 12 Nov 2020 12:30:33 +0100
Subject: [PATCH] Clean up emplace implementations
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Avoid duplicated code paths for GrowsForward vs
GrowsBackward. Special case emplaceing at the
beginning or end  of the awrray where we can
avoid creating a temporary copy.

Change-Id: I2218ffd8d38cfa22e8faca75ebeadb79a682d3cf
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
---
 src/corelib/tools/qarraydataops.h             | 329 ++++++------------
 .../tools/qarraydata/tst_qarraydata.cpp       | 154 --------
 2 files changed, 97 insertions(+), 386 deletions(-)

diff --git a/src/corelib/tools/qarraydataops.h b/src/corelib/tools/qarraydataops.h
index 747417e5a0..6b775fd47c 100644
--- a/src/corelib/tools/qarraydataops.h
+++ b/src/corelib/tools/qarraydataops.h
@@ -58,57 +58,6 @@ template <class T> struct QArrayDataPointer;
 
 namespace QtPrivate {
 
-/*!
-    \internal
-
-    This class provides basic building blocks to do reversible operations. This
-    in turn allows to reason about exception safety under certain conditions.
-
-    This class is not part of the Qt API. It exists for the convenience of other
-    Qt classes.  This class may change from version to version without notice,
-    or even be removed.
-
-    We mean it.
- */
-template<typename T>
-struct QArrayExceptionSafetyPrimitives
-{
-    using parameter_type = typename QArrayDataPointer<T>::parameter_type;
-    using iterator = typename QArrayDataPointer<T>::iterator;
-
-    // Moves the data range in memory by the specified amount. Unless commit()
-    // is called, the data is moved back to the original place at the end of
-    // object lifetime.
-    struct Displacer
-    {
-        T *const begin;
-        T *const end;
-        qsizetype displace;
-
-        static_assert(QTypeInfo<T>::isRelocatable, "Type must be relocatable");
-
-        Displacer(T *start, T *finish, qsizetype diff) noexcept
-            : begin(start), end(finish), displace(diff)
-        {
-            if (displace)
-                ::memmove(static_cast<void *>(begin + displace), static_cast<void *>(begin),
-                          (end - begin) * sizeof(T));
-        }
-        void commit() noexcept { displace = 0; }
-        ~Displacer() noexcept
-        {
-            if constexpr (!std::is_nothrow_copy_constructible_v<T>)
-                if (displace)
-                    ::memmove(static_cast<void *>(begin), static_cast<void *>(begin + displace),
-                              (end - begin) * sizeof(T));
-        }
-    };
-};
-
-// Tags for compile-time dispatch based on backwards vs forward growing policy
-struct GrowsForwardTag {};
-struct GrowsBackwardsTag {};
-
 QT_WARNING_PUSH
 #if defined(Q_CC_GNU) && Q_CC_GNU >= 700
 QT_WARNING_DISABLE_GCC("-Wstringop-overflow")
@@ -237,48 +186,34 @@ public:
             *where++ = copy;
     }
 
-    template <typename ...Args>
-    void emplace(GrowsForwardTag, T *where, Args&&... args)
+    template<typename... Args>
+    void emplace(qsizetype i, Args &&... args)
     {
-        Q_ASSERT(!this->isShared());
-        Q_ASSERT(where >= this->begin() && where <= this->end());
-        Q_ASSERT(this->freeSpaceAtEnd() >= 1);
-
-        if (where == this->end()) {
-            new (this->end()) T(std::forward<Args>(args)...);
-        } else {
-            // Preserve the value, because it might be a reference to some part of the moved chunk
-            T t(std::forward<Args>(args)...);
-
-            ::memmove(static_cast<void *>(where + 1), static_cast<void *>(where),
-                      (static_cast<const T*>(this->end()) - where) * sizeof(T));
-            *where = t;
+        bool detach = this->needsDetach();
+        if (!detach) {
+            if (i == this->size && this->freeSpaceAtEnd()) {
+                new (this->end()) T(std::forward<Args>(args)...);
+                ++this->size;
+                return;
+            }
+            if (i == 0 && this->freeSpaceAtBegin()) {
+                new (this->begin() - 1) T(std::forward<Args>(args)...);
+                --this->ptr;
+                ++this->size;
+                return;
+            }
         }
+        T tmp(std::forward<Args>(args)...);
+        typename QArrayData::GrowthPosition pos = QArrayData::GrowsAtEnd;
+        if (this->size != 0 && i <= (this->size >> 1))
+            pos = QArrayData::GrowsAtBeginning;
+        if (detach ||
+            (pos == QArrayData::GrowsAtBeginning && !this->freeSpaceAtBegin()) ||
+            (pos == QArrayData::GrowsAtEnd && !this->freeSpaceAtEnd()))
+            this->reallocateAndGrow(pos, 1);
 
-        ++this->size;
-    }
-
-    template <typename ...Args>
-    void emplace(GrowsBackwardsTag, T *where, Args&&... args)
-    {
-        Q_ASSERT(!this->isShared());
-        Q_ASSERT(where >= this->begin() && where <= this->end());
-        Q_ASSERT(this->freeSpaceAtBegin() >= 1);
-
-        if (where == this->begin()) {
-            new (this->begin() - 1) T(std::forward<Args>(args)...);
-            --this->ptr;
-        } else {
-            // Preserve the value, because it might be a reference to some part of the moved chunk
-            T t(std::forward<Args>(args)...);
-
-            auto oldBegin = this->begin();
-            --this->ptr;
-            ::memmove(static_cast<void *>(this->begin()), static_cast<void *>(oldBegin), (where - oldBegin) * sizeof(T));
-            *(where - 1) = t;
-        }
-
-        ++this->size;
+        T *where = createHole(pos, i, 1);
+        new (where) T(std::move(tmp));
     }
 
     void erase(T *b, T *e)
@@ -577,26 +512,28 @@ public:
                 where[i] = t;
         }
 
-#if 0
-        void insertHole(T *where)
+        void insertOne(qsizetype pos, T &&t)
         {
-            T *oldEnd = end;
+            setup(pos, 1);
 
-            // create a new element at the end by mov constructing one existing element
-            // inside the array.
-            new (end) T(std::move(end - increment));
-            end += increment;
+            if (sourceCopyConstruct) {
+                Q_ASSERT(sourceCopyConstruct == increment);
+                new (end) T(std::move(t));
+                ++size;
+            } else {
+                // create a new element at the end by move constructing one existing element
+                // inside the array.
+                new (end) T(std::move(*(end - increment)));
+                ++size;
 
-            // now move existing elements towards the end
-            T *to = oldEnd;
-            T *from = oldEnd - increment;
-            while (from != where) {
-                *to = std::move(*from);
-                to -= increment;
-                from -= increment;
+                // now move assign existing elements towards the end
+                for (qsizetype i = 0; i != move; i -= increment)
+                    last[i] = std::move(last[i - increment]);
+
+                // and move the new item into place
+                *where = std::move(t);
             }
         }
-#endif
     };
 
     void insert(qsizetype i, const T *data, qsizetype n)
@@ -627,75 +564,32 @@ public:
     }
 
     template<typename... Args>
-    void emplace(GrowsForwardTag, T *where, Args &&... args)
+    void emplace(qsizetype i, Args &&... args)
     {
-        Q_ASSERT(!this->isShared());
-        Q_ASSERT(where >= this->begin() && where <= this->end());
-        Q_ASSERT(this->freeSpaceAtEnd() >= 1);
-
-        if (where == this->end()) {
-            new (this->end()) T(std::forward<Args>(args)...);
-            ++this->size;
-        } else {
-            T tmp(std::forward<Args>(args)...);
-
-            T *const end = this->end();
-            T *readIter = end - 1;
-            T *writeIter = end;
-
-            // Create new element at the end
-            new (writeIter) T(std::move(*readIter));
-            ++this->size;
-
-            // Move assign over existing elements
-            while (readIter != where) {
-                --readIter;
-                --writeIter;
-                *writeIter = std::move(*readIter);
+        bool detach = this->needsDetach();
+        if (!detach) {
+            if (i == this->size && this->freeSpaceAtEnd()) {
+                new (this->end()) T(std::forward<Args>(args)...);
+                ++this->size;
+                return;
             }
-
-            // Assign new element
-            --writeIter;
-            *writeIter = std::move(tmp);
-        }
-    }
-
-    template<typename... Args>
-    void emplace(GrowsBackwardsTag, T *where, Args &&... args)
-    {
-        Q_ASSERT(!this->isShared());
-        Q_ASSERT(where >= this->begin() && where <= this->end());
-        Q_ASSERT(this->freeSpaceAtBegin() >= 1);
-
-        if (where == this->begin()) {
-            new (this->begin() - 1) T(std::forward<Args>(args)...);
-            --this->ptr;
-            ++this->size;
-        } else {
-            T tmp(std::forward<Args>(args)...);
-
-            T *const begin = this->begin();
-            T *readIter = begin;
-            T *writeIter = begin - 1;
-
-            // Create new element at the beginning
-            new (writeIter) T(std::move(*readIter));
-            --this->ptr;
-            ++this->size;
-
-            ++readIter;
-            ++writeIter;
-
-            // Move assign over existing elements
-            while (readIter != where) {
-                *writeIter = std::move(*readIter);
-                ++readIter;
-                ++writeIter;
+            if (i == 0 && this->freeSpaceAtBegin()) {
+                new (this->begin() - 1) T(std::forward<Args>(args)...);
+                --this->ptr;
+                ++this->size;
+                return;
             }
-
-            // Assign new element
-            *writeIter = std::move(tmp);
         }
+        T tmp(std::forward<Args>(args)...);
+        typename QArrayData::GrowthPosition pos = QArrayData::GrowsAtEnd;
+        if (this->size != 0 && i <= (this->size >> 1))
+            pos = QArrayData::GrowsAtBeginning;
+        if (detach ||
+            (pos == QArrayData::GrowsAtBeginning && !this->freeSpaceAtBegin()) ||
+            (pos == QArrayData::GrowsAtEnd && !this->freeSpaceAtEnd()))
+            this->reallocateAndGrow(pos, 1);
+
+        Inserter(this, pos).insertOne(i, std::move(tmp));
     }
 
     void erase(T *b, T *e)
@@ -856,6 +750,15 @@ public:
                 displaceFrom += increment;
             }
         }
+
+        void insertOne(qsizetype pos, T &&t)
+        {
+            T *where = displace(pos, 1);
+            new (where) T(std::move(t));
+            displaceFrom += increment;
+            Q_ASSERT(displaceFrom == displaceTo);
+        }
+
     };
 
 
@@ -887,47 +790,34 @@ public:
     }
 
     template<typename... Args>
-    void emplace(GrowsForwardTag, T *where, Args &&... args)
+    void emplace(qsizetype i, Args &&... args)
     {
-        Q_ASSERT(!this->isShared());
-        Q_ASSERT(where >= this->begin() && where <= this->end());
-        Q_ASSERT(this->freeSpaceAtEnd() >= 1);
-
-        if (where == this->end()) {
-            new (where) T(std::forward<Args>(args)...);
-        } else {
-            T tmp(std::forward<Args>(args)...);
-            typedef typename QArrayExceptionSafetyPrimitives<T>::Displacer ReversibleDisplace;
-            ReversibleDisplace displace(where, this->end(), 1);
-            new (where) T(std::move(tmp));
-            displace.commit();
+        bool detach = this->needsDetach();
+        if (!detach) {
+            if (i == this->size && this->freeSpaceAtEnd()) {
+                new (this->end()) T(std::forward<Args>(args)...);
+                ++this->size;
+                return;
+            }
+            if (i == 0 && this->freeSpaceAtBegin()) {
+                new (this->begin() - 1) T(std::forward<Args>(args)...);
+                --this->ptr;
+                ++this->size;
+                return;
+            }
         }
-        ++this->size;
+        T tmp(std::forward<Args>(args)...);
+        typename QArrayData::GrowthPosition pos = QArrayData::GrowsAtEnd;
+        if (this->size != 0 && i <= (this->size >> 1))
+            pos = QArrayData::GrowsAtBeginning;
+        if (detach ||
+            (pos == QArrayData::GrowsAtBeginning && !this->freeSpaceAtBegin()) ||
+            (pos == QArrayData::GrowsAtEnd && !this->freeSpaceAtEnd()))
+            this->reallocateAndGrow(pos, 1);
+
+        Inserter(this, pos).insertOne(i, std::move(tmp));
     }
 
-    template<typename... Args>
-    void emplace(GrowsBackwardsTag, T *where, Args &&... args)
-    {
-        Q_ASSERT(!this->isShared());
-        Q_ASSERT(where >= this->begin() && where <= this->end());
-        Q_ASSERT(this->freeSpaceAtBegin() >= 1);
-
-        if (where == this->begin()) {
-            new (where - 1) T(std::forward<Args>(args)...);
-        } else {
-            T tmp(std::forward<Args>(args)...);
-            typedef typename QArrayExceptionSafetyPrimitives<T>::Displacer ReversibleDisplace;
-            ReversibleDisplace displace(this->begin(), where, -1);
-            new (where - 1) T(std::move(tmp));
-            displace.commit();
-        }
-        --this->ptr;
-        ++this->size;
-    }
-
-    // use moving emplace
-    using QGenericArrayOps<T>::emplace;
-
     void erase(T *b, T *e)
     {
         Q_ASSERT(this->isMutable());
@@ -1044,31 +934,6 @@ public:
     }
 
 public:
-    template<typename... Args>
-    void emplace(qsizetype i, Args &&... args)
-    {
-        typename QArrayData::GrowthPosition pos = QArrayData::GrowsAtEnd;
-        if (this->size != 0 && i <= (this->size >> 1))
-            pos = QArrayData::GrowsAtBeginning;
-        if (this->needsDetach() ||
-            (pos == QArrayData::GrowsAtBeginning && !this->freeSpaceAtBegin()) ||
-            (pos == QArrayData::GrowsAtEnd && !this->freeSpaceAtEnd())) {
-            T tmp(std::forward<Args>(args)...);
-            this->reallocateAndGrow(pos, 1);
-
-            T *where = this->begin() + i;
-            if (pos == QArrayData::GrowsAtBeginning)
-                Base::emplace(GrowsBackwardsTag{}, where, std::move(tmp));
-            else
-                Base::emplace(GrowsForwardTag{}, where, std::move(tmp));
-        } else {
-            T *where = this->begin() + i;
-            if (pos == QArrayData::GrowsAtBeginning)
-                Base::emplace(GrowsBackwardsTag{}, where, std::forward<Args>(args)...);
-            else
-                Base::emplace(GrowsForwardTag{}, where, std::forward<Args>(args)...);
-        }
-    }
 
     template <typename ...Args>
     void emplaceBack(Args&&... args)
diff --git a/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp b/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp
index 7ab93d636a..3bad143af5 100644
--- a/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp
+++ b/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp
@@ -84,9 +84,6 @@ private slots:
     void dataPointerAllocate();
     void selfEmplaceBackwards();
     void selfEmplaceForward();
-#ifndef QT_NO_EXCEPTIONS
-    void exceptionSafetyPrimitives_displacer();
-#endif
 };
 
 template <class T> const T &const_(const T &t) { return t; }
@@ -2278,156 +2275,5 @@ void tst_QArrayData::selfEmplaceForward()
     RUN_TEST_FUNC(testSelfEmplace, MyComplexQString(), 4, complexObjs);
 }
 
-#ifndef QT_NO_EXCEPTIONS
-struct ThrowingTypeWatcher
-{
-    std::vector<int> destroyedIds;
-    bool watch = false;
-    void destroyed(int id)
-    {
-        if (watch)
-            destroyedIds.push_back(id);
-    }
-};
-ThrowingTypeWatcher& throwingTypeWatcher() { static ThrowingTypeWatcher global; return global; }
-
-struct ThrowingType
-{
-    static unsigned int throwOnce;
-    static constexpr char throwString[] = "Requested to throw";
-    static constexpr char throwStringDtor[] = "Requested to throw in dtor";
-    void checkThrow()  {
-        // deferred throw
-        if (throwOnce > 0) {
-            --throwOnce;
-            if (throwOnce == 0) {
-                throw std::runtime_error(throwString);
-            }
-        }
-        return;
-    }
-    int id = 0;
-
-    ThrowingType(int val = 0) noexcept(false) : id(val)
-    {
-        checkThrow();
-    }
-    ThrowingType(const ThrowingType &other) noexcept(false) : id(other.id)
-    {
-        checkThrow();
-    }
-    ThrowingType& operator=(const ThrowingType &other) noexcept(false)
-    {
-        id = other.id;
-        checkThrow();
-        return *this;
-    }
-    ~ThrowingType()
-    {
-        throwingTypeWatcher().destroyed(id);  // notify global watcher
-        id = -1;
-
-    }
-};
-unsigned int ThrowingType::throwOnce = 0;
-bool operator==(const ThrowingType &a, const ThrowingType &b) {
-    return a.id == b.id;
-}
-QT_BEGIN_NAMESPACE
-Q_DECLARE_TYPEINFO(ThrowingType, Q_RELOCATABLE_TYPE);
-QT_END_NAMESPACE
-
-template<typename T>  // T must be constructible from a single int parameter
-static QArrayDataPointer<T> createDataPointer(qsizetype capacity, qsizetype initSize)
-{
-    QArrayDataPointer<T> adp(QTypedArrayData<T>::allocate(capacity));
-    adp->appendInitialize(initSize);
-    // assign unique values
-    int i = 0;
-    std::generate(adp.begin(), adp.end(), [&i] () { return T(i++); });
-    return adp;
-}
-
-void tst_QArrayData::exceptionSafetyPrimitives_displacer()
-{
-    QVERIFY(QTypeInfo<ThrowingType>::isRelocatable);
-    using Prims = QtPrivate::QArrayExceptionSafetyPrimitives<ThrowingType>;
-    const auto doDisplace = [] (auto &dataPointer, auto start, auto finish, qsizetype diff) {
-        typename Prims::Displacer displace(start, finish, diff);
-        new (start) ThrowingType(42);
-        ++dataPointer.size;
-        displace.commit();
-    };
-
-    // successful operation with displace to the right
-    {
-        auto data = createDataPointer<ThrowingType>(20, 10);
-        auto reference = createDataPointer<ThrowingType>(20, 10);
-        reference->insert(reference.size - 1, 1, ThrowingType(42));
-
-        auto where = data.end() - 1;
-        doDisplace(data, where, data.end(), 1);
-
-        QCOMPARE(data.size, reference.size);
-        for (qsizetype i = 0; i < data.size; ++i)
-            QCOMPARE(data.data()[i], reference.data()[i]);
-    }
-
-    // failed operation with displace to the right
-    {
-        auto data = createDataPointer<ThrowingType>(20, 10);
-        auto reference = createDataPointer<ThrowingType>(20, 10);
-        try {
-            ThrowingType::throwOnce = 1;
-            doDisplace(data, data.end() - 1, data.end(), 1);
-            QFAIL("Unreachable line!");
-        } catch (const std::exception &e) {
-            QCOMPARE(std::string(e.what()), ThrowingType::throwString);
-            QCOMPARE(data.size, reference.size);
-            for (qsizetype i = 0; i < data.size; ++i)
-                QCOMPARE(data.data()[i], reference.data()[i]);
-        }
-    }
-
-    // successful operation with displace to the left
-    {
-        auto data = createDataPointer<ThrowingType>(20, 10);
-        auto reference = createDataPointer<ThrowingType>(20, 10);
-        reference.data()[0] = reference.data()[1];
-        reference.data()[1] = ThrowingType(42);
-
-        data.begin()->~ThrowingType();  // free space at begin
-        --data.size;
-        auto where = data.begin() + 1;
-        doDisplace(data, where, where + 1, -1);
-
-        QCOMPARE(data.size, reference.size);
-        for (qsizetype i = 0; i < data.size; ++i)
-            QCOMPARE(data.data()[i], reference.data()[i]);
-    }
-
-    // failed operation with displace to the left
-    {
-        auto data = createDataPointer<ThrowingType>(20, 10);
-        auto reference = createDataPointer<ThrowingType>(20, 10);
-        reference->erase(reference.begin(), reference.begin() + 1);
-
-        try {
-            data.begin()->~ThrowingType();  // free space at begin
-            --data.size;
-            ThrowingType::throwOnce = 1;
-            auto where = data.begin() + 1;
-            doDisplace(data, where, where + 1, -1);
-            QFAIL("Unreachable line!");
-        } catch (const std::exception &e) {
-            QCOMPARE(std::string(e.what()), ThrowingType::throwString);
-            QCOMPARE(data.size, reference.size);
-            for (qsizetype i = 0; i < data.size; ++i)
-                QCOMPARE(data.data()[i + 1], reference.data()[i]);
-        }
-    }
-}
-#endif  // QT_NO_EXCEPTIONS
-
 QTEST_APPLESS_MAIN(tst_QArrayData)
 #include "tst_qarraydata.moc"