Simplify code in QExceptionSafetyPrimitives

QExceptionSafetyPrimitives::Destructor doesn't need an additional
template argument, and the freeze() method was unused.

Some methods of the Constructor class could also be simplified.

Change-Id: Iacf35bc8634f402519a8bd875b5efea7841f9db5
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

2 files changed, 22 insertions, 60 deletions

diff --git a/src/corelib/tools/qarraydataops.h b/src/corelib/tools/qarraydataops.h
index 30a322c892..d8659ff0d7 100644
--- a/src/corelib/tools/qarraydataops.h
+++ b/src/corelib/tools/qarraydataops.h
@@ -85,11 +85,6 @@ struct QArrayExceptionSafetyPrimitives
         T *const where;
         size_t n = 0;
 
-        template<typename It>
-        using iterator_copy_value = decltype(*std::declval<It>());
-        template<typename It>
-        using iterator_move_value = decltype(std::move(*std::declval<It>()));
-
         Constructor(T *w) noexcept : where(w) {}
         qsizetype create(size_t size) noexcept(std::is_nothrow_default_constructible_v<T>)
         {
@@ -100,9 +95,7 @@ struct QArrayExceptionSafetyPrimitives
             }
             return qsizetype(std::exchange(n, 0));
         }
-        template<typename ForwardIt>
-        qsizetype copy(ForwardIt first, ForwardIt last)
-            noexcept(std::is_nothrow_constructible_v<T, iterator_copy_value<ForwardIt>>)
+        qsizetype copy(const T *first, const T *last) noexcept(std::is_nothrow_copy_constructible_v<T>)
         {
             n = 0;
             for (; first != last; ++first) {
@@ -111,8 +104,7 @@ struct QArrayExceptionSafetyPrimitives
             }
             return qsizetype(std::exchange(n, 0));
         }
-        qsizetype clone(size_t size, parameter_type t)
-            noexcept(std::is_nothrow_constructible_v<T, parameter_type>)
+        qsizetype clone(size_t size, parameter_type t) noexcept(std::is_nothrow_constructible_v<T, parameter_type>)
         {
             n = 0;
             while (n != size) {
@@ -121,9 +113,7 @@ struct QArrayExceptionSafetyPrimitives
             }
             return qsizetype(std::exchange(n, 0));
         }
-        template<typename ForwardIt>
-        qsizetype move(ForwardIt first, ForwardIt last)
-            noexcept(std::is_nothrow_constructible_v<T, iterator_move_value<ForwardIt>>)
+        qsizetype move(T *first, T *last) noexcept(std::is_nothrow_move_constructible_v<T>)
         {
             n = 0;
             for (; first != last; ++first) {
@@ -139,30 +129,25 @@ struct QArrayExceptionSafetyPrimitives
         }
     };
 
-    // Watches passed iterator. Unless commit() is called, all the elements that
+    // Watches passed pointer. Unless commit() is called, all the elements that
     // the watched iterator passes through are deleted at the end of object
     // lifetime.
     //
     // Requirements: when not at starting position, the iterator is expected to
     //               point to a valid object (to initialized memory)
