From a1b825b5b44d7ceb4688b07ff9192ab3b899e8b6 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 25 Sep 2025 13:57:53 -0700 Subject: [PATCH] performance improvements to CString (#9259) - make sure CString("foo") calls the literal constructor - avoid memory allocations when comparing to a literal --- libs/utils/include/utils/CString.h | 72 ++++++++++++++++++++++++++---- libs/utils/src/CString.cpp | 4 -- libs/utils/test/test_CString.cpp | 48 +++++++++++++++++++- 3 files changed, 111 insertions(+), 13 deletions(-) diff --git a/libs/utils/include/utils/CString.h b/libs/utils/include/utils/CString.h index 5abca6dbeb..f3eabbb265 100644 --- a/libs/utils/include/utils/CString.h +++ b/libs/utils/include/utils/CString.h @@ -85,7 +85,10 @@ public: // Allocates memory and copies traditional C string content. Unlike the above constructor, this // does not allow embedded nulls. This is explicit because this operation is costly. - explicit CString(const char* cstr); + // This is a template to ensure it's not preferred over the string literal constructor below. + template>> + explicit CString(T cstr) : CString(cstr, cstr ? strlen(cstr) : 0) { + } template CString(StringLiteral const& other) noexcept // NOLINT(google-explicit-constructor) @@ -171,7 +174,7 @@ public: // insert - CString& insert(size_type pos, char c) & noexcept { + CString& insert(size_type const pos, char const c) & noexcept { const char s[1] = { c }; return replace(pos, 0, s, 1); } @@ -193,7 +196,7 @@ public: return *this; } - CString&& insert(size_type pos, char c) && noexcept { + CString&& insert(size_type const pos, char const c) && noexcept { this->insert(pos, c); return std::move(*this); } @@ -217,7 +220,7 @@ public: // append - CString& append(char c) & noexcept { + CString& append(char const c) & noexcept { return insert(length(), c); } @@ -235,7 +238,7 @@ public: return insert(length(), str); } - CString&& append(char c) && noexcept { + CString&& append(char const c) && noexcept { this->append(c); return std::move(*this); } @@ -258,7 +261,7 @@ public: } // operator+= - CString& operator+=(char c) & noexcept { + CString& operator+=(char const c) & noexcept { return append(c); } @@ -387,16 +390,69 @@ inline CString operator+(const char* lhs, CString rhs) { return rhs; } -inline CString operator+(CString lhs, char rhs) { +inline CString operator+(CString lhs, char const rhs) { lhs += rhs; return lhs; } -inline CString operator+(char lhs, CString rhs) { +inline CString operator+(char const lhs, CString rhs) { rhs.insert(0, lhs); return rhs; } +// CString vs StringLiteral +template + bool operator==(CString const& lhs, const StringLiteral& rhs) noexcept { + return std::string_view{lhs.data(), lhs.size()} == std::string_view{rhs, N - 1}; +} +template + bool operator!=(CString const& lhs, const StringLiteral& rhs) noexcept { + return std::string_view{lhs.data(), lhs.size()} != std::string_view{rhs, N - 1}; +} +template + bool operator<(CString const& lhs, const StringLiteral& rhs) noexcept { + return std::string_view{lhs.data(), lhs.size()} < std::string_view{rhs, N - 1}; +} +template + bool operator>(CString const& lhs, const StringLiteral& rhs) noexcept { + return std::string_view{lhs.data(), lhs.size()} > std::string_view{rhs, N - 1}; +} +template + bool operator<=(CString const& lhs, const StringLiteral& rhs) noexcept { + return std::string_view{lhs.data(), lhs.size()} <= std::string_view{rhs, N - 1}; +} +template + bool operator>=(CString const& lhs, const StringLiteral& rhs) noexcept { + return std::string_view{lhs.data(), lhs.size()} >= std::string_view{rhs, N - 1}; +} + +// StringLiteral vs CString +template + bool operator==(const StringLiteral& lhs, CString const& rhs) noexcept { + return std::string_view{lhs, M - 1} == std::string_view{rhs.data(), rhs.size()}; +} +template + bool operator!=(const StringLiteral& lhs, CString const& rhs) noexcept { + return std::string_view{lhs, M - 1} != std::string_view{rhs.data(), rhs.size()}; +} +template + bool operator<(const StringLiteral& lhs, CString const& rhs) noexcept { + return std::string_view{lhs, M - 1} < std::string_view{rhs.data(), rhs.size()}; +} +template + bool operator>(const StringLiteral& lhs, CString const& rhs) noexcept { + return std::string_view{lhs, M - 1} > std::string_view{rhs.data(), rhs.size()}; +} +template + bool operator<=(const StringLiteral& lhs, CString const& rhs) noexcept { + return std::string_view{lhs, M - 1} <= std::string_view{rhs.data(), rhs.size()}; +} +template + bool operator>=(const StringLiteral& lhs, CString const& rhs) noexcept { + return std::string_view{lhs, M - 1} >= std::string_view{rhs.data(), rhs.size()}; +} + + // Implement this for your type for automatic conversion to CString. Failing to do so leads // to a compile-time failure. template diff --git a/libs/utils/src/CString.cpp b/libs/utils/src/CString.cpp index 13c30d7daf..d515d02457 100644 --- a/libs/utils/src/CString.cpp +++ b/libs/utils/src/CString.cpp @@ -53,10 +53,6 @@ CString::CString(size_t const length) { } } -CString::CString(const char* cstr) - : CString(cstr, size_type(cstr ? strlen(cstr) : 0)) { -} - CString::CString(const CString& rhs) : CString(rhs.c_str(), rhs.size()) { } diff --git a/libs/utils/test/test_CString.cpp b/libs/utils/test/test_CString.cpp index a69b59397c..e37861aa92 100644 --- a/libs/utils/test/test_CString.cpp +++ b/libs/utils/test/test_CString.cpp @@ -21,6 +21,7 @@ #include #include #include +#include using namespace utils; @@ -56,7 +57,8 @@ TEST(CString, Constructors) { } // CString(const char* cstr) { - CString str("hello"); + const char* hello_cstr = "hello"; + CString str(hello_cstr); EXPECT_STREQ("hello", str.c_str()); EXPECT_EQ(5, str.length()); } @@ -84,6 +86,50 @@ TEST(CString, Constructors) { } } +TEST(CString, LiteralConstructor) { + // This test verifies that CString can be constructed from a string literal, + // and that this uses the desired template constructor, not the explicit + // (and costly) `const char*` constructor. + + // Test copy-initialization. CString should be convertible from a string literal. + // This works because "literal" has type `const char[N]` which matches + // `StringLiteral` and calls the non-explicit template constructor. + static_assert(std::is_convertible_v, + "CString should be convertible from a string literal."); + CString s_copy = "literal"; + EXPECT_STREQ("literal", s_copy.c_str()); + EXPECT_EQ(7, s_copy.length()); + + // Test direct-initialization. This should also use the same efficient constructor. + static_assert(std::is_constructible_v, + "CString should be constructible from a string literal."); + CString s_direct("literal"); + EXPECT_STREQ("literal", s_direct.c_str()); + EXPECT_EQ(7, s_direct.length()); + + // Verify that direct-initialization uses the sized constructor by using an embedded null. + // If the strlen() constructor were used, the length would be 3. + // The size of "abc\0def" is 8 (including the terminating null). The constructor + // calculates the length as N-1 = 7. + CString const s_direct_null("abc\0def"); + EXPECT_EQ(7, s_direct_null.length()); + + // CString should NOT be convertible from a `const char*`. + // This is because the `const char*` constructor is explicit. + static_assert(!std::is_convertible_v, + "CString should not be convertible from a const char*."); + + // This demonstrates the non-convertibility. The following line would fail to compile: + // const char* cstr = "hello"; + // CString s2 = cstr; + + // Explicit construction from a const char* should still work (via direct-initialization). + const char* cstr = "explicit"; + CString s2(cstr); + EXPECT_STREQ("explicit", s2.c_str()); + EXPECT_EQ(8, s2.length()); +} + TEST(CString, Assignment) { // Copy assignment {