diff options
| author | Andrei Golubev <andrei.golubev@qt.io> | 2020-09-02 15:19:16 +0200 |
|---|---|---|
| committer | Andrei Golubev <andrei.golubev@qt.io> | 2020-09-10 11:24:19 +0200 |
| commit | 6e8985e3576a4439bd66c0767f9912d1e124682c (patch) | |
| tree | b728bc9b0db96215af3a9bf6662b9ef1b0907c0d | |
| parent | 030962b01c72ffd6e25c8322a3ee6957027d3278 (diff) | |
Make Q*ArrayOps erase aligned with std::vector::erase
Scoped GrowsBackwards-optimized erase to only be applied
when erase starts at the beginning of the element range.
In other cases, old "left-shifting" erase is used to align
with std::vector::erase invalidation policy
Task-number: QTBUG-84320
Change-Id: I2e7f3b96b056bc371119eb2d36cc7c74af52c394
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
| -rw-r--r-- | src/corelib/tools/qarraydataops.h | 10 | ||||
| -rw-r--r-- | tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp | 18 |
2 files changed, 12 insertions, 16 deletions
diff --git a/src/corelib/tools/qarraydataops.h b/src/corelib/tools/qarraydataops.h index db795e24135..0230677330c 100644 --- a/src/corelib/tools/qarraydataops.h +++ b/src/corelib/tools/qarraydataops.h @@ -1497,11 +1497,11 @@ public: Q_ASSERT(b >= this->begin() && b < this->end()); Q_ASSERT(e > this->begin() && e <= this->end()); - // Qt5 QList in erase: try to move less data around - // Now: - const T *begin = this->begin(); - const T *end = this->end(); - if (b - begin < end - e) { + // Comply with std::vector::erase(): erased elements and all after them + // are invalidated. However, erasing from the beginning effectively + // means that all iterators are invalidated. We can use this freedom to + // erase by moving towards the end. + if (b == this->begin()) { Base::erase(GrowsBackwardsTag{}, b, e); } else { Base::erase(GrowsForwardTag{}, b, e); diff --git a/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp b/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp index 2a1294a8774..7eaa388118f 100644 --- a/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp +++ b/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp @@ -1090,12 +1090,10 @@ void tst_QArrayData::arrayOps2() QVERIFY(vs[i].isNull()); QCOMPARE(vo[i].id, i); - - // Erasing closer to begin causes smaller region [begin, begin + 2) to - // be moved. Thus, to-be-erased region gets reassigned with the elements - // at the beginning + // Erasing not from begin always shifts left - consistency with + // std::vector::erase. Elements before erase position are not affected. QCOMPARE(int(vo[i].flags), CountedObject::DefaultConstructed - | CountedObject::CopyConstructed | CountedObject::CopyAssigned); + | CountedObject::CopyConstructed); } for (size_t i = 2; i < 4; ++i) { @@ -1103,9 +1101,8 @@ void tst_QArrayData::arrayOps2() QVERIFY(vs[i].isNull()); QCOMPARE(vo[i].id, i + 8); - - // Erasing closer to begin does not affect [begin + 2, begin + 4) region - QCOMPARE(int(vo[i].flags), CountedObject::DefaultConstructed); + QCOMPARE(int(vo[i].flags), int(CountedObject::DefaultConstructed) + | CountedObject::CopyAssigned); } for (size_t i = 4; i < 7; ++i) { @@ -1113,9 +1110,8 @@ void tst_QArrayData::arrayOps2() QCOMPARE(vs[i], QString::number(i + 3)); QCOMPARE(vo[i].id, i + 3); - - // Erasing closer to begin does not affect [begin + 2, begin + 4) region - QCOMPARE(int(vo[i].flags), CountedObject::DefaultConstructed); + QCOMPARE(int(vo[i].flags), CountedObject::DefaultConstructed + | CountedObject::CopyAssigned); } } |