Cleanup and optimize stringifying of string-like types

More specifically, made the actual implementation of string-like
type handling take argument as `Catch::StringRef`, instead of
taking `std::string const&`.

This means that string-like types that are not `std::string` no
longer need to pay for an extra construction of `std::string`
(including the potential allocation), before they can be stringified.

The actual string stringification routine is now also better about
reserving sufficient space.

src/catch2/catch_tostring.cpp

@@ -54,6 +54,48 @@ namespace Detail {
         }
     } // end unnamed namespace
 
+    std::string convertIntoString(StringRef string, bool escape_invisibles) {
+        std::string ret;
+        // This is enough for the "don't escape invisibles" case, and a good
+        // lower bound on the "escape invisibles" case.
+        ret.reserve(string.size() + 2);
+
+        if (!escape_invisibles) {
+            ret += '"';
+            ret += string;
+            ret += '"';
+            return ret;
+        }
+
+        ret += '"';
+        for (char c : string) {
+            switch (c) {
+            case '\r':
+                ret.append("\\r");
+                break;
+            case '\n':
+                ret.append("\\n");
+                break;
+            case '\t':
+                ret.append("\\t");
+                break;
+            case '\f':
+                ret.append("\\f");
+                break;
+            default:
+                ret.push_back(c);
+                break;
+            }
+        }
+        ret += '"';
+
+        return ret;
+    }
+
+    std::string convertIntoString(StringRef string) {
+        return convertIntoString(string, getCurrentContext().getConfig()->showInvisibles());
+    }
+
     std::string rawMemoryToString( const void *object, std::size_t size ) {
         // Reverse order for little endian architectures
         int i = 0, end = static_cast<int>( size ), inc = 1;
@@ -80,44 +122,25 @@ namespace Detail {
 //// ======================================================= ////
 
 std::string StringMaker<std::string>::convert(const std::string& str) {
-    if (!getCurrentContext().getConfig()->showInvisibles()) {
+    return Detail::convertIntoString( str );
-        return '"' + str + '"';
-    }
-
-    std::string s("\"");
-    for (char c : str) {
-        switch (c) {
-        case '\n':
-            s.append("\\n");
-            break;
-        case '\t':
-            s.append("\\t");
-            break;
-        default:
-            s.push_back(c);
-            break;
-        }
-    }
-    s.append("\"");
-    return s;
 }
 
 #ifdef CATCH_CONFIG_CPP17_STRING_VIEW
 std::string StringMaker<std::string_view>::convert(std::string_view str) {
-    return ::Catch::Detail::stringify(std::string{ str });
+    return Detail::convertIntoString( StringRef( str.data(), str.size() ) );
 }
 #endif
 
 std::string StringMaker<char const*>::convert(char const* str) {
     if (str) {
-        return ::Catch::Detail::stringify(std::string{ str });
+        return Detail::convertIntoString( str );
     } else {
         return{ "{null string}" };
     }
 }
 std::string StringMaker<char*>::convert(char* str) {
     if (str) {
-        return ::Catch::Detail::stringify(std::string{ str });
+        return Detail::convertIntoString( str );
     } else {
         return{ "{null string}" };
     }

src/catch2/catch_tostring.hpp

@@ -13,6 +13,7 @@
 #include <cstddef>
 #include <type_traits>
 #include <string>
+#include <string.h>
 #include <catch2/internal/catch_compiler_capabilities.hpp>
 #include <catch2/internal/catch_config_wchar.hpp>
 #include <catch2/internal/catch_stream.hpp>
@@ -32,6 +33,13 @@ namespace Catch {
 
         constexpr StringRef unprintableString = "{?}"_sr;
 
+        //! Encases `string in quotes, and optionally escapes invisibles
+        std::string convertIntoString( StringRef string, bool escapeInvisibles );
+
+        //! Encases `string` in quotes, and escapes invisibles if user requested
+        //! it via CLI
+        std::string convertIntoString( StringRef string );
+
         std::string rawMemoryToString( const void *object, std::size_t size );
 
         template<typename T>
@@ -186,24 +194,31 @@ namespace Catch {
     };
 #endif // CATCH_CONFIG_WCHAR
 
-    // TBD: Should we use `strnlen` to ensure that we don't go out of the buffer,
-    //      while keeping string semantics?
     template<int SZ>
     struct StringMaker<char[SZ]> {
         static std::string convert(char const* str) {
-            return ::Catch::Detail::stringify(std::string{ str });
+            // Note that `strnlen` is not actually part of standard C++,
+            // but both POSIX and Windows cstdlib provide it.
+            return Detail::convertIntoString(
+                StringRef( str, strnlen( str, SZ ) ) );
         }
     };
     template<int SZ>
     struct StringMaker<signed char[SZ]> {
         static std::string convert(signed char const* str) {
-            return ::Catch::Detail::stringify(std::string{ reinterpret_cast<char const *>(str) });
+            // See the plain `char const*` overload
+            auto reinterpreted = reinterpret_cast<char const*>(str);
+            return Detail::convertIntoString(
+                StringRef(reinterpreted, strnlen(reinterpreted, SZ)));
         }
     };
     template<int SZ>
     struct StringMaker<unsigned char[SZ]> {
         static std::string convert(unsigned char const* str) {
-            return ::Catch::Detail::stringify(std::string{ reinterpret_cast<char const *>(str) });
+            // See the plain `char const*` overload
+            auto reinterpreted = reinterpret_cast<char const*>(str);
+            return Detail::convertIntoString(
+                StringRef(reinterpreted, strnlen(reinterpreted, SZ)));
         }
     };
 

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

@@ -205,6 +205,9 @@ Message from section two
 :test-result: PASS String matchers
 :test-result: PASS StringRef
 :test-result: PASS StringRef at compilation time
+:test-result: PASS Stringifying char arrays with statically known sizes - char
+:test-result: PASS Stringifying char arrays with statically known sizes - signed char
+:test-result: PASS Stringifying char arrays with statically known sizes - unsigned char
 :test-result: PASS Stringifying std::chrono::duration helpers
 :test-result: PASS Stringifying std::chrono::duration with weird ratios
 :test-result: PASS Stringifying std::chrono::time_point<system_clock>

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

@@ -1532,6 +1532,12 @@ String.tests.cpp:<line number>: passed: with 1 message: '!(sr1.empty())'
 String.tests.cpp:<line number>: passed: with 1 message: 'sr1.size() == 3'
 String.tests.cpp:<line number>: passed: with 1 message: 'sr2.empty()'
 String.tests.cpp:<line number>: passed: with 1 message: 'sr2.size() == 0'
+ToString.tests.cpp:<line number>: passed: ::Catch::Detail::stringify( with_null_terminator ) == R"("abc")"s for: ""abc"" == ""abc""
+ToString.tests.cpp:<line number>: passed: ::Catch::Detail::stringify( no_null_terminator ) == R"("abc")"s for: ""abc"" == ""abc""
+ToString.tests.cpp:<line number>: passed: ::Catch::Detail::stringify( with_null_terminator ) == R"("abc")"s for: ""abc"" == ""abc""
+ToString.tests.cpp:<line number>: passed: ::Catch::Detail::stringify( no_null_terminator ) == R"("abc")"s for: ""abc"" == ""abc""
+ToString.tests.cpp:<line number>: passed: ::Catch::Detail::stringify( with_null_terminator ) == R"("abc")"s for: ""abc"" == ""abc""
+ToString.tests.cpp:<line number>: passed: ::Catch::Detail::stringify( no_null_terminator ) == R"("abc")"s for: ""abc"" == ""abc""
 ToStringChrono.tests.cpp:<line number>: passed: minute == seconds for: 1 m == 60 s
 ToStringChrono.tests.cpp:<line number>: passed: hour != seconds for: 1 h != 60 s
 ToStringChrono.tests.cpp:<line number>: passed: micro != milli for: 1 us != 1 ms

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

@@ -1386,6 +1386,6 @@ due to unexpected exception with message:
   Why would you throw a std::string?
 
 ===============================================================================
-test cases:  360 |  284 passed |  70 failed |  6 failed as expected
+test cases:  363 |  287 passed |  70 failed |  6 failed as expected
-assertions: 2084 | 1932 passed | 129 failed | 23 failed as expected
+assertions: 2090 | 1938 passed | 129 failed | 23 failed as expected
 

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

@@ -10865,6 +10865,54 @@ String.tests.cpp:<line number>: PASSED:
 with message:
   sr2.size() == 0
 
+-------------------------------------------------------------------------------
+Stringifying char arrays with statically known sizes - char
+-------------------------------------------------------------------------------
+ToString.tests.cpp:<line number>
+...............................................................................
+
+ToString.tests.cpp:<line number>: PASSED:
+  CHECK( ::Catch::Detail::stringify( with_null_terminator ) == R"("abc")"s )
+with expansion:
+  ""abc"" == ""abc""
+
+ToString.tests.cpp:<line number>: PASSED:
+  CHECK( ::Catch::Detail::stringify( no_null_terminator ) == R"("abc")"s )
+with expansion:
+  ""abc"" == ""abc""
+
+-------------------------------------------------------------------------------
+Stringifying char arrays with statically known sizes - signed char
+-------------------------------------------------------------------------------
+ToString.tests.cpp:<line number>
+...............................................................................
+
+ToString.tests.cpp:<line number>: PASSED:
+  CHECK( ::Catch::Detail::stringify( with_null_terminator ) == R"("abc")"s )
+with expansion:
+  ""abc"" == ""abc""
+
+ToString.tests.cpp:<line number>: PASSED:
+  CHECK( ::Catch::Detail::stringify( no_null_terminator ) == R"("abc")"s )
+with expansion:
+  ""abc"" == ""abc""
+
+-------------------------------------------------------------------------------
+Stringifying char arrays with statically known sizes - unsigned char
+-------------------------------------------------------------------------------
+ToString.tests.cpp:<line number>
+...............................................................................
+
+ToString.tests.cpp:<line number>: PASSED:
+  CHECK( ::Catch::Detail::stringify( with_null_terminator ) == R"("abc")"s )
+with expansion:
+  ""abc"" == ""abc""
+
+ToString.tests.cpp:<line number>: PASSED:
+  CHECK( ::Catch::Detail::stringify( no_null_terminator ) == R"("abc")"s )
+with expansion:
+  ""abc"" == ""abc""
+
 -------------------------------------------------------------------------------
 Stringifying std::chrono::duration helpers
 -------------------------------------------------------------------------------
@@ -16782,6 +16830,6 @@ Misc.tests.cpp:<line number>
 Misc.tests.cpp:<line number>: PASSED:
 
 ===============================================================================
-test cases:  360 |  268 passed |  86 failed |  6 failed as expected
+test cases:  363 |  271 passed |  86 failed |  6 failed as expected
-assertions: 2101 | 1932 passed | 146 failed | 23 failed as expected
+assertions: 2107 | 1938 passed | 146 failed | 23 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="130" tests="2102" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
+  <testsuite name="<exe-name>" errors="17" failures="130" tests="2108" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
     <properties>
       <property name="filters" value="~[!nonportable]~[!benchmark]~[approvals] *"/>
       <property name="random-seed" value="1"/>
@@ -1194,6 +1194,9 @@ Matchers.tests.cpp:<line number>
     <testcase classname="<exe-name>.global" name="StringRef/StringRef + StringRef" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="StringRef at compilation time/Simple constructors" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="StringRef at compilation time/UDL construction" time="{duration}" status="run"/>
+    <testcase classname="<exe-name>.global" name="Stringifying char arrays with statically known sizes - char" time="{duration}" status="run"/>
+    <testcase classname="<exe-name>.global" name="Stringifying char arrays with statically known sizes - signed char" time="{duration}" status="run"/>
+    <testcase classname="<exe-name>.global" name="Stringifying char arrays with statically known sizes - unsigned char" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Stringifying std::chrono::duration helpers" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Stringifying std::chrono::duration with weird ratios" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Stringifying std::chrono::time_point&lt;system_clock>" time="{duration}" status="run"/>

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

@@ -222,6 +222,9 @@
   <file path="tests/<exe-name>/IntrospectiveTests/ToString.tests.cpp">
     <testCase name="Directly creating an EnumInfo" duration="{duration}"/>
     <testCase name="Range type with sentinel" duration="{duration}"/>
+    <testCase name="Stringifying char arrays with statically known sizes - char" duration="{duration}"/>
+    <testCase name="Stringifying char arrays with statically known sizes - signed char" duration="{duration}"/>
+    <testCase name="Stringifying char arrays with statically known sizes - unsigned char" duration="{duration}"/>
     <testCase name="parseEnums/No enums" duration="{duration}"/>
     <testCase name="parseEnums/One enum value" duration="{duration}"/>
     <testCase name="parseEnums/Multiple enum values" duration="{duration}"/>

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

@@ -2743,6 +2743,18 @@ ok {test-number} - with 1 message: 'sr1.size() == 3'
 ok {test-number} - with 1 message: 'sr2.empty()'
 # StringRef at compilation time
 ok {test-number} - with 1 message: 'sr2.size() == 0'
+# Stringifying char arrays with statically known sizes - char
+ok {test-number} - ::Catch::Detail::stringify( with_null_terminator ) == R"("abc")"s for: ""abc"" == ""abc""
+# Stringifying char arrays with statically known sizes - char
+ok {test-number} - ::Catch::Detail::stringify( no_null_terminator ) == R"("abc")"s for: ""abc"" == ""abc""
+# Stringifying char arrays with statically known sizes - signed char
+ok {test-number} - ::Catch::Detail::stringify( with_null_terminator ) == R"("abc")"s for: ""abc"" == ""abc""
+# Stringifying char arrays with statically known sizes - signed char
+ok {test-number} - ::Catch::Detail::stringify( no_null_terminator ) == R"("abc")"s for: ""abc"" == ""abc""
+# Stringifying char arrays with statically known sizes - unsigned char
+ok {test-number} - ::Catch::Detail::stringify( with_null_terminator ) == R"("abc")"s for: ""abc"" == ""abc""
+# Stringifying char arrays with statically known sizes - unsigned char
+ok {test-number} - ::Catch::Detail::stringify( no_null_terminator ) == R"("abc")"s for: ""abc"" == ""abc""
 # Stringifying std::chrono::duration helpers
 ok {test-number} - minute == seconds for: 1 m == 60 s
 # Stringifying std::chrono::duration helpers
@@ -4204,5 +4216,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0
 ok {test-number} -
 # xmlentitycheck
 ok {test-number} -
-1..2101
+1..2107
 

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

@@ -510,6 +510,12 @@ Matchers.tests.cpp:<line number>|nexpression failed|n  CHECK_THAT( testStringFor
 ##teamcity[testFinished name='StringRef' duration="{duration}"]
 ##teamcity[testStarted name='StringRef at compilation time']
 ##teamcity[testFinished name='StringRef at compilation time' duration="{duration}"]
+##teamcity[testStarted name='Stringifying char arrays with statically known sizes - char']
+##teamcity[testFinished name='Stringifying char arrays with statically known sizes - char' duration="{duration}"]
+##teamcity[testStarted name='Stringifying char arrays with statically known sizes - signed char']
+##teamcity[testFinished name='Stringifying char arrays with statically known sizes - signed char' duration="{duration}"]
+##teamcity[testStarted name='Stringifying char arrays with statically known sizes - unsigned char']
+##teamcity[testFinished name='Stringifying char arrays with statically known sizes - unsigned char' duration="{duration}"]
 ##teamcity[testStarted name='Stringifying std::chrono::duration helpers']
 ##teamcity[testFinished name='Stringifying std::chrono::duration helpers' duration="{duration}"]
 ##teamcity[testStarted name='Stringifying std::chrono::duration with weird ratios']

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

@@ -12900,6 +12900,63 @@ Message from section two
       </Section>
       <OverallResult success="true"/>
     </TestCase>
+    <TestCase name="Stringifying char arrays with statically known sizes - char" tags="[toString]" filename="tests/<exe-name>/IntrospectiveTests/ToString.tests.cpp" >
+      <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/ToString.tests.cpp" >
+        <Original>
+          ::Catch::Detail::stringify( with_null_terminator ) == R"("abc")"s
+        </Original>
+        <Expanded>
+          ""abc"" == ""abc""
+        </Expanded>
+      </Expression>
+      <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/ToString.tests.cpp" >
+        <Original>
+          ::Catch::Detail::stringify( no_null_terminator ) == R"("abc")"s
+        </Original>
+        <Expanded>
+          ""abc"" == ""abc""
+        </Expanded>
+      </Expression>
+      <OverallResult success="true"/>
+    </TestCase>
+    <TestCase name="Stringifying char arrays with statically known sizes - signed char" tags="[toString]" filename="tests/<exe-name>/IntrospectiveTests/ToString.tests.cpp" >
+      <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/ToString.tests.cpp" >
+        <Original>
+          ::Catch::Detail::stringify( with_null_terminator ) == R"("abc")"s
+        </Original>
+        <Expanded>
+          ""abc"" == ""abc""
+        </Expanded>
+      </Expression>
+      <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/ToString.tests.cpp" >
+        <Original>
+          ::Catch::Detail::stringify( no_null_terminator ) == R"("abc")"s
+        </Original>
+        <Expanded>
+          ""abc"" == ""abc""
+        </Expanded>
+      </Expression>
+      <OverallResult success="true"/>
+    </TestCase>
+    <TestCase name="Stringifying char arrays with statically known sizes - unsigned char" tags="[toString]" filename="tests/<exe-name>/IntrospectiveTests/ToString.tests.cpp" >
+      <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/ToString.tests.cpp" >
+        <Original>
+          ::Catch::Detail::stringify( with_null_terminator ) == R"("abc")"s
+        </Original>
+        <Expanded>
+          ""abc"" == ""abc""
+        </Expanded>
+      </Expression>
+      <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/ToString.tests.cpp" >
+        <Original>
+          ::Catch::Detail::stringify( no_null_terminator ) == R"("abc")"s
+        </Original>
+        <Expanded>
+          ""abc"" == ""abc""
+        </Expanded>
+      </Expression>
+      <OverallResult success="true"/>
+    </TestCase>
     <TestCase name="Stringifying std::chrono::duration helpers" tags="[chrono][toString]" filename="tests/<exe-name>/UsageTests/ToStringChrono.tests.cpp" >
       <Expression success="true" type="REQUIRE" filename="tests/<exe-name>/UsageTests/ToStringChrono.tests.cpp" >
         <Original>
@@ -19745,9 +19802,9 @@ loose text artifact
       </Section>
       <OverallResult success="true"/>
     </TestCase>
-    <OverallResults successes="1932" failures="147" expectedFailures="23"/>
+    <OverallResults successes="1938" failures="147" expectedFailures="23"/>
-    <OverallResultsCases successes="268" failures="86" expectedFailures="6"/>
+    <OverallResultsCases successes="271" failures="86" expectedFailures="6"/>
   </Group>
-  <OverallResults successes="1932" failures="146" expectedFailures="23"/>
+  <OverallResults successes="1938" failures="146" expectedFailures="23"/>
-  <OverallResultsCases successes="268" failures="86" expectedFailures="6"/>
+  <OverallResultsCases successes="271" failures="86" expectedFailures="6"/>
 </Catch>

tests/SelfTest/IntrospectiveTests/ToString.tests.cpp

@@ -1,6 +1,7 @@
 #include <catch2/internal/catch_enum_values_registry.hpp>
 #include <catch2/matchers/catch_matchers_vector.hpp>
 #include <catch2/catch_test_macros.hpp>
+#include <catch2/catch_template_test_macros.hpp>
 
 enum class EnumClass3 { Value1, Value2, Value3, Value4 };
 
@@ -51,3 +52,38 @@ TEST_CASE( "Directly creating an EnumInfo" ) {
 TEST_CASE("Range type with sentinel") {
     CHECK( Catch::Detail::stringify(UsesSentinel{}) == "{  }" );
 }
+
+TEST_CASE("convertIntoString stringification helper", "[toString][approvals]") {
+    using namespace std::string_literals;
+    using Catch::Detail::convertIntoString;
+    using namespace Catch;
+
+    SECTION("No escaping") {
+        CHECK(convertIntoString(""_sr, false) == R"("")"s);
+        CHECK(convertIntoString("abcd"_sr, false) == R"("abcd")"s);
+        CHECK(convertIntoString("ab\ncd"_sr, false) == "\"ab\ncd\""s);
+        CHECK(convertIntoString("ab\r\ncd"_sr, false) == "\"ab\r\ncd\""s);
+        CHECK(convertIntoString("ab\"cd"_sr, false) == R"("ab"cd")"s);
+    }
+    SECTION("Escaping invisibles") {
+        CHECK(convertIntoString(""_sr, true) == R"("")"s);
+        CHECK(convertIntoString("ab\ncd"_sr, true) == R"("ab\ncd")"s);
+        CHECK(convertIntoString("ab\r\ncd"_sr, true) == R"("ab\r\ncd")"s);
+        CHECK(convertIntoString("ab\tcd"_sr, true) == R"("ab\tcd")"s);
+        CHECK(convertIntoString("ab\fcd"_sr, true) == R"("ab\fcd")"s);
+        CHECK(convertIntoString("ab\"cd"_sr, true) == R"("ab"cd")"s);
+    }
+}
+
+TEMPLATE_TEST_CASE( "Stringifying char arrays with statically known sizes",
+                    "[toString]",
+                    char,
+                    signed char,
+                    unsigned char ) {
+    using namespace std::string_literals;
+    TestType with_null_terminator[10] = "abc";
+    CHECK( ::Catch::Detail::stringify( with_null_terminator ) == R"("abc")"s );
+
+    TestType no_null_terminator[3] = { 'a', 'b', 'c' };
+    CHECK( ::Catch::Detail::stringify( no_null_terminator ) == R"("abc")"s );
+}