whytofly
/
qt6-bb10
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QFlatMap: make insertion STL-compatible 

That is, insert() doesn't overwrite an existing entry, and range
insert inserts the first of equivalent keys' values, not the last.

This allowed this author to optimize the implementation of
makeUnique() to a O(N) algorithm (was: O(N²)). Said optimization would
have been possible with the old semantics, too, but I wrote the
algorithm first and only then noticed the broken insert() behavior is
present on QFlatMap, too, so I decided not to let good code go to
waste and to fix both problems at the same time.

In order to give users a hint of the changed semantics, make the new
API opt-in until Qt 6.5, so Qt 6.4 ships with the both the old and the
new semantics disabled, where they contradict.

Fixes: QTBUG-100092
Change-Id: Ic96d8bfe6bed9068dbe8c0d7171bd8921050fd95
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
bb10
Marc Mutz 2022-01-25 13:51:28 +01:00
parent e516a7bcf8
commit 1977c922e9
2 changed files with 54 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
src/corelib/tools/qflatmap_p.h
View File

@ -78,6 +78,19 @@ QT_BEGIN_NAMESPACE
QFlatMap<float, int, std::less<float>, std::vector<float>, std::vector<int>>
*/
// Qt 6.4:
// - removed QFlatMap API which was incompatible with STL semantics
// - will be released with said API disabled, to catch any out-of-tree users
// - also allows opting in to the new API using QFLATMAP_ENABLE_STL_COMPATIBLE_INSERT
// Qt 6.5
// - will make QFLATMAP_ENABLE_STL_COMPATIBLE_INSERT the default:
#ifndef QFLATMAP_ENABLE_STL_COMPATIBLE_INSERT
# if QT_VERSION >= QT_VERSION_CHECK(6, 5, 0)
# define QFLATMAP_ENABLE_STL_COMPATIBLE_INSERT
# endif
#endif
namespace Qt {
struct OrderedUniqueRange_t {};
@ -431,7 +444,7 @@ private:
public:
QFlatMap() = default;
#ifdef QFLATMAP_TEMPORARILY_REMOVED
#ifdef QFLATMAP_ENABLE_STL_COMPATIBLE_INSERT
explicit QFlatMap(const key_container_type &keys, const mapped_container_type &values)
: c{keys, values}
{
@ -509,7 +522,7 @@ public:
{
}
#ifdef QFLATMAP_TEMPORARILY_REMOVED
#ifdef QFLATMAP_ENABLE_STL_COMPATIBLE_INSERT
explicit QFlatMap(const key_container_type &keys, const mapped_container_type &values,
const Compare &compare)
: value_compare(compare), c{keys, values}
@ -690,25 +703,25 @@ public:
return value(key);
}
#ifdef QFLATMAP_TEMPORARILY_REMOVED
#ifdef QFLATMAP_ENABLE_STL_COMPATIBLE_INSERT
std::pair<iterator, bool> insert(const Key &key, const T &value)
{
return insert_or_assign(key, value);
return try_emplace(key, value);
}
std::pair<iterator, bool> insert(Key &&key, const T &value)
{
return insert_or_assign(std::move(key), value);
return try_emplace(std::move(key), value);
}
std::pair<iterator, bool> insert(const Key &key, T &&value)
{
return insert_or_assign(key, std::move(value));
return try_emplace(key, std::move(value));
}
std::pair<iterator, bool> insert(Key &&key, T &&value)
{
return insert_or_assign(std::move(key), std::move(value));
return try_emplace(std::move(key), std::move(value));
}
#endif
@ -754,7 +767,7 @@ public:
return r;
}
#ifdef QFLATMAP_TEMPORARILY_REMOVED
#ifdef QFLATMAP_ENABLE_STL_COMPATIBLE_INSERT
template <class InputIt, is_compatible_iterator<InputIt> = nullptr>
void insert(InputIt first, InputIt last)
{
@ -1078,24 +1091,39 @@ private:
void makeUnique()
{
if (c.keys.size() < 2)
// std::unique, but over two ranges
auto equivalent = [this](const auto &lhs, const auto &rhs) {
return !key_compare::operator()(lhs, rhs) && !key_compare::operator()(rhs, lhs);
};
const auto kb = c.keys.begin();
const auto ke = c.keys.end();
auto k = std::adjacent_find(kb, ke, equivalent);
if (k == ke)
return;
auto k = std::end(c.keys) - 1;
auto i = k - 1;
for (;;) {
if (key_compare::operator()(*i, *k) || key_compare::operator()(*k, *i)) {
if (i == std::begin(c.keys))
break;
--i;
--k;
} else {
c.values.erase(std::begin(c.values) + std::distance(std::begin(c.keys), i));
i = c.keys.erase(i);
if (i == std::begin(c.keys))
break;
k = i + 1;
// equivalent keys found, we need to do actual work:
auto v = std::next(c.values.begin(), std::distance(kb, k));
auto kdest = k;
auto vdest = v;
++k;
++v;
// Loop Invariants:
//
// - [keys.begin(), kdest] and [values.begin(), vdest] are unique
// - k is not keys.end(), v is not values.end()
// - [next(k), keys.end()[ and [next(v), values.end()[ still need to be checked
while ((++v, ++k) != ke) {
if (!equivalent(*kdest, *k)) {
*++kdest = std::move(*k);
*++vdest = std::move(*v);
}
}
c.keys.erase(std::next(kdest), ke);
c.values.erase(std::next(vdest), c.values.end());
}
containers c;

19
tests/auto/corelib/tools/qflatmap/tst_qflatmap.cpp
View File

@ -27,6 +27,7 @@
****************************************************************************/
#define QT_USE_QSTRINGBUILDER
#define QFLATMAP_ENABLE_STL_COMPATIBLE_INSERT
#include <QTest>
@ -58,11 +59,9 @@ class tst_QFlatMap : public QObject
{
Q_OBJECT
private slots:
#ifdef QFLATMAP_TEMPORARILY_REMOVED
void constructing();
void constAccess();
void insertion();
#endif
void insertRValuesAndLValues();
void removal();
void extraction();
@ -84,7 +83,6 @@ private:
void remove_if_impl(Predicate p, bool removeNonEmptyValues = false);
};
#ifdef QFLATMAP_TEMPORARILY_REMOVED
void tst_QFlatMap::constructing()
{
using Map = QFlatMap<int, QByteArray>;
@ -169,7 +167,7 @@ void tst_QFlatMap::insertion()
QCOMPARE(m.value("foo").data(), "FOO");
QCOMPARE(m.value("bar").data(), "BAR");
QCOMPARE(m.value("baz").data(), "BAZ");
QCOMPARE(m.value("oof").data(), "OOF");
QCOMPARE(m.value("oof").data(), "eek");
QCOMPARE(m.value("bla").data(), "BLA");
QCOMPARE(m.value("blubb").data(), "BLUBB");
@ -183,14 +181,12 @@ void tst_QFlatMap::insertion()
m.insert(std::begin(a1), std::end(a1));
m.insert(Qt::OrderedUniqueRange, std::begin(a2), std::end(a2));
QCOMPARE(m.size(), 10);
QCOMPARE(m.value("narf").data(), "NARFFFFFF");
QCOMPARE(m.value("narf").data(), "NARF");
QCOMPARE(m.value("gnampf").data(), "GNAMPF");
}
#endif
void tst_QFlatMap::insertRValuesAndLValues()
{
#ifdef QFLATMAP_TEMPORARILY_REMOVED
using Map = QFlatMap<QByteArray, QByteArray>;
const QByteArray foo = QByteArrayLiteral("foo");
const QByteArray bar = QByteArrayLiteral("bar");
@ -223,7 +219,6 @@ void tst_QFlatMap::insertRValuesAndLValues()
}
#undef lvalue
#endif
}
void tst_QFlatMap::extraction()
@ -458,7 +453,6 @@ void tst_QFlatMap::remove_if_impl(Pred p, bool removeNonEmptyValues)
void tst_QFlatMap::removal()
{
#ifdef QFLATMAP_TEMPORARILY_REMOVED
using Map = QFlatMap<int, QByteArray>;
Map m({ { 2, "bar" }, { 3, "baz" }, { 1, "foo" } });
QCOMPARE(m.value(2).data(), "bar");
@ -481,12 +475,10 @@ void tst_QFlatMap::removal()
it = m.erase(it);
QCOMPARE(it.key(), 2);
QVERIFY(!m.contains(1));
#endif
}
void tst_QFlatMap::statefulComparator()
{
#ifdef QFLATMAP_TEMPORARILY_REMOVED
struct CountingCompare {
mutable int count = 0;
@ -504,7 +496,6 @@ void tst_QFlatMap::statefulComparator()
QCOMPARE(m2.key_comp().count, m1.key_comp().count);
m2.insert(m1.begin(), m1.end());
QVERIFY(m2.key_comp().count > m1.key_comp().count);
#endif
}
void tst_QFlatMap::transparency_using()
@ -536,7 +527,6 @@ void tst_QFlatMap::transparency_struct()
template <typename StringViewCompare>
void tst_QFlatMap::transparency_impl()
{
#ifdef QFLATMAP_TEMPORARILY_REMOVED
using Map = QFlatMap<QString, QString, StringViewCompare>;
auto m = Map{ { "one", "een" }, { "two", "twee" }, { "three", "dree" } };
@ -560,7 +550,6 @@ void tst_QFlatMap::transparency_impl()
QVERIFY(m.contains(QLatin1String("one")));
QVERIFY(m.remove(QAnyStringView(u8"one")));
QVERIFY(!m.contains(QLatin1String("one")));
#endif
}
void tst_QFlatMap::try_emplace_and_insert_or_assign()
@ -707,7 +696,6 @@ void tst_QFlatMap::try_emplace_and_insert_or_assign()
void tst_QFlatMap::viewIterators()
{
#ifdef QFLATMAP_TEMPORARILY_REMOVED
using Map = QFlatMap<QByteArray, QByteArray>;
Map m({ { "yksi", "een"}, { "kaksi", "twee" }, { "kolme", "dree" } });
{
@ -752,7 +740,6 @@ void tst_QFlatMap::viewIterators()
it--;
QCOMPARE(*it, "dree");
}
#endif
}
void tst_QFlatMap::varLengthArray()