QString, QByteArray: don't detach in removeIf/erase/eraseif()

If the object is shared, instead of detaching, copy characters from
"this" to a new object except for the chacters that would be erased,
this is more efficient than detaching (which would copy the whole data
then erase).

- Extend tst_QString::removeIf() to catch a corner-case (that I saw
  with tst_QByteArray::removeIf()).

- Add q_uninitialized_remove_copy_if, which works like
  std::remove_copy_if but for uninitialized memory like
  q_uninitialized_relocate_n (but copies rather than relocates/moves).
  With the same static_assert from q_relocate_overlap_n that the type
  destructor is non-throwing.

Added q_uninitialized_remove_copy_if in this commit rather than a
separate one so that it's unittested by its usage in eraseIf().

[ChangeLog][QtCore][QString, QByteArray] Removing characters from a
currently shared string or byte array is now done more efficiently

Task-number: QTBUG-106181
Task-number: QTBUG-106183
Change-Id: Icc0ed31633cef71d482b97e0d2d20d763163d383
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/corelib/text/qbytearray.h

@@ -247,7 +247,7 @@ public:
     template <typename Predicate>
     QByteArray &removeIf(Predicate pred)
     {
-        QtPrivate::sequential_erase_if(*this, pred);
+        removeIf_helper(pred);
         return *this;
     }
 
@@ -478,9 +478,20 @@ private:
     static QByteArray trimmed_helper(QByteArray &a);
     static QByteArray simplified_helper(const QByteArray &a);
     static QByteArray simplified_helper(QByteArray &a);
+    template <typename Predicate>
+    qsizetype removeIf_helper(Predicate pred)
+    {
+        const qsizetype result = d->eraseIf(pred);
+        if (result > 0)
+            d.data()[d.size] = '\0';
+        return result;
+    }
 
     friend class QString;
     friend Q_CORE_EXPORT QByteArray qUncompress(const uchar *data, qsizetype nbytes);
