AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QString: don't detach in remove(QChar ch, Qt::CaseSensitivity cs)

Browse Source 
- If the string isn't shared, don't call detach(), instead remove characters
  matching ch, and resize()
- If the string is shared, create a new string, and copy all characters
  except the ones that would be removed, see task for details

Update unittets so that calls to this overload of remove() test both code
paths (replace() calls remove(QChar, cs) internally).

Drive-by change: use QCOMPARE() instead of QTEST()

Task-number: QTBUG-106181
Change-Id: I1fa08cf29baac2560fca62861fc4a81967b54e92
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Ahmad Samir 2022-10-25 09:19:51 +02:00
parent 358b7a9e74
commit b8b675014f
2 changed files with 44 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
src/corelib/text/qstring.cpp
View File

@ -3416,13 +3416,26 @@ QString &QString::remove(QChar ch, Qt::CaseSensitivity cs)
return ch == (isCase ? x : x.toCaseFolded());
};
detach();
auto begin = d.begin();
auto first_match = begin + idx;
auto end = d.end();
auto it = std::remove_if(first_match, end, match);
d->erase(it, std::distance(it, end));
d.data()[d.size] = u'\0';
if (!d->isShared()) {
auto it = std::remove_if(first_match, end, match);
d->erase(it, std::distance(it, end));
d.data()[d.size] = u'\0';
} else {
// Instead of detaching, create a new string and copy all characters except for
// the ones we're removing
// TODO: size() is more than the needed since "copy" would be shorter
QString copy{size(), Qt::Uninitialized};
auto dst = copy.d.begin();
auto it = std::copy(begin, first_match, dst); // Chunk before idx
it = std::remove_copy_if(first_match + 1, end, it, match);
copy.d.size = std::distance(dst, it);
copy.d.data()[copy.d.size] = u'\0';
*this = copy;
}
return *this;
}

36
tests/auto/corelib/text/qstring/tst_qstring.cpp
View File

@ -830,6 +830,7 @@ void tst_QString::replace_string_data()
QTest::newRow( "rem20" ) << QString("a") << QString("A") << QString("") << QString("") << false;
QTest::newRow( "rem21" ) << QString("A") << QString("a") << QString("") << QString("") << false;
QTest::newRow( "rem22" ) << QString("Alpha beta") << QString("a") << QString("") << QString("lph bet") << false;
QTest::newRow( "rem23" ) << QString("+00:00") << QString(":") << QString("") << QString("+0000") << false;
QTest::newRow( "rep00" ) << QString("ABC") << QString("B") << QString("-") << QString("A-C") << true;
QTest::newRow( "rep01" ) << QString("$()*+.?[\\]^{|}") << QString("$()*+.?[\\]^{|}") << QString("X") << QString("X") << true;
@ -3208,26 +3209,35 @@ void tst_QString::replace_string()
QFETCH( QString, before );
QFETCH( QString, after );
QFETCH( bool, bcs );
QFETCH(QString, result);
Qt::CaseSensitivity cs = bcs ? Qt::CaseSensitive : Qt::CaseInsensitive;
if ( before.size() == 1 ) {
QChar ch = before.at( 0 );
// Test when isShared() is true
QString s1 = string;
s1.replace( ch, after, cs );
QTEST( s1, "result" );
QCOMPARE(s1, result);
QString s4 = string;
s4.begin(); // Test when isShared() is false
s4.replace(ch, after, cs);
QCOMPARE(s4, result);
// What is this one testing? it calls the same replace() overload
// as the previous two
if ( QChar(ch.toLatin1()) == ch ) {
QString s2 = string;
s2.replace( ch.toLatin1(), after, cs );
QTEST( s2, "result" );
QCOMPARE(s2, result);
}
}
QString s3 = string;
s3.replace( before, after, cs );
QTEST( s3, "result" );
QCOMPARE(s3, result);
}
void tst_QString::replace_string_extra()
@ -3343,6 +3353,7 @@ void tst_QString::remove_string()
QFETCH( QString, before );
QFETCH( QString, after );
QFETCH( bool, bcs );
QFETCH(QString, result);
Qt::CaseSensitivity cs = bcs ? Qt::CaseSensitive : Qt::CaseInsensitive;
@ -3350,28 +3361,35 @@ void tst_QString::remove_string()
if ( before.size() == 1 && cs ) {
QChar ch = before.at( 0 );
// Test when isShared() is true
QString s1 = string;
s1.remove( ch );
QTEST( s1, "result" );
QCOMPARE(s1, result);
// Test again with isShared() is false
QString s4 = string;
s4.begin(); // Detach
s4.remove( ch );
QCOMPARE(s4, result);
// What is this one testing? it calls the same remove() overload
// as the previous two
if ( QChar(ch.toLatin1()) == ch ) {
QString s2 = string;
s2.remove( ch );
QTEST( s2, "result" );
QCOMPARE(s2, result);
}
}
QString s3 = string;
s3.remove( before, cs );
QTEST( s3, "result" );
QCOMPARE(s3, result);
if (QtPrivate::isLatin1(before)) {
QString s6 = string;
s6.remove( QLatin1String(before.toLatin1()), cs );
QTEST( s6, "result" );
QCOMPARE(s6, result);
}
} else {
QCOMPARE( 0, 0 ); // shut Qt Test
}
}