less copies and allocations in replaceInPlace

src/catch2/internal/catch_string_manip.cpp

@@ -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>
 
@@ -65,17 +66,26 @@ 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();
+        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 ) {

tests/SelfTest/Baselines/compact.sw.approved.txt

@@ -2483,6 +2483,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" )
@@ -2690,6 +2694,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: 2260 | 2079 passed | 146 failed | 35 failed as expected
+assertions: 2264 | 2083 passed | 146 failed | 35 failed as expected
 
 

tests/SelfTest/Baselines/compact.sw.multi.approved.txt

@@ -2472,6 +2472,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" )
@@ -2679,6 +2683,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: 2260 | 2079 passed | 146 failed | 35 failed as expected
+assertions: 2264 | 2083 passed | 146 failed | 35 failed as expected
 
 

tests/SelfTest/Baselines/console.std.approved.txt

@@ -1589,5 +1589,5 @@ due to unexpected exception with message:
 
 ===============================================================================
 test cases:  417 |  326 passed |  70 failed | 7 skipped | 14 failed as expected
-assertions: 2243 | 2079 passed | 129 failed | 35 failed as expected
+assertions: 2247 | 2083 passed | 129 failed | 35 failed as expected
 

tests/SelfTest/Baselines/console.sw.approved.txt

@@ -17309,6 +17309,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 '
@@ -18752,5 +18788,5 @@ Misc.tests.cpp:<line number>: PASSED:
 
 ===============================================================================
 test cases:  417 |  312 passed |  85 failed | 6 skipped | 14 failed as expected
-assertions: 2260 | 2079 passed | 146 failed | 35 failed as expected
+assertions: 2264 | 2083 passed | 146 failed | 35 failed as expected
 

tests/SelfTest/Baselines/console.sw.multi.approved.txt

@@ -17298,6 +17298,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 '
@@ -18741,5 +18777,5 @@ Misc.tests.cpp:<line number>: PASSED:
 
 ===============================================================================
 test cases:  417 |  312 passed |  85 failed | 6 skipped | 14 failed as expected
-assertions: 2260 | 2079 passed | 146 failed | 35 failed as expected
+assertions: 2264 | 2083 passed | 146 failed | 35 failed as expected
 

tests/SelfTest/Baselines/junit.sw.approved.txt

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <testsuitesloose text artifact
 >
-  <testsuite name="<exe-name>" errors="17" failures="129" skipped="12" tests="2272" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
+  <testsuite name="<exe-name>" errors="17" failures="129" skipped="12" tests="2276" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
     <properties>
       <property name="random-seed" value="1"/>
       <property name="filters" value="&quot;*&quot; ~[!nonportable] ~[!benchmark] ~[approvals]"/>
@@ -1966,6 +1966,8 @@ at Message.tests.cpp:<line number>
     <testcase classname="<exe-name>.global" name="replaceInPlace/replace last char" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="replaceInPlace/replace all chars" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="replaceInPlace/replace no chars" time="{duration}" status="run"/>
+    <testcase classname="<exe-name>.global" name="replaceInPlace/no replace in already-replaced string/lengthening" time="{duration}" status="run"/>
+    <testcase classname="<exe-name>.global" name="replaceInPlace/no replace in already-replaced string/shortening" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="replaceInPlace/escape '" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="request an unknown %-starting stream fails" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="resolution" time="{duration}" status="run"/>

tests/SelfTest/Baselines/junit.sw.multi.approved.txt

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <testsuites>
-  <testsuite name="<exe-name>" errors="17" failures="129" skipped="12" tests="2272" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
+  <testsuite name="<exe-name>" errors="17" failures="129" skipped="12" tests="2276" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
     <properties>
       <property name="random-seed" value="1"/>
       <property name="filters" value="&quot;*&quot; ~[!nonportable] ~[!benchmark] ~[approvals]"/>
@@ -1965,6 +1965,8 @@ at Message.tests.cpp:<line number>
     <testcase classname="<exe-name>.global" name="replaceInPlace/replace last char" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="replaceInPlace/replace all chars" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="replaceInPlace/replace no chars" time="{duration}" status="run"/>
+    <testcase classname="<exe-name>.global" name="replaceInPlace/no replace in already-replaced string/lengthening" time="{duration}" status="run"/>
+    <testcase classname="<exe-name>.global" name="replaceInPlace/no replace in already-replaced string/shortening" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="replaceInPlace/escape '" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="request an unknown %-starting stream fails" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="resolution" time="{duration}" status="run"/>

tests/SelfTest/Baselines/sonarqube.sw.approved.txt

@@ -262,6 +262,8 @@ at AssertionHandler.tests.cpp:<line number>
     <testCase name="replaceInPlace/replace last char" duration="{duration}"/>
     <testCase name="replaceInPlace/replace all chars" duration="{duration}"/>
     <testCase name="replaceInPlace/replace no chars" duration="{duration}"/>
+    <testCase name="replaceInPlace/no replace in already-replaced string/lengthening" duration="{duration}"/>
+    <testCase name="replaceInPlace/no replace in already-replaced string/shortening" duration="{duration}"/>
     <testCase name="replaceInPlace/escape '" duration="{duration}"/>
     <testCase name="splitString" duration="{duration}"/>
     <testCase name="startsWith" duration="{duration}"/>

tests/SelfTest/Baselines/sonarqube.sw.multi.approved.txt

@@ -261,6 +261,8 @@ at AssertionHandler.tests.cpp:<line number>
     <testCase name="replaceInPlace/replace last char" duration="{duration}"/>
     <testCase name="replaceInPlace/replace all chars" duration="{duration}"/>
     <testCase name="replaceInPlace/replace no chars" duration="{duration}"/>
+    <testCase name="replaceInPlace/no replace in already-replaced string/lengthening" duration="{duration}"/>
+    <testCase name="replaceInPlace/no replace in already-replaced string/shortening" duration="{duration}"/>
     <testCase name="replaceInPlace/escape '" duration="{duration}"/>
     <testCase name="splitString" duration="{duration}"/>
     <testCase name="startsWith" duration="{duration}"/>

tests/SelfTest/Baselines/tap.sw.approved.txt

@@ -4206,6 +4206,14 @@ ok {test-number} - !(Catch::replaceInPlace(letters, "x", "z")) for: !false
 # replaceInPlace
 ok {test-number} - letters == letters for: "abcdefcg" == "abcdefcg"
 # replaceInPlace
+ok {test-number} - Catch::replaceInPlace(letters, "c", "cc") for: true
+# replaceInPlace
+ok {test-number} - letters == "abccdefccg" for: "abccdefccg" == "abccdefccg"
+# replaceInPlace
+ok {test-number} - Catch::replaceInPlace(s, "--", "-") for: true
+# replaceInPlace
+ok {test-number} - s == "--" for: "--" == "--"
+# replaceInPlace
 ok {test-number} - Catch::replaceInPlace(s, "'", "|'") for: true
 # replaceInPlace
 ok {test-number} - s == "didn|'t" for: "didn|'t" == "didn|'t"
@@ -4549,5 +4557,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0
 ok {test-number} -
 # xmlentitycheck
 ok {test-number} -
-1..2272
+1..2276
 

tests/SelfTest/Baselines/tap.sw.multi.approved.txt

@@ -4195,6 +4195,14 @@ ok {test-number} - !(Catch::replaceInPlace(letters, "x", "z")) for: !false
 # replaceInPlace
 ok {test-number} - letters == letters for: "abcdefcg" == "abcdefcg"
 # replaceInPlace
+ok {test-number} - Catch::replaceInPlace(letters, "c", "cc") for: true
+# replaceInPlace
+ok {test-number} - letters == "abccdefccg" for: "abccdefccg" == "abccdefccg"
+# replaceInPlace
+ok {test-number} - Catch::replaceInPlace(s, "--", "-") for: true
+# replaceInPlace
+ok {test-number} - s == "--" for: "--" == "--"
+# replaceInPlace
 ok {test-number} - Catch::replaceInPlace(s, "'", "|'") for: true
 # replaceInPlace
 ok {test-number} - s == "didn|'t" for: "didn|'t" == "didn|'t"
@@ -4538,5 +4546,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0
 ok {test-number} -
 # xmlentitycheck
 ok {test-number} -
-1..2272
+1..2276
 

tests/SelfTest/Baselines/xml.sw.approved.txt

@@ -20066,6 +20066,50 @@ b1!
       </Expression>
       <OverallResults successes="2" failures="0" expectedFailures="0" skipped="false"/>
     </Section>
+    <Section name="no replace in already-replaced string" filename="tests/<exe-name>/IntrospectiveTests/StringManip.tests.cpp" >
+      <Section name="lengthening" filename="tests/<exe-name>/IntrospectiveTests/StringManip.tests.cpp" >
+        <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/StringManip.tests.cpp" >
+          <Original>
+            Catch::replaceInPlace(letters, "c", "cc")
+          </Original>
+          <Expanded>
+            true
+          </Expanded>
+        </Expression>
+        <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/StringManip.tests.cpp" >
+          <Original>
+            letters == "abccdefccg"
+          </Original>
+          <Expanded>
+            "abccdefccg" == "abccdefccg"
+          </Expanded>
+        </Expression>
+        <OverallResults successes="2" failures="0" expectedFailures="0" skipped="false"/>
+      </Section>
+      <OverallResults successes="2" failures="0" expectedFailures="0" skipped="false"/>
+    </Section>
+    <Section name="no replace in already-replaced string" filename="tests/<exe-name>/IntrospectiveTests/StringManip.tests.cpp" >
+      <Section name="shortening" filename="tests/<exe-name>/IntrospectiveTests/StringManip.tests.cpp" >
+        <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/StringManip.tests.cpp" >
+          <Original>
+            Catch::replaceInPlace(s, "--", "-")
+          </Original>
+          <Expanded>
+            true
+          </Expanded>
+        </Expression>
+        <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/StringManip.tests.cpp" >
+          <Original>
+            s == "--"
+          </Original>
+          <Expanded>
+            "--" == "--"
+          </Expanded>
+        </Expression>
+        <OverallResults successes="2" failures="0" expectedFailures="0" skipped="false"/>
+      </Section>
+      <OverallResults successes="2" failures="0" expectedFailures="0" skipped="false"/>
+    </Section>
     <Section name="escape '" filename="tests/<exe-name>/IntrospectiveTests/StringManip.tests.cpp" >
       <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/StringManip.tests.cpp" >
         <Original>
@@ -21707,6 +21751,6 @@ b1!
     </Section>
     <OverallResult success="true" skips="0"/>
   </TestCase>
-  <OverallResults successes="2079" failures="146" expectedFailures="35" skips="12"/>
+  <OverallResults successes="2083" failures="146" expectedFailures="35" skips="12"/>
   <OverallResultsCases successes="312" failures="85" expectedFailures="14" skips="6"/>
 </Catch2TestRun>