-    template<typename It = iterator>
     struct Destructor
     {
-        It *iter;
-        It end;
-        It intermediate;
+        T **iter;
+        T *end;
+        T *intermediate;
 
-        Destructor(It &it) noexcept(std::is_nothrow_copy_constructible_v<It>)
+        Destructor(T *&it) noexcept
             : iter(std::addressof(it)), end(it)
         { }
         void commit() noexcept
         {
             iter = std::addressof(end);
         }
-        void freeze() noexcept(std::is_nothrow_copy_constructible_v<It>)
-        {
-            intermediate = *iter; iter = std::addressof(intermediate);
-        }
         ~Destructor() noexcept(std::is_nothrow_destructible_v<T>)
         {
             // Step is either 1 or -1 and depends on the interposition of *iter
@@ -554,7 +539,7 @@ public:
         Q_ASSERT(e <= where || b > this->end() || where == this->end()); // No overlap or append
         Q_ASSERT((e - b) <= this->freeSpaceAtEnd());
 
-        typedef typename QArrayExceptionSafetyPrimitives<T>::template Destructor<T *> Destructor;
+        using Destructor = typename QArrayExceptionSafetyPrimitives<T>::Destructor;
 
         // Array may be truncated at where in case of exceptions
 
@@ -607,7 +592,7 @@ public:
         Q_ASSERT(e <= where || b > this->end() || where == this->end()); // No overlap or append
         Q_ASSERT((e - b) <= this->freeSpaceAtBegin());
 
-        typedef typename QArrayExceptionSafetyPrimitives<T>::template Destructor<T *> Destructor;
+        using Destructor = typename QArrayExceptionSafetyPrimitives<T>::Destructor;
 
         T *begin = this->begin();
         T *readIter = begin;
@@ -655,7 +640,7 @@ public:
         Q_ASSERT(where >= this->begin() && where <= this->end());
         Q_ASSERT(size_t(this->freeSpaceAtEnd()) >= n);
 
-        typedef typename QArrayExceptionSafetyPrimitives<T>::template Destructor<T *> Destructor;
+        using Destructor = typename QArrayExceptionSafetyPrimitives<T>::Destructor;
 
         // Array may be truncated at where in case of exceptions
         T *end = this->end();
@@ -705,7 +690,7 @@ public:
         Q_ASSERT(where >= this->begin() && where <= this->end());
         Q_ASSERT(size_t(this->freeSpaceAtBegin()) >= n);
 
-        typedef typename QArrayExceptionSafetyPrimitives<T>::template Destructor<T *> Destructor;
+        using Destructor = typename QArrayExceptionSafetyPrimitives<T>::Destructor;
 
         T *begin = this->begin();
         T *readIter = begin;
diff --git a/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp b/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp
index e5acee6971..d82f4c4a22 100644
--- a/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp
+++ b/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp
@@ -2384,7 +2384,7 @@ void tst_QArrayData::exceptionSafetyPrimitives_constructor()
     {
         auto data = createDataPointer<ThrowingType>(20, 10);
         const auto originalSize = data.size;
-        const std::array<ThrowingType, 0> emptyRange{};
+        std::array<ThrowingType, 0> emptyRange{};
 
         doConstruction(data, data.end(), [] (Constructor &ctor) { return ctor.create(0); });
         QCOMPARE(data.size, originalSize);
@@ -2450,7 +2450,7 @@ void tst_QArrayData::exceptionSafetyPrimitives_constructor()
     {
         auto data = createDataPointer<ThrowingType>(20, 10);
         auto reference = createDataPointer<ThrowingType>(20, 10);
-        const std::array<ThrowingType, 3> source = {
+        std::array<ThrowingType, 3> source = {
             ThrowingType(42), ThrowingType(43), ThrowingType(44)
         };
         reference->copyAppend(source.begin(), source.end());
@@ -2542,7 +2542,7 @@ void tst_QArrayData::exceptionSafetyPrimitives_constructor()
     {
         auto data = createDataPointer<ThrowingType>(20, 10);
         auto reference = createDataPointer<ThrowingType>(20, 10);
-        const std::array<ThrowingType, 4> source = {
+        std::array<ThrowingType, 4> source = {
             ThrowingType(42), ThrowingType(43), ThrowingType(44), ThrowingType(170)
         };
 
@@ -2570,7 +2570,7 @@ void tst_QArrayData::exceptionSafetyPrimitives_constructor()
 void tst_QArrayData::exceptionSafetyPrimitives_destructor()
 {
     using Prims = QtPrivate::QArrayExceptionSafetyPrimitives<ThrowingType>;
-    using Destructor = typename Prims::Destructor<>;
+    using Destructor = typename Prims::Destructor;
 
     struct WatcherScope
     {
@@ -2590,7 +2590,7 @@ void tst_QArrayData::exceptionSafetyPrimitives_destructor()
 
         WatcherScope scope; Q_UNUSED(scope);
         {
-            auto where = data.end() - 1;
+            ThrowingType *where = data.end() - 1;
             Destructor destroyer(where);
             for (int i = 0; i < 2; ++i) {
                 new (where + 1) ThrowingType(42);
@@ -2613,7 +2613,7 @@ void tst_QArrayData::exceptionSafetyPrimitives_destructor()
 
         WatcherScope scope; Q_UNUSED(scope);
         try {
-            auto where = data.end() - 1;
+             ThrowingType *where = data.end() - 1;
             Destructor destroyer(where);
             for (int i = 0; i < 2; ++i) {
                 new (where + 1) ThrowingType(42 + i);
@@ -2644,7 +2644,7 @@ void tst_QArrayData::exceptionSafetyPrimitives_destructor()
         data.size -= 2;
         WatcherScope scope; Q_UNUSED(scope);
         {
-            auto where = data.begin() + 2;  // Note: not updated data ptr, so begin + 2
+            ThrowingType *where = data.begin() + 2;  // Note: not updated data ptr, so begin + 2
             Destructor destroyer(where);
             for (int i = 0; i < 2; ++i) {
                 new (where - 1) ThrowingType(42);
@@ -2670,7 +2670,7 @@ void tst_QArrayData::exceptionSafetyPrimitives_destructor()
         data.size -= 2;
         WatcherScope scope; Q_UNUSED(scope);
         try {
-            auto where = data.begin() + 2;  // Note: not updated data ptr, so begin + 2
+            ThrowingType *where = data.begin() + 2;  // Note: not updated data ptr, so begin + 2
             Destructor destroyer(where);
             for (int i = 0; i < 2; ++i) {
                 new (where - 1) ThrowingType(42 + i);
@@ -2697,7 +2697,7 @@ void tst_QArrayData::exceptionSafetyPrimitives_destructor()
 
         WatcherScope scope; Q_UNUSED(scope);
         try {
-            auto where = data.end() - 1;
+            ThrowingType *where = data.end() - 1;
             Destructor destroyer(where);
             ThrowingType::throwOnce = 1;
             new (where + 1) ThrowingType(42);
@@ -2725,7 +2725,7 @@ void tst_QArrayData::exceptionSafetyPrimitives_destructor()
         data.size -= 2;
         WatcherScope scope; Q_UNUSED(scope);
         try {
-            auto where = data.begin() - 1;  // Note: intentionally out of range
+            ThrowingType *where = data.begin() - 1;  // Note: intentionally out of range
             Destructor destroyer(where);
                 for (int i = 0; i < 2; ++i) {
                 new (where + 1) ThrowingType(42);
@@ -2742,29 +2742,6 @@ void tst_QArrayData::exceptionSafetyPrimitives_destructor()
             QVERIFY(throwingTypeWatcher().destroyedIds[0] == 42);
         }
     }
-
-    // extra: special case of freezing the position
-    {
-        auto data = createDataPointer<ThrowingType>(20, 10);
-        auto reference = createDataPointer<ThrowingType>(20, 10);
-        reference->erase(reference.end() - 1, reference.end());
-        data.data()[data.size - 1] = ThrowingType(42);
-
-        WatcherScope scope; Q_UNUSED(scope);
-        {
-            auto where = data.end();
-            Destructor destroyer(where);
-            for (int i = 0; i < 3; ++i) {
-                --where;
-                destroyer.freeze();
-            }
-        }
-        --data.size;  // destroyed 1 element above
-        for (qsizetype i = 0; i < data.size; ++i)
-            QCOMPARE(data.data()[i], reference.data()[i]);
-        QVERIFY(throwingTypeWatcher().destroyedIds.size() == 1);
-        QCOMPARE(throwingTypeWatcher().destroyedIds[0], 42);
-    }
 }
 
 void tst_QArrayData::exceptionSafetyPrimitives_mover()