+
+    template <typename T> friend qsizetype erase(QByteArray &ba, const T &t);
+    template <typename Predicate> friend qsizetype erase_if(QByteArray &ba, Predicate pred);
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS(QByteArray::Base64Options)
@@ -662,13 +673,13 @@ Q_CORE_EXPORT Q_DECL_PURE_FUNCTION size_t qHash(const QByteArray::FromBase64Resu
 template <typename T>
 qsizetype erase(QByteArray &ba, const T &t)
 {
-    return QtPrivate::sequential_erase(ba, t);
+    return ba.removeIf_helper([&t](const auto &e) { return t == e; });
 }
 
 template <typename Predicate>
 qsizetype erase_if(QByteArray &ba, Predicate pred)
 {
-    return QtPrivate::sequential_erase_if(ba, pred);
+    return ba.removeIf_helper(pred);
 }
 
 //

src/corelib/text/qstring.h

@@ -406,9 +406,10 @@ public:
     template <typename Predicate>
     QString &removeIf(Predicate pred)
     {
-        QtPrivate::sequential_erase_if(*this, pred);
+        removeIf_helper(pred);
         return *this;
     }
+
     QString &replace(qsizetype i, qsizetype len, QChar after);
     QString &replace(qsizetype i, qsizetype len, const QChar *s, qsizetype slen);
     QString &replace(qsizetype i, qsizetype len, const QString &after);
@@ -865,10 +866,20 @@ private:
     static qsizetype toUcs4_helper(const char16_t *uc, qsizetype length, char32_t *out);
     static qlonglong toIntegral_helper(QStringView string, bool *ok, int base);
     static qulonglong toIntegral_helper(QStringView string, bool *ok, uint base);
+    template <typename Predicate>
+    qsizetype removeIf_helper(Predicate pred)
+    {
+        const qsizetype result = d->eraseIf(pred);
+        if (result > 0)
+            d.data()[d.size] = u'\0';
+        return result;
+    }
 
     friend class QStringView;
     friend class QByteArray;
     friend struct QAbstractConcatenable;
+    template <typename T> friend qsizetype erase(QString &s, const T &t);
+    template <typename Predicate> friend qsizetype erase_if(QString &s, Predicate pred);
 
     template <typename T> static
     T toIntegral_helper(QStringView string, bool *ok, int base)
@@ -1345,13 +1356,13 @@ QString QLatin1StringView::arg(Args &&...args) const
 template <typename T>
 qsizetype erase(QString &s, const T &t)
 {
-    return QtPrivate::sequential_erase(s, t);
+    return s.removeIf_helper([&t](const auto &e) { return t == e; });
 }
 
 template <typename Predicate>
 qsizetype erase_if(QString &s, Predicate pred)
 {
-    return QtPrivate::sequential_erase_if(s, pred);
+    return s.removeIf_helper(pred);
 }
 
 namespace Qt {

src/corelib/tools/qarraydataops.h

@@ -35,6 +35,8 @@ protected:
 public:
     typedef typename QArrayDataPointer<T>::parameter_type parameter_type;
 
+    using QArrayDataPointer<T>::QArrayDataPointer;
+
     void appendInitialize(qsizetype newSize) noexcept
     {
         Q_ASSERT(this->isMutable());
@@ -213,6 +215,40 @@ public:
         --this->size;
     }
 
+    template <typename Predicate>
+    qsizetype eraseIf(Predicate pred)
+    {
+        qsizetype result = 0;
+        if (this->size == 0)
+            return result;
+
+        if (!this->needsDetach()) {
+            auto end = this->end();
+            auto it = std::remove_if(this->begin(), end, pred);
+            if (it != end) {
+                result = std::distance(it, end);
+                erase(it, result);
+            }
+        } else {
+            const auto begin = this->begin();
+            const auto end = this->end();
+            auto it = std::find_if(begin, end, pred);
+            if (it == end)
+                return result;
+
+            QPodArrayOps<T> other{ Data::allocate(this->size), this->size };
+            Q_CHECK_PTR(other.data());
+            auto dest = other.begin();
+            // std::uninitialized_copy will fallback to ::memcpy/memmove()
+            dest = std::uninitialized_copy(begin, it, dest);
+            dest = q_uninitialized_remove_copy_if(std::next(it), end, dest, pred);
+            other.size = std::distance(other.data(), dest);
+            result = this->size - other.size;
+            this->swap(other);
+        }
+        return result;
+    }
+
     struct Span { T *begin; T *end; };
 
     void copyRanges(std::initializer_list<Span> ranges)

src/corelib/tools/qcontainertools_impl.h

@@ -86,6 +86,41 @@ void q_uninitialized_relocate_n(T* first, N n, T* out)
 
 QT_WARNING_POP
 
+/*!
+    \internal
+    Copies all elements, except the ones for which \a pred returns \c true, from
+    range [first, last), to the uninitialized memory buffer starting at \a out.
+
+    It's undefined behavior if \a out points into [first, last).
+
+    Returns a pointer one past the last copied element.
+
+    If an exception is thrown, all the already copied elements in the destination
+    buffer are destroyed.
+*/
+template <typename T, typename Predicate>
+T *q_uninitialized_remove_copy_if(T *first, T *last, T *out, Predicate &pred)
+{
+    static_assert(std::is_nothrow_destructible_v<T>,
+                  "This algorithm requires that T has a non-throwing destructor");
+    Q_ASSERT(!q_points_into_range(out, first, last));
+
+    T *dest_begin = out;
+    QT_TRY {
+        while (first != last) {
+            if (!pred(*first)) {
+                new (std::addressof(*out)) T(*first);
+                ++out;
+            }
+            ++first;
+        }
+    } QT_CATCH (...) {
+        std::destroy(std::reverse_iterator(out), std::reverse_iterator(dest_begin));
+        QT_RETHROW;
+    }
+    return out;
+}
+
 template<typename iterator, typename N>
 void q_relocate_overlap_n_left_move(iterator first, N n, iterator d_first)
 {

tests/auto/corelib/text/qbytearray/tst_qbytearray.cpp

@@ -1096,7 +1096,15 @@ void tst_QByteArray::removeIf()
     QVERIFY(!a.isDetached());
 
     a = QByteArray("aBcAbC");
+    // Test when it's not shared
+    QVERIFY(a.isDetached());
     QCOMPARE(a.removeIf(removeA), QByteArray("BcbC"));
+
+    a = QByteArray("aBcAbC");
+    QByteArray b = a;
+    // Test when it's shared
+    QVERIFY(!b.isDetached());
+    QCOMPARE(b.removeIf(removeA), QByteArray("BcbC"));
 }
 
 void tst_QByteArray::erase()

tests/auto/corelib/text/qstring/tst_qstring.cpp

@@ -8136,17 +8136,36 @@ void tst_QString::chopped()
 
 void tst_QString::removeIf()
 {
+    const char str[] = "aABbcCDd";
     QString a;
 
     auto pred = [](const QChar &c) { return c.isLower(); };
-
     a.removeIf(pred);
     QVERIFY(a.isEmpty());
     QVERIFY(!a.isDetached());
 
-    a = "aABbcCDd";
+    // Test when the string is not shared
+    a = str;
+    QVERIFY(!a.data_ptr()->needsDetach());
     a.removeIf(pred);
     QCOMPARE(a, u"ABCD");
+
+    // Test when the string is shared
+    a = str;
+    QString b = a;
+    QVERIFY(a.data_ptr()->needsDetach());
+    a.removeIf(pred);
+    QCOMPARE(a, u"ABCD");
+    QCOMPARE(b, str);
+
+    auto removeA = [](const char c) { return c == 'a' || c == 'A'; };
+
+    a = "aBcAbCa"; // Not shared
+    QCOMPARE(a.removeIf(removeA), QString("BcbC"));
+
+    a = "aBcAbCa";
+    b = a; // Shared
+    QCOMPARE(a.removeIf(removeA), QString("BcbC"));
 }
 
 // QString's collation order is only supported during the lifetime as QCoreApplication