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>
This commit is contained in:
Andrei Golubev 2020-09-02 15:19:16 +02:00
parent 030962b01c
commit 6e8985e357
2 changed files with 12 additions and 16 deletions

View File

@ -1497,11 +1497,11 @@ public:
Q_ASSERT(b >= this->begin() && b < this->end()); Q_ASSERT(b >= this->begin() && b < this->end());
Q_ASSERT(e > this->begin() && e <= this->end()); Q_ASSERT(e > this->begin() && e <= this->end());
// Qt5 QList in erase: try to move less data around // Comply with std::vector::erase(): erased elements and all after them
// Now: // are invalidated. However, erasing from the beginning effectively
const T *begin = this->begin(); // means that all iterators are invalidated. We can use this freedom to
const T *end = this->end(); // erase by moving towards the end.
if (b - begin < end - e) { if (b == this->begin()) {
Base::erase(GrowsBackwardsTag{}, b, e); Base::erase(GrowsBackwardsTag{}, b, e);
} else { } else {
Base::erase(GrowsForwardTag{}, b, e); Base::erase(GrowsForwardTag{}, b, e);

View File

@ -1090,12 +1090,10 @@ void tst_QArrayData::arrayOps2()
QVERIFY(vs[i].isNull()); QVERIFY(vs[i].isNull());
QCOMPARE(vo[i].id, i); QCOMPARE(vo[i].id, i);
// Erasing not from begin always shifts left - consistency with
// Erasing closer to begin causes smaller region [begin, begin + 2) to // std::vector::erase. Elements before erase position are not affected.
// be moved. Thus, to-be-erased region gets reassigned with the elements
// at the beginning
QCOMPARE(int(vo[i].flags), CountedObject::DefaultConstructed QCOMPARE(int(vo[i].flags), CountedObject::DefaultConstructed
| CountedObject::CopyConstructed | CountedObject::CopyAssigned); | CountedObject::CopyConstructed);
} }
for (size_t i = 2; i < 4; ++i) { for (size_t i = 2; i < 4; ++i) {
@ -1103,9 +1101,8 @@ void tst_QArrayData::arrayOps2()
QVERIFY(vs[i].isNull()); QVERIFY(vs[i].isNull());
QCOMPARE(vo[i].id, i + 8); QCOMPARE(vo[i].id, i + 8);
QCOMPARE(int(vo[i].flags), int(CountedObject::DefaultConstructed)
// Erasing closer to begin does not affect [begin + 2, begin + 4) region | CountedObject::CopyAssigned);
QCOMPARE(int(vo[i].flags), CountedObject::DefaultConstructed);
} }
for (size_t i = 4; i < 7; ++i) { for (size_t i = 4; i < 7; ++i) {
@ -1113,9 +1110,8 @@ void tst_QArrayData::arrayOps2()
QCOMPARE(vs[i], QString::number(i + 3)); QCOMPARE(vs[i], QString::number(i + 3));
QCOMPARE(vo[i].id, i + 3); QCOMPARE(vo[i].id, i + 3);
QCOMPARE(int(vo[i].flags), CountedObject::DefaultConstructed
// Erasing closer to begin does not affect [begin + 2, begin + 4) region | CountedObject::CopyAssigned);
QCOMPARE(int(vo[i].flags), CountedObject::DefaultConstructed);
} }
} }