diff options
author | Lars Knoll <lars.knoll@qt.io> | 2020-11-06 15:57:03 +0100 |
---|---|---|
committer | Lars Knoll <lars.knoll@qt.io> | 2020-11-17 11:45:52 +0100 |
commit | fc172c43132a15e17bd435db90e85722c7100361 (patch) | |
tree | b3a48eea73904e9262ef9ba1cee0e0c1300a8a63 | |
parent | ee122077b09430da54ca09750589b37326a22d85 (diff) |
Simplify the code for QList::emplace()
Unify it with the code in QArrayDataOps and only have one
emplace method there that handles it all.
Adjust autotests to API changes in QArrayDataOps and fix a
wrong test case (that just happened to pass by chance before).
Change-Id: Ia08cadebe2f74b82c31f856b1ff8a3d8dc400a3c
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
-rw-r--r-- | src/corelib/tools/qarraydataops.h | 34 | ||||
-rw-r--r-- | src/corelib/tools/qlist.h | 20 | ||||
-rw-r--r-- | tests/auto/corelib/tools/qarraydata/simplevector.h | 11 | ||||
-rw-r--r-- | tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp | 16 |
4 files changed, 32 insertions, 49 deletions
diff --git a/src/corelib/tools/qarraydataops.h b/src/corelib/tools/qarraydataops.h index 3548424b53..c2414f22f2 100644 --- a/src/corelib/tools/qarraydataops.h +++ b/src/corelib/tools/qarraydataops.h @@ -1345,22 +1345,28 @@ public: } template<typename... Args> - void emplace(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->allocatedCapacity() - this->size >= 1); - - const T *begin = this->begin(); - const T *end = this->end(); - // Qt5 QList in insert(1): try to move less data around - // Now: - const bool shouldInsertAtBegin = - (where - begin) < (end - where) || this->freeSpaceAtEnd() <= 0; - if (this->freeSpaceAtBegin() > 0 && shouldInsertAtBegin) { - Base::emplace(GrowsBackwardsTag{}, where, std::forward<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 { - Base::emplace(GrowsForwardTag{}, where, std::forward<Args>(args)...); + 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)...); } } diff --git a/src/corelib/tools/qlist.h b/src/corelib/tools/qlist.h index a5cb560dc0..9c95d49bae 100644 --- a/src/corelib/tools/qlist.h +++ b/src/corelib/tools/qlist.h @@ -743,25 +743,7 @@ typename QList<T>::iterator QList<T>::emplace(qsizetype i, Args&&... args) { Q_ASSERT_X(i >= 0 && i <= d->size, "QList<T>::insert", "index out of range"); - - if (d->needsDetach() || (d.size == d.constAllocatedCapacity())) { - typename QArrayData::GrowthPosition pos = QArrayData::GrowsAtEnd; - if (d.size != 0 && i <= (d.size >> 1)) - pos = QArrayData::GrowsAtBeginning; - - DataPointer detached(DataPointer::allocateGrow(d, 1, pos)); - const_iterator where = constBegin() + i; - - // protect against args being an element of the container - T tmp(std::forward<Args>(args)...); - - detached->copyAppend(constBegin(), where); - detached->emplace(detached.end(), std::move(tmp)); - detached->copyAppend(where, constEnd()); - d.swap(detached); - } else { - d->emplace(d.begin() + i, std::forward<Args>(args)...); - } + d->emplace(i, std::forward<Args>(args)...); return d.begin() + i; } diff --git a/tests/auto/corelib/tools/qarraydata/simplevector.h b/tests/auto/corelib/tools/qarraydata/simplevector.h index 152bf02ce3..1dc9be765c 100644 --- a/tests/auto/corelib/tools/qarraydata/simplevector.h +++ b/tests/auto/corelib/tools/qarraydata/simplevector.h @@ -216,15 +216,10 @@ public: auto requiredSize = qsizetype(last - first); if (d->needsDetach() || d.freeSpaceAtEnd() < requiredSize) { - SimpleVector detached(DataPointer::allocateGrow(d, requiredSize, QArrayData::GrowsAtEnd)); - - if (d->size) { - const T *const begin = constBegin(); - detached.d->copyAppend(begin, begin + d->size); - } - detached.d->copyAppend(first, last); - detached.swap(*this); + DataPointer oldData; + d.reallocateAndGrow(QArrayData::GrowsAtEnd, requiredSize, &oldData); + d->copyAppend(first, last); return; } diff --git a/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp b/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp index 68e8a26cb2..e5acee6971 100644 --- a/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp +++ b/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp @@ -1632,7 +1632,7 @@ void tst_QArrayData::arrayOpsExtra() const size_t originalSize = dataPointer.size; auto copy = cloneArrayDataPointer(dataPointer, dataPointer.size); - dataPointer->emplace(dataPointer.begin() + pos, value); + dataPointer->emplace(pos, value); QCOMPARE(size_t(dataPointer.size), originalSize + 1); size_t i = 0; for (; i < pos; ++i) @@ -2136,7 +2136,7 @@ struct MyQStringWrapper : public QString bool movedFrom = false; MyQStringWrapper() = default; MyQStringWrapper(QChar c) : QString(c) { } - MyQStringWrapper(MyQStringWrapper &&other) : QString(std::move(static_cast<QString>(other))) + MyQStringWrapper(MyQStringWrapper &&other) : QString(std::move(static_cast<QString &>(other))) { movedTo = true; movedFrom = other.movedFrom; @@ -2144,7 +2144,7 @@ struct MyQStringWrapper : public QString } MyQStringWrapper &operator=(MyQStringWrapper &&other) { - QString::operator=(std::move(static_cast<QString>(other))); + QString::operator=(std::move(static_cast<QString &>(other))); movedTo = true; movedFrom = other.movedFrom; other.movedFrom = true; @@ -2216,13 +2216,13 @@ void tst_QArrayData::selfEmplaceBackwards() QVERIFY(!adp.freeSpaceAtEnd()); QVERIFY(adp.freeSpaceAtBegin()); - adp->emplace(adp.end(), adp.data()[0]); + adp->emplace(adp.size, adp.data()[0]); for (qsizetype i = 0; i < adp.size - 1; ++i) { QCOMPARE(adp.data()[i], initValues[i]); } QCOMPARE(adp.data()[adp.size - 1], initValues[0]); - adp->emplace(adp.end(), std::move(adp.data()[0])); + adp->emplace(adp.size, std::move(adp.data()[0])); for (qsizetype i = 1; i < adp.size - 2; ++i) { QCOMPARE(adp.data()[i], initValues[i]); } @@ -2259,14 +2259,14 @@ void tst_QArrayData::selfEmplaceForward() QVERIFY(!adp.freeSpaceAtBegin()); QVERIFY(adp.freeSpaceAtEnd()); - adp->emplace(adp.begin(), adp.data()[adp.size - 1]); + adp->emplace(0, adp.data()[adp.size - 1]); for (qsizetype i = 1; i < adp.size; ++i) { QCOMPARE(adp.data()[i], initValues[i - 1]); } QCOMPARE(adp.data()[0], initValues[spaceAtBegin - 1]); - adp->emplace(adp.begin(), std::move(adp.data()[adp.size - 1])); - for (qsizetype i = 2; i < adp.size; ++i) { + adp->emplace(0, std::move(adp.data()[adp.size - 1])); + for (qsizetype i = 2; i < adp.size - 1; ++i) { QCOMPARE(adp.data()[i], initValues[i - 2]); } QCOMPARE(adp.data()[1], initValues[spaceAtBegin - 1]); |