From 89672efd58452e65038a8f29ef4e603df88305ba Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@qt.io>
Date: Mon, 9 Jan 2023 10:14:31 +0100
Subject: [PATCH] QVarLengthArray: cope with vector<unique_ptr>'s copyability
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Despite being move-only, std::vector<unique_ptr> advertizes
is_copyable:
  https://quuxplusone.github.io/blog/2020/02/05/vector-is-copyable-except-when-its-not/

Our combined reallocation and resizing function, reallocate_impl(),
runs afoul of this when it uses std::is_copyable in a constexpr-if to
implement resize(n, v) without running into problems with move-only
types: the trait is true, but actual instantation runs into a
static_assert in the STL implementation.

To fix, move the problematic resize functionality out of
reallocate_impl() and into the resp. resize_impl overloads. The shrink
functionality remains in reallocate_impl(), because there are many
more users, and it only requires destructible<T>, which isn't
constraining at all.

Amends a00a1d8806cfbf17e04b88d1b4ff4a9cf5b6294a.

Fixes: QTBUG-109745
Change-Id: Ibc5b9cf5375108eb3d8f6c8a16d4fd02dadd73b1
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
(cherry picked from commit 800ebd84f57092ccba24984f3888f97bb5433d8e)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/corelib/tools/qvarlengtharray.h           | 39 ++++++++-----------
 .../qvarlengtharray/tst_qvarlengtharray.cpp   | 23 +++++++++++
 2 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/src/corelib/tools/qvarlengtharray.h b/src/corelib/tools/qvarlengtharray.h
index 204364b52fe..9377cc50b4e 100644
--- a/src/corelib/tools/qvarlengtharray.h
+++ b/src/corelib/tools/qvarlengtharray.h
@@ -208,14 +208,27 @@ protected:
     }
 
     void append_impl(qsizetype prealloc, void *array, const T *buf, qsizetype n);
-    void reallocate_impl(qsizetype prealloc, void *array, qsizetype size, qsizetype alloc, const T *v = nullptr);
+    void reallocate_impl(qsizetype prealloc, void *array, qsizetype size, qsizetype alloc);
     void resize_impl(qsizetype prealloc, void *array, qsizetype sz, const T &v)
     {
-        reallocate_impl(prealloc, array, sz, qMax(sz, capacity()), &v);
+        reallocate_impl(prealloc, array, sz, qMax(sz, capacity()));
+        while (size() < sz) {
+            new (data() + size()) T(v);
+            ++s;
+        }
     }
     void resize_impl(qsizetype prealloc, void *array, qsizetype sz)
     {
         reallocate_impl(prealloc, array, sz, qMax(sz, capacity()));
+        if constexpr (QTypeInfo<T>::isComplex) {
+            // call default constructor for new objects (which can throw)
+            while (size() < sz) {
+                new (data() + size()) T;
+                ++s;
+            }
+        } else {
+            s = sz;
+        }
     }
 
     bool isValidIterator(const const_iterator &i) const
@@ -714,7 +727,7 @@ Q_OUTOFLINE_TEMPLATE void QVLABase<T>::append_impl(qsizetype prealloc, void *arr
 }
 
 template <class T>
-Q_OUTOFLINE_TEMPLATE void QVLABase<T>::reallocate_impl(qsizetype prealloc, void *array, qsizetype asize, qsizetype aalloc, const T *v)
+Q_OUTOFLINE_TEMPLATE void QVLABase<T>::reallocate_impl(qsizetype prealloc, void *array, qsizetype asize, qsizetype aalloc)
 {
     Q_ASSERT(aalloc >= asize);
     Q_ASSERT(data());
@@ -755,26 +768,6 @@ Q_OUTOFLINE_TEMPLATE void QVLABase<T>::reallocate_impl(qsizetype prealloc, void
 
     if (oldPtr != reinterpret_cast<T *>(array) && oldPtr != data())
         free(oldPtr);
-
-    if (v) {
-        if constexpr (std::is_copy_constructible_v<T>) {
-            while (size() < asize) {
-                new (data() + size()) T(*v);
-                ++s;
-            }
-        } else {
-            Q_UNREACHABLE();
-        }
-    } else if constexpr (QTypeInfo<T>::isComplex) {
-        // call default constructor for new objects (which can throw)
-        while (size() < asize) {
-            new (data() + size()) T;
-            ++s;
-        }
-    } else {
-        s = asize;
-    }
-
 }
 
 template <class T>
diff --git a/tests/auto/corelib/tools/qvarlengtharray/tst_qvarlengtharray.cpp b/tests/auto/corelib/tools/qvarlengtharray/tst_qvarlengtharray.cpp
index 81edb36916c..105a3b27bf8 100644
--- a/tests/auto/corelib/tools/qvarlengtharray/tst_qvarlengtharray.cpp
+++ b/tests/auto/corelib/tools/qvarlengtharray/tst_qvarlengtharray.cpp
@@ -92,6 +92,8 @@ private slots:
     void remove();
     void erase();
 
+    // special cases:
+    void copesWithCopyabilityOfMoveOnlyVector(); // QTBUG-109745
 private:
     template <typename T>
     void defaultConstructor();
@@ -1640,5 +1642,26 @@ void tst_QVarLengthArray::erase()
     QCOMPARE(arr, QVarLengthArray<QString>({ "val0" }));
 }
 
+void tst_QVarLengthArray::copesWithCopyabilityOfMoveOnlyVector()
+{
+    // std::vector<move-only-type> is_copyable
+    // (https://quuxplusone.github.io/blog/2020/02/05/vector-is-copyable-except-when-its-not/)
+
+    QVarLengthArray<std::vector<std::unique_ptr<int>>, 2> vla;
+    vla.emplace_back(42);
+    vla.emplace_back(43);
+    vla.emplace_back(44); // goes to the heap
+    QCOMPARE_EQ(vla.size(), 3);
+    QCOMPARE_EQ(vla.front().size(), 42U);
+    QCOMPARE_EQ(vla.front().front(), nullptr);
+    QCOMPARE_EQ(vla.back().size(), 44U);
+
+    auto moved = std::move(vla);
+    QCOMPARE_EQ(moved.size(), 3);
+    QCOMPARE_EQ(moved.front().size(), 42U);
+    QCOMPARE_EQ(moved.front().front(), nullptr);
+    QCOMPARE_EQ(moved.back().size(), 44U);
+}
+
 QTEST_APPLESS_MAIN(tst_QVarLengthArray)
 #include "tst_qvarlengtharray.moc"