Skip to content

Commit

Permalink
less copies and allocations in replaceInPlace
Browse files Browse the repository at this point in the history
  • Loading branch information
@mjerabek @horenmar
mjerabek authored and horenmar committed Mar 1, 2024
1 parent cde3509 commit 4d8affc
Show file tree
Hide file tree
Showing 15 changed files with 318 additions and 102 deletions.
27 changes: 20 additions & 7 deletions src/catch2/internal/catch_string_manip.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// https://www.boost.org/LICENSE_1_0.txt)

// SPDX-License-Identifier: BSL-1.0
#include <catch2/internal/catch_move_and_forward.hpp>
#include <catch2/internal/catch_string_manip.hpp>
#include <catch2/internal/catch_stringref.hpp>

Expand Down Expand Up @@ -65,17 +66,29 @@ namespace Catch {
}

bool replaceInPlace( std::string& str, std::string const& replaceThis, std::string const& withThis ) {
bool replaced = false;
std::size_t i = str.find( replaceThis );
while( i != std::string::npos ) {
replaced = true;
str = str.substr( 0, i ) + withThis + str.substr( i+replaceThis.size() );
if( i < str.size()-withThis.size() )
i = str.find( replaceThis, i+withThis.size() );
if (i == std::string::npos) {
return false;
}
std::size_t copyBegin = 0;
std::string origStr = CATCH_MOVE(str);
str.clear();
// There is at least one replacement, so reserve with the best guess
// we can make without actually counting the number of occurences.
str.reserve(origStr.size() - replaceThis.size() + withThis.size());
do {
str.append(origStr, copyBegin, i-copyBegin );
str += withThis;
copyBegin = i + replaceThis.size();
if( copyBegin < origStr.size() )
i = origStr.find( replaceThis, copyBegin );
else
i = std::string::npos;
} while( i != std::string::npos );
if ( copyBegin < origStr.size() ) {
str.append(origStr, copyBegin, origStr.size() );
}
return replaced;
return true;
}

std::vector<StringRef> splitStringRef( StringRef str, char delimiter ) {
Expand Down
20 changes: 12 additions & 8 deletions tests/SelfTest/Baselines/compact.sw.approved.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1733,13 +1733,13 @@ Tag.tests.cpp:<line number>: passed: testCase.tags, VectorContains( Tag( "tag wi
Class.tests.cpp:<line number>: passed: Template_Fixture<TestType>::m_a == 1 for: 1 == 1
Class.tests.cpp:<line number>: passed: Template_Fixture<TestType>::m_a == 1 for: 1 == 1
Class.tests.cpp:<line number>: passed: Template_Fixture<TestType>::m_a == 1 for: 1.0 == 1
Misc.tests.cpp:<line number>: passed: sizeof(TestType) > 0 for: 1 > 0
Misc.tests.cpp:<line number>: passed: sizeof(TestType) > 0 for: 4 > 0
Misc.tests.cpp:<line number>: passed: sizeof(TestType) > 0 for: 1 > 0
Misc.tests.cpp:<line number>: passed: sizeof(TestType) > 0 for: 4 > 0
Misc.tests.cpp:<line number>: passed: sizeof(TestType) > 0 for: 4 > 0
Misc.tests.cpp:<line number>: passed: sizeof(TestType) > 0 for: 1 > 0
Misc.tests.cpp:<line number>: passed: sizeof(TestType) > 0 for: 4 > 0
Misc.tests.cpp:<line number>: passed: std::is_default_constructible<TestType>::value for: true
Misc.tests.cpp:<line number>: passed: std::is_default_constructible<TestType>::value for: true
Misc.tests.cpp:<line number>: passed: std::is_trivially_copyable<TestType>::value for: true
Misc.tests.cpp:<line number>: passed: std::is_trivially_copyable<TestType>::value for: true
Misc.tests.cpp:<line number>: passed: std::is_arithmetic<TestType>::value for: true
Misc.tests.cpp:<line number>: passed: std::is_arithmetic<TestType>::value for: true
Misc.tests.cpp:<line number>: passed: std::is_arithmetic<TestType>::value for: true
Misc.tests.cpp:<line number>: passed: v.size() == 5 for: 5 == 5
Misc.tests.cpp:<line number>: passed: v.capacity() >= 5 for: 5 >= 5
Misc.tests.cpp:<line number>: passed: v.size() == 10 for: 10 == 10
Expand Down Expand Up @@ -2479,6 +2479,10 @@ StringManip.tests.cpp:<line number>: passed: Catch::replaceInPlace(letters, lett
StringManip.tests.cpp:<line number>: passed: letters == "replaced" for: "replaced" == "replaced"
StringManip.tests.cpp:<line number>: passed: !(Catch::replaceInPlace(letters, "x", "z")) for: !false
StringManip.tests.cpp:<line number>: passed: letters == letters for: "abcdefcg" == "abcdefcg"
StringManip.tests.cpp:<line number>: passed: Catch::replaceInPlace(letters, "c", "cc") for: true
StringManip.tests.cpp:<line number>: passed: letters == "abccdefccg" for: "abccdefccg" == "abccdefccg"
StringManip.tests.cpp:<line number>: passed: Catch::replaceInPlace(s, "--", "-") for: true
StringManip.tests.cpp:<line number>: passed: s == "--" for: "--" == "--"
StringManip.tests.cpp:<line number>: passed: Catch::replaceInPlace(s, "'", "|'") for: true
StringManip.tests.cpp:<line number>: passed: s == "didn|'t" for: "didn|'t" == "didn|'t"
Stream.tests.cpp:<line number>: passed: Catch::makeStream( "%somestream" )
Expand Down Expand Up @@ -2686,6 +2690,6 @@ InternalBenchmark.tests.cpp:<line number>: passed: q3 == 23. for: 23.0 == 23.0
Misc.tests.cpp:<line number>: passed:
Misc.tests.cpp:<line number>: passed:
test cases: 417 | 312 passed | 85 failed | 6 skipped | 14 failed as expected
assertions: 2256 | 2075 passed | 146 failed | 35 failed as expected
assertions: 2260 | 2079 passed | 146 failed | 35 failed as expected


20 changes: 12 additions & 8 deletions tests/SelfTest/Baselines/compact.sw.multi.approved.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1726,13 +1726,13 @@ Tag.tests.cpp:<line number>: passed: testCase.tags, VectorContains( Tag( "tag wi
Class.tests.cpp:<line number>: passed: Template_Fixture<TestType>::m_a == 1 for: 1 == 1
Class.tests.cpp:<line number>: passed: Template_Fixture<TestType>::m_a == 1 for: 1 == 1
Class.tests.cpp:<line number>: passed: Template_Fixture<TestType>::m_a == 1 for: 1.0 == 1
Misc.tests.cpp:<line number>: passed: sizeof(TestType) > 0 for: 1 > 0
Misc.tests.cpp:<line number>: passed: sizeof(TestType) > 0 for: 4 > 0
Misc.tests.cpp:<line number>: passed: sizeof(TestType) > 0 for: 1 > 0
Misc.tests.cpp:<line number>: passed: sizeof(TestType) > 0 for: 4 > 0
Misc.tests.cpp:<line number>: passed: sizeof(TestType) > 0 for: 4 > 0
Misc.tests.cpp:<line number>: passed: sizeof(TestType) > 0 for: 1 > 0
Misc.tests.cpp:<line number>: passed: sizeof(TestType) > 0 for: 4 > 0
Misc.tests.cpp:<line number>: passed: std::is_default_constructible<TestType>::value for: true
Misc.tests.cpp:<line number>: passed: std::is_default_constructible<TestType>::value for: true
Misc.tests.cpp:<line number>: passed: std::is_trivially_copyable<TestType>::value for: true
Misc.tests.cpp:<line number>: passed: std::is_trivially_copyable<TestType>::value for: true
Misc.tests.cpp:<line number>: passed: std::is_arithmetic<TestType>::value for: true
Misc.tests.cpp:<line number>: passed: std::is_arithmetic<TestType>::value for: true
Misc.tests.cpp:<line number>: passed: std::is_arithmetic<TestType>::value for: true
Misc.tests.cpp:<line number>: passed: v.size() == 5 for: 5 == 5
Misc.tests.cpp:<line number>: passed: v.capacity() >= 5 for: 5 >= 5
Misc.tests.cpp:<line number>: passed: v.size() == 10 for: 10 == 10
Expand Down Expand Up @@ -2468,6 +2468,10 @@ StringManip.tests.cpp:<line number>: passed: Catch::replaceInPlace(letters, lett
StringManip.tests.cpp:<line number>: passed: letters == "replaced" for: "replaced" == "replaced"
StringManip.tests.cpp:<line number>: passed: !(Catch::replaceInPlace(letters, "x", "z")) for: !false
StringManip.tests.cpp:<line number>: passed: letters == letters for: "abcdefcg" == "abcdefcg"
StringManip.tests.cpp:<line number>: passed: Catch::replaceInPlace(letters, "c", "cc") for: true
StringManip.tests.cpp:<line number>: passed: letters == "abccdefccg" for: "abccdefccg" == "abccdefccg"
StringManip.tests.cpp:<line number>: passed: Catch::replaceInPlace(s, "--", "-") for: true
StringManip.tests.cpp:<line number>: passed: s == "--" for: "--" == "--"
StringManip.tests.cpp:<line number>: passed: Catch::replaceInPlace(s, "'", "|'") for: true
StringManip.tests.cpp:<line number>: passed: s == "didn|'t" for: "didn|'t" == "didn|'t"
Stream.tests.cpp:<line number>: passed: Catch::makeStream( "%somestream" )
Expand Down Expand Up @@ -2675,6 +2679,6 @@ InternalBenchmark.tests.cpp:<line number>: passed: q3 == 23. for: 23.0 == 23.0
Misc.tests.cpp:<line number>: passed:
Misc.tests.cpp:<line number>: passed:
test cases: 417 | 312 passed | 85 failed | 6 skipped | 14 failed as expected
assertions: 2256 | 2075 passed | 146 failed | 35 failed as expected
assertions: 2260 | 2079 passed | 146 failed | 35 failed as expected


2 changes: 1 addition & 1 deletion tests/SelfTest/Baselines/console.std.approved.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1589,5 +1589,5 @@ due to unexpected exception with message:

===============================================================================
test cases: 417 | 326 passed | 70 failed | 7 skipped | 14 failed as expected
assertions: 2239 | 2075 passed | 129 failed | 35 failed as expected
assertions: 2243 | 2079 passed | 129 failed | 35 failed as expected

66 changes: 51 additions & 15 deletions tests/SelfTest/Baselines/console.sw.approved.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11655,9 +11655,9 @@ Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: PASSED:
REQUIRE( sizeof(TestType) > 0 )
REQUIRE( std::is_default_constructible<TestType>::value )
with expansion:
1 > 0
true

-------------------------------------------------------------------------------
Template test case with test types specified inside non-copyable and non-
Expand All @@ -11667,9 +11667,9 @@ Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: PASSED:
REQUIRE( sizeof(TestType) > 0 )
REQUIRE( std::is_default_constructible<TestType>::value )
with expansion:
4 > 0
true

-------------------------------------------------------------------------------
Template test case with test types specified inside non-default-constructible
Expand All @@ -11679,9 +11679,9 @@ Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: PASSED:
REQUIRE( sizeof(TestType) > 0 )
REQUIRE( std::is_trivially_copyable<TestType>::value )
with expansion:
1 > 0
true

-------------------------------------------------------------------------------
Template test case with test types specified inside non-default-constructible
Expand All @@ -11691,9 +11691,9 @@ Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: PASSED:
REQUIRE( sizeof(TestType) > 0 )
REQUIRE( std::is_trivially_copyable<TestType>::value )
with expansion:
4 > 0
true

-------------------------------------------------------------------------------
Template test case with test types specified inside std::tuple - MyTypes - 0
Expand All @@ -11702,9 +11702,9 @@ Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: PASSED:
REQUIRE( sizeof(TestType) > 0 )
REQUIRE( std::is_arithmetic<TestType>::value )
with expansion:
4 > 0
true

-------------------------------------------------------------------------------
Template test case with test types specified inside std::tuple - MyTypes - 1
Expand All @@ -11713,9 +11713,9 @@ Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: PASSED:
REQUIRE( sizeof(TestType) > 0 )
REQUIRE( std::is_arithmetic<TestType>::value )
with expansion:
1 > 0
true

-------------------------------------------------------------------------------
Template test case with test types specified inside std::tuple - MyTypes - 2
Expand All @@ -11724,9 +11724,9 @@ Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: PASSED:
REQUIRE( sizeof(TestType) > 0 )
REQUIRE( std::is_arithmetic<TestType>::value )
with expansion:
4 > 0
true

-------------------------------------------------------------------------------
TemplateTest: vectors can be sized and resized - float
Expand Down Expand Up @@ -17289,6 +17289,42 @@ StringManip.tests.cpp:<line number>: PASSED:
with expansion:
"abcdefcg" == "abcdefcg"

-------------------------------------------------------------------------------
replaceInPlace
no replace in already-replaced string
lengthening
-------------------------------------------------------------------------------
StringManip.tests.cpp:<line number>
...............................................................................

StringManip.tests.cpp:<line number>: PASSED:
CHECK( Catch::replaceInPlace(letters, "c", "cc") )
with expansion:
true

StringManip.tests.cpp:<line number>: PASSED:
CHECK( letters == "abccdefccg" )
with expansion:
"abccdefccg" == "abccdefccg"

-------------------------------------------------------------------------------
replaceInPlace
no replace in already-replaced string
shortening
-------------------------------------------------------------------------------
StringManip.tests.cpp:<line number>
...............................................................................

StringManip.tests.cpp:<line number>: PASSED:
CHECK( Catch::replaceInPlace(s, "--", "-") )
with expansion:
true

StringManip.tests.cpp:<line number>: PASSED:
CHECK( s == "--" )
with expansion:
"--" == "--"

-------------------------------------------------------------------------------
replaceInPlace
escape '
Expand Down Expand Up @@ -18732,5 +18768,5 @@ Misc.tests.cpp:<line number>: PASSED:

===============================================================================
test cases: 417 | 312 passed | 85 failed | 6 skipped | 14 failed as expected
assertions: 2256 | 2075 passed | 146 failed | 35 failed as expected
assertions: 2260 | 2079 passed | 146 failed | 35 failed as expected

66 changes: 51 additions & 15 deletions tests/SelfTest/Baselines/console.sw.multi.approved.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11648,9 +11648,9 @@ Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: PASSED:
REQUIRE( sizeof(TestType) > 0 )
REQUIRE( std::is_default_constructible<TestType>::value )
with expansion:
1 > 0
true

-------------------------------------------------------------------------------
Template test case with test types specified inside non-copyable and non-
Expand All @@ -11660,9 +11660,9 @@ Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: PASSED:
REQUIRE( sizeof(TestType) > 0 )
REQUIRE( std::is_default_constructible<TestType>::value )
with expansion:
4 > 0
true

-------------------------------------------------------------------------------
Template test case with test types specified inside non-default-constructible
Expand All @@ -11672,9 +11672,9 @@ Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: PASSED:
REQUIRE( sizeof(TestType) > 0 )
REQUIRE( std::is_trivially_copyable<TestType>::value )
with expansion:
1 > 0
true

-------------------------------------------------------------------------------
Template test case with test types specified inside non-default-constructible
Expand All @@ -11684,9 +11684,9 @@ Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: PASSED:
REQUIRE( sizeof(TestType) > 0 )
REQUIRE( std::is_trivially_copyable<TestType>::value )
with expansion:
4 > 0
true

-------------------------------------------------------------------------------
Template test case with test types specified inside std::tuple - MyTypes - 0
Expand All @@ -11695,9 +11695,9 @@ Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: PASSED:
REQUIRE( sizeof(TestType) > 0 )
REQUIRE( std::is_arithmetic<TestType>::value )
with expansion:
4 > 0
true

-------------------------------------------------------------------------------
Template test case with test types specified inside std::tuple - MyTypes - 1
Expand All @@ -11706,9 +11706,9 @@ Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: PASSED:
REQUIRE( sizeof(TestType) > 0 )
REQUIRE( std::is_arithmetic<TestType>::value )
with expansion:
1 > 0
true

-------------------------------------------------------------------------------
Template test case with test types specified inside std::tuple - MyTypes - 2
Expand All @@ -11717,9 +11717,9 @@ Misc.tests.cpp:<line number>
...............................................................................

Misc.tests.cpp:<line number>: PASSED:
REQUIRE( sizeof(TestType) > 0 )
REQUIRE( std::is_arithmetic<TestType>::value )
with expansion:
4 > 0
true

-------------------------------------------------------------------------------
TemplateTest: vectors can be sized and resized - float
Expand Down Expand Up @@ -17278,6 +17278,42 @@ StringManip.tests.cpp:<line number>: PASSED:
with expansion:
"abcdefcg" == "abcdefcg"

-------------------------------------------------------------------------------
replaceInPlace
no replace in already-replaced string
lengthening
-------------------------------------------------------------------------------
StringManip.tests.cpp:<line number>
...............................................................................

StringManip.tests.cpp:<line number>: PASSED:
CHECK( Catch::replaceInPlace(letters, "c", "cc") )
with expansion:
true

StringManip.tests.cpp:<line number>: PASSED:
CHECK( letters == "abccdefccg" )
with expansion:
"abccdefccg" == "abccdefccg"

-------------------------------------------------------------------------------
replaceInPlace
no replace in already-replaced string
shortening
-------------------------------------------------------------------------------
StringManip.tests.cpp:<line number>
...............................................................................

StringManip.tests.cpp:<line number>: PASSED:
CHECK( Catch::replaceInPlace(s, "--", "-") )
with expansion:
true

StringManip.tests.cpp:<line number>: PASSED:
CHECK( s == "--" )
with expansion:
"--" == "--"

-------------------------------------------------------------------------------
replaceInPlace
escape '
Expand Down Expand Up @@ -18721,5 +18757,5 @@ Misc.tests.cpp:<line number>: PASSED:

===============================================================================
test cases: 417 | 312 passed | 85 failed | 6 skipped | 14 failed as expected
assertions: 2256 | 2075 passed | 146 failed | 35 failed as expected
assertions: 2260 | 2079 passed | 146 failed | 35 failed as expected

Loading

0 comments on commit 4d8affc

Please sign in to comment.