Don't rely on iterators being stable while modifying the hash

The Mapping struct referred to where it got inserted into the
source_index mapping table by a const iterator to the table. That
is rather fragile, as changing the table invalidates the iterator.

It happened to work with QHash in Qt 5, but will break with the new
implementation in Qt 6.

Instead simply store the key in the Mapping struct so that it can
be quickly found in the hash.

Also fix one place, where we unconditionally call erase on an iterator
returned by constFind(). Turns out constFind() did sometimes not find
the item in question and returns end().

Change-Id: I0420a06d496f640a3150478e8c644d4cc669ceff
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
This commit is contained in:
Lars Knoll 2020-01-28 19:31:24 +01:00
parent dfaa61482c
commit 7caaf4eb7f
2 changed files with 9 additions and 6 deletions

View File

@ -852,7 +852,9 @@ void QAbstractItemModelPrivate::rowsRemoved(const QModelIndex &parent,
for (QVector<QPersistentModelIndexData *>::const_iterator it = persistent_invalidated.constBegin();
it != persistent_invalidated.constEnd(); ++it) {
QPersistentModelIndexData *data = *it;
persistent.indexes.erase(persistent.indexes.constFind(data->index));
auto pit = persistent.indexes.constFind(data->index);
if (pit != persistent.indexes.cend())
persistent.indexes.erase(pit);
data->index = QModelIndex();
}
}

View File

@ -280,7 +280,7 @@ public:
QVector<int> proxy_rows;
QVector<int> proxy_columns;
QVector<QModelIndex> mapped_children;
QHash<QModelIndex, Mapping *>::const_iterator map_iter;
QModelIndex source_parent;
};
mutable QHash<QModelIndex, Mapping*> source_index_mapping;
@ -321,7 +321,7 @@ public:
const void *p = proxy_index.internalPointer();
Q_ASSERT(p);
QHash<QModelIndex, Mapping *>::const_iterator it =
static_cast<const Mapping*>(p)->map_iter;
source_index_mapping.constFind(static_cast<const Mapping*>(p)->source_parent);
Q_ASSERT(it != source_index_mapping.constEnd());
Q_ASSERT(it.value());
return it;
@ -517,8 +517,7 @@ IndexMap::const_iterator QSortFilterProxyModelPrivate::create_mapping(
m->proxy_columns.resize(source_cols);
build_source_to_proxy_mapping(m->source_columns, m->proxy_columns);
it = IndexMap::const_iterator(source_index_mapping.insert(source_parent, m));
m->map_iter = it;
m->source_parent = source_parent;
if (source_parent.isValid()) {
QModelIndex source_grand_parent = source_parent.parent();
@ -527,6 +526,7 @@ IndexMap::const_iterator QSortFilterProxyModelPrivate::create_mapping(
it2.value()->mapped_children.append(source_parent);
}
it = IndexMap::const_iterator(source_index_mapping.insert(source_parent, m));
Q_ASSERT(it != source_index_mapping.constEnd());
Q_ASSERT(it.value());
@ -1169,7 +1169,8 @@ void QSortFilterProxyModelPrivate::updateChildrenMapping(const QModelIndex &sour
// reinsert moved, mapped indexes
QVector<QPair<QModelIndex, Mapping*> >::iterator it = moved_source_index_mappings.begin();
for (; it != moved_source_index_mappings.end(); ++it) {
(*it).second->map_iter = QHash<QModelIndex, Mapping *>::const_iterator(source_index_mapping.insert((*it).first, (*it).second));
it->second->source_parent = it->first;
source_index_mapping.insert(it->first, it->second);
}
}