diff options
author | Marc Mutz <marc.mutz@kdab.com> | 2020-06-04 21:56:09 +0200 |
---|---|---|
committer | Marc Mutz <marc.mutz@kdab.com> | 2020-06-22 17:24:07 +0000 |
commit | ffb73175e6c5b35e6367c88479cc0bf160482016 (patch) | |
tree | 1dc68ee77b96196a4731f38000150e285bb7de5d | |
parent | 670c5bd140b86a8b589595cb6cae62ae4e24cb2d (diff) |
QVarLengthArray: add missing move special member functions
A QVLA is copyable, so it should be movable, too.
Added a helper function a la P1144's uninitialized_relocate_n to deal
with the QTypeInfoQuery stuff. This way, the code is re-usable
everywhere it's needed. The same cannot be said for QArrayDataOps,
which only a parent can love...
[ChangeLog][QtCore][QVarLengthArray] Added missing move constructor
and move-assignment operator.
Task-number: QTBUG-39111
Change-Id: If0dc2aa78eb29062d73dcd3dc4647ba345ae39e6
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
-rw-r--r-- | src/corelib/tools/qcontainertools_impl.h | 21 | ||||
-rw-r--r-- | src/corelib/tools/qvarlengtharray.h | 41 | ||||
-rw-r--r-- | src/corelib/tools/qvarlengtharray.qdoc | 11 | ||||
-rw-r--r-- | tests/auto/corelib/tools/qvarlengtharray/tst_qvarlengtharray.cpp | 92 |
4 files changed, 150 insertions, 15 deletions
diff --git a/src/corelib/tools/qcontainertools_impl.h b/src/corelib/tools/qcontainertools_impl.h index 3a0c4381f1..b43c3bb1c4 100644 --- a/src/corelib/tools/qcontainertools_impl.h +++ b/src/corelib/tools/qcontainertools_impl.h @@ -47,12 +47,33 @@ #define QCONTAINERTOOLS_IMPL_H #include <QtCore/qglobal.h> +#include <QtCore/qtypeinfo.h> + #include <iterator> +#include <memory> QT_BEGIN_NAMESPACE namespace QtPrivate { + +template <typename T, typename N> +void q_uninitialized_relocate_n(T* first, N n, T* out) +{ + if constexpr (QTypeInfoQuery<T>::isRelocatable) { + if (n != N(0)) { // even if N == 0, out == nullptr or first == nullptr are UB for memmove() + memmove(static_cast<void*>(out), + static_cast<const void*>(first), + n * sizeof(T)); + } + } else { + std::uninitialized_move_n(first, n, out); + if constexpr (QTypeInfoQuery<T>::isComplex) + std::destroy_n(first, n); + } +} + + template <typename Iterator> using IfIsInputIterator = typename std::enable_if< std::is_convertible<typename std::iterator_traits<Iterator>::iterator_category, std::input_iterator_tag>::value, diff --git a/src/corelib/tools/qvarlengtharray.h b/src/corelib/tools/qvarlengtharray.h index f90e455fe4..cea0e14d41 100644 --- a/src/corelib/tools/qvarlengtharray.h +++ b/src/corelib/tools/qvarlengtharray.h @@ -71,6 +71,26 @@ public: append(other.constData(), other.size()); } + QVarLengthArray(QVarLengthArray &&other) + noexcept(std::is_nothrow_move_constructible_v<T>) + : a{other.a}, + s{other.s}, + ptr{other.ptr} + { + const auto otherInlineStorage = reinterpret_cast<T*>(other.array); + if (ptr == otherInlineStorage) { + // inline buffer - move into our inline buffer: + ptr = reinterpret_cast<T*>(array); + QtPrivate::q_uninitialized_relocate_n(otherInlineStorage, s, ptr); + } else { + // heap buffer - we just stole the memory + } + // reset other to internal storage: + other.a = Prealloc; + other.s = 0; + other.ptr = otherInlineStorage; + } + QVarLengthArray(std::initializer_list<T> args) : QVarLengthArray(args.begin(), args.end()) { @@ -102,6 +122,27 @@ public: return *this; } + QVarLengthArray &operator=(QVarLengthArray &&other) + noexcept(std::is_nothrow_move_constructible_v<T>) + { + // we're only required to be self-move-assignment-safe + // when we're in the moved-from state (Hinnant criterion) + // the moved-from state is the empty state, so we're good with the clear() here: + clear(); + Q_ASSERT(capacity() >= Prealloc); + const auto otherInlineStorage = reinterpret_cast<T*>(other.array); + if (other.ptr != otherInlineStorage) { + // heap storage: steal the external buffer, reset other to otherInlineStorage + a = std::exchange(other.a, Prealloc); + ptr = std::exchange(other.ptr, otherInlineStorage); + } else { + // inline storage: move into our storage (doesn't matter whether inline or external) + QtPrivate::q_uninitialized_relocate_n(other.ptr, other.s, ptr); + } + s = std::exchange(other.s, 0); + return *this; + } + QVarLengthArray<T, Prealloc> &operator=(std::initializer_list<T> list) { resize(qsizetype(list.size())); diff --git a/src/corelib/tools/qvarlengtharray.qdoc b/src/corelib/tools/qvarlengtharray.qdoc index e43d6f152a..6371419ae5 100644 --- a/src/corelib/tools/qvarlengtharray.qdoc +++ b/src/corelib/tools/qvarlengtharray.qdoc @@ -404,6 +404,12 @@ Assigns \a other to this array and returns a reference to this array. */ +/*! \fn template<class T, qsizetype Prealloc> QVarLengthArray<T, Prealloc> &QVarLengthArray<T, Prealloc>::operator=(QVarLengthArray<T, Prealloc> &&other) + Move-assigns \a other to this array and returns a reference to this array. + After the move, \a other is empty. + \since 6.0 + */ + /*! \fn template<class T, qsizetype Prealloc> QVarLengthArray<T, Prealloc> &QVarLengthArray<T, Prealloc>::operator=(std::initializer_list<T> list) \since 5.5 @@ -417,6 +423,11 @@ Constructs a copy of \a other. */ +/*! \fn template<class T, qsizetype Prealloc> QVarLengthArray<T, Prealloc>::QVarLengthArray(QVarLengthArray<T, Prealloc> &&other) + Move-constructs this variable-length array from \a other. After the move, \a other is empty. + \since 6.0 + */ + /*! \fn template<class T, qsizetype Prealloc> const T &QVarLengthArray<T, Prealloc>::at(qsizetype i) const Returns a reference to the item at index position \a i. diff --git a/tests/auto/corelib/tools/qvarlengtharray/tst_qvarlengtharray.cpp b/tests/auto/corelib/tools/qvarlengtharray/tst_qvarlengtharray.cpp index 74654a3862..1ce5f1e0e6 100644 --- a/tests/auto/corelib/tools/qvarlengtharray/tst_qvarlengtharray.cpp +++ b/tests/auto/corelib/tools/qvarlengtharray/tst_qvarlengtharray.cpp @@ -29,14 +29,55 @@ #include <QtTest/QtTest> #include <qvarlengtharray.h> #include <qvariant.h> +#include <qscopeguard.h> #include <memory> +struct Tracker +{ + static int count; + Tracker() { ++count; } + Tracker(const Tracker &) { ++count; } + Tracker(Tracker &&) { ++count; } + + Tracker &operator=(const Tracker &) = default; + Tracker &operator=(Tracker &&) = default; + + ~Tracker() { --count; } + +}; + +int Tracker::count = 0; + +template <typename T> +class ValueTracker +{ + Tracker m_tracker; +public: + ValueTracker() = default; + ValueTracker(T value) : value{std::move(value)} {} + T value; + + friend bool operator==(const ValueTracker &lhs, const ValueTracker &rhs) noexcept + { return lhs.value == rhs.value; } + friend bool operator!=(const ValueTracker &lhs, const ValueTracker &rhs) noexcept + { return !operator==(lhs, rhs); } +}; + class tst_QVarLengthArray : public QObject { Q_OBJECT private slots: void append(); + void move_int_1() { move_int<1>(); } + void move_int_2() { move_int<2>(); } + void move_int_3() { move_int<3>(); } + void move_QString_1() { move_QString<1>(); } + void move_QString_2() { move_QString<2>(); } + void move_QString_3() { move_QString<3>(); } + void move_Tracker_1() { move_Tracker<1>(); } + void move_Tracker_2() { move_Tracker<2>(); } + void move_Tracker_3() { move_Tracker<3>(); } void removeLast(); void oldTests(); void appendCausingRealloc(); @@ -61,25 +102,18 @@ private slots: void implicitDefaultCtor(); private: + template <qsizetype N, typename T> + void move(T t1, T t2); + template <qsizetype N> + void move_int() { move<N, int>(42, 24); } + template <qsizetype N> + void move_QString() { move<N, QString>("Hello", "World"); } + template <qsizetype N> + void move_Tracker(); template<typename T> void initializeList(); }; -struct Tracker -{ - static int count; - Tracker() { ++count; } - Tracker(const Tracker &) { ++count; } - Tracker(Tracker &&) { ++count; } - - Tracker &operator=(const Tracker &) = default; - Tracker &operator=(Tracker &&) = default; - - ~Tracker() { --count; } -}; - -int Tracker::count = 0; - void tst_QVarLengthArray::append() { QVarLengthArray<QString, 2> v; @@ -102,6 +136,34 @@ void tst_QVarLengthArray::append() v2.append(5); } +template <qsizetype N> +void tst_QVarLengthArray::move_Tracker() +{ + const auto reset = qScopeGuard([] { Tracker::count = 0; }); + move<N, ValueTracker<int>>({24}, {24}); + QCOMPARE(Tracker::count, 0); +} + +template <qsizetype N, typename T> +void tst_QVarLengthArray::move(T t1, T t2) +{ + { + QVarLengthArray<T, N> v; + v.append(t1); + v.append(t2); + + auto moved = std::move(v); + QCOMPARE(moved.size(), 2); + QCOMPARE(moved[0], t1); + QCOMPARE(moved[1], t2); + + v = std::move(moved); + QCOMPARE(v.size(), 2); + QCOMPARE(v[0], t1); + QCOMPARE(v[1], t2); + } +} + void tst_QVarLengthArray::removeLast() { { |