From 9ee50a14b72706334c7a26469afac3999d3bdac5 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Fri, 30 Oct 2020 13:01:56 +0100 Subject: [PATCH] Move insert() operation into QArrayDataOps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows us to unify and simplify the code base between QList, QString and QByteArray. Change-Id: Idc8f360d78f508a68f38eb3ef0ed6e5d37f90574 Reviewed-by: Andrei Golubev Reviewed-by: Mårten Nordheim --- src/corelib/text/qbytearray.cpp | 87 +++++++++++++------------------ src/corelib/text/qstring.cpp | 48 ++++++++--------- src/corelib/tools/qarraydataops.h | 45 ++++++++++++++++ src/corelib/tools/qlist.h | 24 +-------- 4 files changed, 104 insertions(+), 100 deletions(-) diff --git a/src/corelib/text/qbytearray.cpp b/src/corelib/text/qbytearray.cpp index 1b5fa8e5eb..d729667ff7 100644 --- a/src/corelib/text/qbytearray.cpp +++ b/src/corelib/text/qbytearray.cpp @@ -1922,39 +1922,34 @@ QByteArray& QByteArray::append(char ch) QByteArray &QByteArray::insert(qsizetype i, QByteArrayView data) { const char *str = data.data(); - qsizetype len = data.size(); - if (i < 0 || str == nullptr || len <= 0) + qsizetype size = data.size(); + if (i < 0 || size <= 0) return *this; - if (points_into_range(str, d.data(), d.data() + d.size)) { - QVarLengthArray a(str, str + len); + // handle this specially, as QArrayDataOps::insert() doesn't handle out of + // bounds positions + if (i >= d->size) { + // In case when data points into the range or is == *this, we need to + // defer a call to free() so that it comes after we copied the data from + // the old memory: + DataPointer detached{}; // construction is free + if (d->needsDetach() || i + size - d->size > d.freeSpaceAtEnd()) { + detached = DataPointer::allocateGrow(d, i + size - d->size, Data::AllocateAtEnd); + detached->copyAppend(d.constBegin(), d.constEnd()); + d.swap(detached); + } + d->copyAppend(i - d->size, ' '); + d->copyAppend(str, str + size); + d.data()[d.size] = '\0'; + return *this; + } + + if (!d->needsDetach() && points_into_range(str, d.data(), d.data() + d.size)) { + QVarLengthArray a(str, str + size); return insert(i, a); } - const auto oldSize = this->size(); - qsizetype sizeToGrow = len; - if (i > oldSize) - sizeToGrow += i - oldSize; - - if (d->needsDetach() || (sizeToGrow > d.freeSpaceAtBegin() && sizeToGrow > d.freeSpaceAtEnd())) { - QArrayData::AllocationPosition pos = QArrayData::AllocateAtEnd; - if (oldSize != 0 && i <= (oldSize >> 1)) - pos = QArrayData::AllocateAtBeginning; - - DataPointer detached(DataPointer::allocateGrow(d, sizeToGrow, pos)); - auto where = d.constBegin() + qMin(i, d->size); - detached->copyAppend(d.constBegin(), where); - if (i > oldSize) - detached->copyAppend(i - oldSize, u' '); - detached->copyAppend(str, str + len); - detached->copyAppend(where, d.constEnd()); - d.swap(detached); - } else { - if (i > oldSize) // set spaces in the uninitialized gap - d->copyAppend(i - oldSize, ' '); - - d->insert(d.begin() + i, str, str + len); - } + d->insert(i, str, size); d.data()[d.size] = '\0'; return *this; } @@ -1995,30 +1990,20 @@ QByteArray &QByteArray::insert(qsizetype i, qsizetype count, char ch) if (i < 0 || count <= 0) return *this; - const auto oldSize = this->size(); - qsizetype sizeToGrow = count; - if (i > oldSize) - sizeToGrow += i - oldSize; - - if (d->needsDetach() || (sizeToGrow > d.freeSpaceAtBegin() && sizeToGrow > d.freeSpaceAtEnd())) { - QArrayData::AllocationPosition pos = QArrayData::AllocateAtEnd; - if (oldSize != 0 && i <= (oldSize >> 1)) - pos = QArrayData::AllocateAtBeginning; - - DataPointer detached(DataPointer::allocateGrow(d, sizeToGrow, pos)); - auto where = d.constBegin() + qMin(i, d->size); - detached->copyAppend(d.constBegin(), where); - if (i > oldSize) - detached->copyAppend(i - oldSize, ' '); - detached->copyAppend(count, ch); - detached->copyAppend(where, d.constEnd()); - d.swap(detached); - } else { - if (i > oldSize) // set spaces in the uninitialized gap - d->copyAppend(i - oldSize, u' '); - - d->insert(d.begin() + i, count, ch); + if (i >= d->size) { + // handle this specially, as QArrayDataOps::insert() doesn't handle out of bounds positions + if (d->needsDetach() || i + count - d->size > d.freeSpaceAtEnd()) { + DataPointer detached(DataPointer::allocateGrow(d, i + count - d->size, Data::AllocateAtEnd)); + detached->copyAppend(d.constBegin(), d.constEnd()); + d.swap(detached); + } + d->copyAppend(i - d->size, ' '); + d->copyAppend(count, ch); + d.data()[d.size] = '\0'; + return *this; } + + d->insert(i, count, ch); d.data()[d.size] = '\0'; return *this; } diff --git a/src/corelib/text/qstring.cpp b/src/corelib/text/qstring.cpp index 40d69c9d9c..6e7809d868 100644 --- a/src/corelib/text/qstring.cpp +++ b/src/corelib/text/qstring.cpp @@ -2728,34 +2728,30 @@ QString& QString::insert(qsizetype i, const QChar *unicode, qsizetype size) return *this; const auto s = reinterpret_cast(unicode); - if (points_into_range(s, d.data(), d.data() + d.size)) + + // handle this specially, as QArrayDataOps::insert() doesn't handle out of + // bounds positions + if (i >= d->size) { + // In case when data points into the range or is == *this, we need to + // defer a call to free() so that it comes after we copied the data from + // the old memory: + DataPointer detached{}; // construction is free + if (d->needsDetach() || i + size - d->size > d.freeSpaceAtEnd()) { + detached = DataPointer::allocateGrow(d, i + size - d->size, Data::AllocateAtEnd); + detached->copyAppend(d.constBegin(), d.constEnd()); + d.swap(detached); + } + d->copyAppend(i - d->size, u' '); + d->copyAppend(s, s + size); + d.data()[d.size] = u'\0'; + return *this; + } + + if (!d->needsDetach() && points_into_range(s, d.data(), d.data() + d.size)) return insert(i, QStringView{QVarLengthArray(s, s + size)}); - const auto oldSize = this->size(); - qsizetype sizeToGrow = size; - if (i > oldSize) - sizeToGrow += i - oldSize; - - if (d->needsDetach() || (sizeToGrow > d.freeSpaceAtBegin() && sizeToGrow > d.freeSpaceAtEnd())) { - QArrayData::AllocationPosition pos = QArrayData::AllocateAtEnd; - if (oldSize != 0 && i <= (oldSize >> 1)) - pos = QArrayData::AllocateAtBeginning; - - DataPointer detached(DataPointer::allocateGrow(d, sizeToGrow, pos)); - auto where = d.constBegin() + qMin(i, d->size); - detached->copyAppend(d.constBegin(), where); - if (i > oldSize) - detached->copyAppend(i - oldSize, u' '); - detached->copyAppend(s, s + size); - detached->copyAppend(where, d.constEnd()); - d.swap(detached); - } else { - if (i > oldSize) // set spaces in the uninitialized gap - d->copyAppend(i - oldSize, u' '); - - d->insert(d.begin() + i, s, s + size); - } - d.data()[d.size] = '\0'; + d->insert(i, s, size); + d.data()[d.size] = u'\0'; return *this; } diff --git a/src/corelib/tools/qarraydataops.h b/src/corelib/tools/qarraydataops.h index 7121da6660..02f52fbefe 100644 --- a/src/corelib/tools/qarraydataops.h +++ b/src/corelib/tools/qarraydataops.h @@ -1024,6 +1024,7 @@ struct QCommonArrayOps : QArrayOpsSelector::Type { using Base = typename QArrayOpsSelector::Type; using Data = QTypedArrayData; + using DataPointer = QArrayDataPointer; using parameter_type = typename Base::parameter_type; using iterator = typename Base::iterator; using const_iterator = typename Base::const_iterator; @@ -1216,6 +1217,50 @@ public: Base::insert(GrowsForwardTag{}, where, qsizetype(n) - beginSize, t); } + void insert(qsizetype i, qsizetype n, parameter_type t) + { + if (this->needsDetach() || (n > this->freeSpaceAtBegin() && n > this->freeSpaceAtEnd())) { + typename Data::AllocationPosition pos = Data::AllocateAtEnd; + if (this->size != 0 && i <= (this->size >> 1)) + pos = Data::AllocateAtBeginning; + + DataPointer detached(DataPointer::allocateGrow(*this, n, pos)); + const_iterator where = this->constBegin() + i; + detached->copyAppend(this->constBegin(), where); + detached->copyAppend(n, t); + detached->copyAppend(where, this->constEnd()); + this->swap(detached); + } else { + // we're detached and we can just move data around + if (i == this->size && n <= this->freeSpaceAtEnd()) { + copyAppend(n, t); + } else { + T copy(t); + insert(this->begin() + i, n, copy); + } + } + } + + void insert(qsizetype i, const T *data, qsizetype n) + { + if (this->needsDetach() || (n > this->freeSpaceAtBegin() && n > this->freeSpaceAtEnd())) { + typename Data::AllocationPosition pos = Data::AllocateAtEnd; + if (this->size != 0 && i <= (this->size >> 1)) + pos = Data::AllocateAtBeginning; + + DataPointer detached(DataPointer::allocateGrow(*this, n, pos)); + auto where = this->constBegin() + i; + detached->copyAppend(this->constBegin(), where); + detached->copyAppend(data, data + n); + detached->copyAppend(where, this->constEnd()); + this->swap(detached); + } else { + insert(this->begin() + i, data, data + n); + } + + } + + template void emplace(iterator where, Args&&... args) { diff --git a/src/corelib/tools/qlist.h b/src/corelib/tools/qlist.h index 960bf18cac..da3a5926ea 100644 --- a/src/corelib/tools/qlist.h +++ b/src/corelib/tools/qlist.h @@ -732,29 +732,7 @@ QList::insert(qsizetype i, qsizetype n, parameter_type t) { Q_ASSERT_X(size_t(i) <= size_t(d->size), "QList::insert", "index out of range"); - // we don't have a quick exit for n == 0 - // it's not worth wasting CPU cycles for that - - if (d->needsDetach() || (n > d.freeSpaceAtBegin() && n > d.freeSpaceAtEnd())) { - typename QArrayData::AllocationPosition pos = QArrayData::AllocateAtEnd; - if (d.size != 0 && i <= (d.size >> 1)) - pos = QArrayData::AllocateAtBeginning; - - DataPointer detached(DataPointer::allocateGrow(d, n, pos)); - const_iterator where = constBegin() + i; - detached->copyAppend(constBegin(), where); - detached->copyAppend(n, t); - detached->copyAppend(where, constEnd()); - d.swap(detached); - } else { - // we're detached and we can just move data around - if (i == size() && n <= d.freeSpaceAtEnd()) { - d->copyAppend(n, t); - } else { - T copy(t); - d->insert(d.begin() + i, n, copy); - } - } + d->insert(i, n, t); return d.begin() + i; }