QItemSelection: fix a quadratic loop

QList::erase(it) is a linear operation. Calling it in a loop
constitutes quadratic behavior.

Fix by using the erase-remove idiom.

Write a generic function object QtFunctionObjects::IsNotValid, instead
of a lambda, because I am building up a collection of such function
objects in my tree, to be submitted any time now™, so this will reduce
the churn once that gets merged. Wrap it in the unnamed namespace to
avoid ODR violations in case this pattern repeats.

Replace an existing erase-remove idiom's isNotValid lambda function
with the new function object.

Change-Id: I4f0af04e7b201eb168e79beeda332eee3c6f33c3
Reviewed-by: Anton Kudryavtsev <antkudr@mail.ru>
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>

1 files changed, 20 insertions, 12 deletions

diff --git a/src/corelib/itemmodels/qitemselectionmodel.cpp b/src/corelib/itemmodels/qitemselectionmodel.cpp
index 2ff311bc5a..8dcd80808b 100644
--- a/src/corelib/itemmodels/qitemselectionmodel.cpp
+++ b/src/corelib/itemmodels/qitemselectionmodel.cpp
@@ -1267,6 +1267,21 @@ void QItemSelectionModel::select(const QModelIndex &index, QItemSelectionModel::
                           convenience.
 */
 
+namespace {
+namespace QtFunctionObjects {
+struct IsNotValid {
+    typedef bool result_type;
+    struct is_transparent : std::true_type {};
+    template <typename T>
+    Q_DECL_CONSTEXPR bool operator()(T &t) const Q_DECL_NOEXCEPT_EXPR(noexcept(t.isValid()))
+    { return !t.isValid(); }
+    template <typename T>
+    Q_DECL_CONSTEXPR bool operator()(T *t) const Q_DECL_NOEXCEPT_EXPR(noexcept(t->isValid()))
+    { return !t->isValid(); }
+};
+}
+} // unnamed namespace
+
 /*!
     Selects the item \a selection using the specified \a command, and emits
     selectionChanged().
@@ -1290,13 +1305,9 @@ void QItemSelectionModel::select(const QItemSelection &selection, QItemSelection
     // be too late if another model observer is connected to the same modelReset slot and is invoked first
     // it might call select() on this selection model before any such QItemSelectionModelPrivate::_q_modelReset() slot
     // is invoked, so it would not be cleared yet. We clear it invalid ranges in it here.
-    QItemSelection::iterator it = d->ranges.begin();
-    while (it != d->ranges.end()) {
-      if (!it->isValid())
-        it = d->ranges.erase(it);
-      else
-        ++it;
-    }
+    using namespace QtFunctionObjects;
+    d->ranges.erase(std::remove_if(d->ranges.begin(), d->ranges.end(), IsNotValid()),
+                    d->ranges.end());
 
     QItemSelection old = d->ranges;
     old.merge(d->currentSelection, d->currentCommand);
@@ -1748,12 +1759,9 @@ const QItemSelection QItemSelectionModel::selection() const
     selected.merge(d->currentSelection, d->currentCommand);
     // make sure we have no invalid ranges
     // ###  should probably be handled more generic somewhere else
-    auto isNotValid = [](const QItemSelectionRange& range) {
-        return !range.isValid();
-    };
-
+    using namespace QtFunctionObjects;
     selected.erase(std::remove_if(selected.begin(), selected.end(),
-                                  isNotValid),
+                                  IsNotValid()),
                    selected.end());
     return selected;
 }