[libc-commits] [libc] [llvm] [libc][cpp::string] Allocate fewer temp strings in operator= and += (PR #210895)
Jackson Stogel via libc-commits
libc-commits at lists.llvm.org
Tue Jul 21 16:45:54 PDT 2026
https://github.com/jtstogel updated https://github.com/llvm/llvm-project/pull/210895
>From caeeaa36f4c0c2a613bdd32dc56b855317012ae8 Mon Sep 17 00:00:00 2001
From: jtstogel <jtstogel at gmail.com>
Date: Mon, 13 Jul 2026 14:38:32 -0700
Subject: [PATCH 1/4] [libc][cpp::string] Allocate fewer temp strings in
operator= and +=
This PR generally updates `cpp::string` to avoid incidental allocations. Specifically, it:
- Updates `opreator=(string_view)` to avoid allocating a temporary string: https://github.com/llvm/llvm-project/blob/67ebc4b221c3e94028b33004cd5cd08deee95048/libc/src/__support/CPP/string.h#L106-L108
- Changes `operator+=(const string&)` to accept a `string_view` so that strings may be appended without allocation.
- Makes the `string(string_view)` constructor explicit. Before, there were non-obvious allocations because of the implicit conversion.
This PR assumes that self-assignment and self-appends are not supported with `cpp::string`, and adds debug asserts preventing them. Currently, assignment doesn't work. `cpp::string s = "abc"; s = s;` will just zero out the string. This PR is a slight regression in that `operator+=(const cpp::string&)` previously worked, but now it errors in debug mode, and would result in a use-after-free without the assert statement. I don't think self-append/self-assignment is something `cpp::string` needs to support, so I omitted it, but if a reviewer thinks this behavior is important happy to amend this PR.
---
libc/src/__support/CPP/CMakeLists.txt | 2 +
libc/src/__support/CPP/string.h | 38 ++++++++++++-------
libc/test/UnitTest/LibcTest.cpp | 2 +-
libc/test/src/__support/CPP/CMakeLists.txt | 1 +
libc/test/src/__support/CPP/string_test.cpp | 13 +++++++
libc/test/src/stdlib/realpath_test.cpp | 5 ++-
.../llvm-project-overlay/libc/BUILD.bazel | 2 +
.../libc/test/src/__support/CPP/BUILD.bazel | 5 ++-
8 files changed, 51 insertions(+), 17 deletions(-)
diff --git a/libc/src/__support/CPP/CMakeLists.txt b/libc/src/__support/CPP/CMakeLists.txt
index 49cb07f329111..af2f4b4b359f4 100644
--- a/libc/src/__support/CPP/CMakeLists.txt
+++ b/libc/src/__support/CPP/CMakeLists.txt
@@ -88,8 +88,10 @@ add_header_library(
libc.hdr.func.free
libc.hdr.func.malloc
libc.hdr.func.realloc
+ libc.hdr.stdint_proxy
libc.src.__support.common
libc.src.__support.integer_to_string
+ libc.src.__support.libc_assert
libc.src.__support.macros.null_check
libc.src.string.memory_utils.inline_memcpy
libc.src.string.memory_utils.inline_memset
diff --git a/libc/src/__support/CPP/string.h b/libc/src/__support/CPP/string.h
index 274c6b67cb4d8..a22e06e759572 100644
--- a/libc/src/__support/CPP/string.h
+++ b/libc/src/__support/CPP/string.h
@@ -12,8 +12,10 @@
#include "hdr/func/free.h"
#include "hdr/func/malloc.h"
#include "hdr/func/realloc.h"
+#include "hdr/stdint_proxy.h"
#include "src/__support/CPP/string_view.h"
#include "src/__support/integer_to_string.h" // IntegerToString
+#include "src/__support/libc_assert.h"
#include "src/__support/macros/config.h"
#include "src/__support/macros/null_check.h"
#include "src/string/memory_utils/inline_memcpy.h"
@@ -61,6 +63,15 @@ class string {
buffer_[size_] = NULL_CHARACTER;
}
+ // Whether ptr lies within this string's buffer.
+ LIBC_INLINE bool addr_in_string_bounds(const char *ptr) {
+ uintptr_t addr = reinterpret_cast<uintptr_t>(ptr);
+ uintptr_t start = reinterpret_cast<uintptr_t>(data());
+ uintptr_t end = start + capacity_;
+
+ return start <= addr && addr < end;
+ }
+
public:
LIBC_INLINE constexpr string() {}
LIBC_INLINE string(const string &other) { this->operator+=(other); }
@@ -72,7 +83,7 @@ class string {
resize(count);
inline_memcpy(buffer_, cstr, count);
}
- LIBC_INLINE string(const string_view &view)
+ LIBC_INLINE explicit string(const string_view &view)
: string(view.data(), view.size()) {}
LIBC_INLINE string(const char *cstr)
: string(cstr, ::LIBC_NAMESPACE::internal::string_length(cstr)) {}
@@ -83,13 +94,11 @@ class string {
}
LIBC_INLINE string &operator=(const string &other) {
- resize(0);
- return (*this) += other;
+ return (*this) = string_view(other);
}
LIBC_INLINE string &operator=(char other) {
- resize(0);
- return (*this) += other;
+ return (*this) = string_view(&other, 1);
}
LIBC_INLINE string &operator=(string &&other) {
@@ -103,8 +112,11 @@ class string {
return *this;
}
- LIBC_INLINE string &operator=(const string_view &view) {
- return *this = string(view);
+ LIBC_INLINE string &operator=(string_view view) {
+ LIBC_ASSERT(!addr_in_string_bounds(view.data()));
+
+ resize(0);
+ return (*this) += view;
}
LIBC_INLINE ~string() {
@@ -189,7 +201,9 @@ class string {
return res;
}
- LIBC_INLINE string &operator+=(const string &rhs) {
+ LIBC_INLINE string &operator+=(string_view rhs) {
+ LIBC_ASSERT(!addr_in_string_bounds(rhs.data()));
+
const size_t new_size = size_ + rhs.size();
reserve(new_size);
inline_memcpy(buffer_ + size_, rhs.data(), rhs.size());
@@ -198,11 +212,7 @@ class string {
}
LIBC_INLINE string &operator+=(const char c) {
- const size_t new_size = size_ + 1;
- reserve(new_size);
- buffer_[size_] = c;
- set_size_and_add_null_character(new_size);
- return *this;
+ return *this += string_view(&c, 1);
}
};
@@ -239,7 +249,7 @@ LIBC_INLINE string operator+(const char *lhs, const string &rhs) {
namespace internal {
template <typename T> string to_dec_string(T value) {
const IntegerToString<T> buffer(value);
- return buffer.view();
+ return string(buffer.view());
}
} // namespace internal
diff --git a/libc/test/UnitTest/LibcTest.cpp b/libc/test/UnitTest/LibcTest.cpp
index b03ceb6c7c77d..d8e5314ece213 100644
--- a/libc/test/UnitTest/LibcTest.cpp
+++ b/libc/test/UnitTest/LibcTest.cpp
@@ -45,7 +45,7 @@ cpp::enable_if_t<(cpp::is_integral_v<T> && (sizeof(T) > sizeof(uint64_t))) ||
cpp::string>
describeValue(T Value) {
const IntegerToString<T, radix::Hex::WithPrefix> buffer(Value);
- return buffer.view();
+ return cpp::string(buffer.view());
}
// When the value is of a standard integral type, just display it as normal.
diff --git a/libc/test/src/__support/CPP/CMakeLists.txt b/libc/test/src/__support/CPP/CMakeLists.txt
index ac0e8289e64bd..71a0c94918dd4 100644
--- a/libc/test/src/__support/CPP/CMakeLists.txt
+++ b/libc/test/src/__support/CPP/CMakeLists.txt
@@ -181,6 +181,7 @@ add_libc_test(
string_test.cpp
DEPENDS
libc.hdr.func.free
+ libc.hdr.signal_macros
libc.src.__support.CPP.string
libc.src.__support.CPP.string_view
)
diff --git a/libc/test/src/__support/CPP/string_test.cpp b/libc/test/src/__support/CPP/string_test.cpp
index 4a8c043d5d7a8..7661db12e9bb6 100644
--- a/libc/test/src/__support/CPP/string_test.cpp
+++ b/libc/test/src/__support/CPP/string_test.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "hdr/func/free.h"
+#include "hdr/signal_macros.h"
#include "src/__support/CPP/string.h"
#include "test/UnitTest/Test.h"
@@ -259,3 +260,15 @@ TEST(LlvmLibcStringTest, ToString) {
}
}
}
+
+#if !defined(NDEBUG) && defined(ENABLE_SUBPROCESS_TESTS)
+TEST(LlvmLibcStringTest, SelfAssignDebugDeathTest) {
+ string s("abc");
+ ASSERT_DEATH([&]() { s = string_view(s).substr(2); }, WITH_SIGNAL(SIGABRT));
+}
+
+TEST(LlvmLibcStringTest, SelfAppendDebugDeathTest) {
+ string s("abc");
+ ASSERT_DEATH([&]() { s += string_view(s).substr(2); }, WITH_SIGNAL(SIGABRT));
+}
+#endif
diff --git a/libc/test/src/stdlib/realpath_test.cpp b/libc/test/src/stdlib/realpath_test.cpp
index c987179ff89cf..f7dc440a26278 100644
--- a/libc/test/src/stdlib/realpath_test.cpp
+++ b/libc/test/src/stdlib/realpath_test.cpp
@@ -118,7 +118,10 @@ class TestDir {
// Returns the absolute path of `relative_path` in this test directory.
cpp::string absolute_path(cpp::string_view relative_path) const {
- return path + "/" + relative_path;
+ cpp::string res = path;
+ res += "/";
+ res += relative_path;
+ return res;
}
// Returns this test directory path as a C string.
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index 6020f511d0d3b..4b97a13d95c45 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -1207,11 +1207,13 @@ libc_support_library(
":__support_common",
":__support_cpp_string_view",
":__support_integer_to_string",
+ ":__support_libc_assert",
":__support_macros_config",
":__support_macros_null_check",
":func_free",
":func_malloc",
":func_realloc",
+ ":hdr_stdint_proxy",
":string_memory_utils",
":string_utils",
],
diff --git a/utils/bazel/llvm-project-overlay/libc/test/src/__support/CPP/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/src/__support/CPP/BUILD.bazel
index 399a88982f19a..9be61e1c27acd 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/src/__support/CPP/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/src/__support/CPP/BUILD.bazel
@@ -81,7 +81,10 @@ libc_test(
libc_test(
name = "string_test",
srcs = ["string_test.cpp"],
- deps = ["//libc:__support_cpp_string"],
+ deps = [
+ "//libc:__support_cpp_string",
+ "//libc:hdr_signal_macros",
+ ],
)
libc_test(
>From 1e273cad5d73b0e95769aaaa48ababf03603d93f Mon Sep 17 00:00:00 2001
From: jtstogel <jtstogel at gmail.com>
Date: Tue, 21 Jul 2026 11:45:48 -0700
Subject: [PATCH 2/4] Handling aliasing
---
libc/src/__support/CPP/CMakeLists.txt | 1 +
libc/src/__support/CPP/string.h | 122 ++++++++++++++------
libc/test/src/__support/CPP/CMakeLists.txt | 1 -
libc/test/src/__support/CPP/string_test.cpp | 45 ++++++--
4 files changed, 121 insertions(+), 48 deletions(-)
diff --git a/libc/src/__support/CPP/CMakeLists.txt b/libc/src/__support/CPP/CMakeLists.txt
index af2f4b4b359f4..fb75a6f7fffa7 100644
--- a/libc/src/__support/CPP/CMakeLists.txt
+++ b/libc/src/__support/CPP/CMakeLists.txt
@@ -94,6 +94,7 @@ add_header_library(
libc.src.__support.libc_assert
libc.src.__support.macros.null_check
libc.src.string.memory_utils.inline_memcpy
+ libc.src.string.memory_utils.inline_memmove
libc.src.string.memory_utils.inline_memset
libc.src.string.string_utils
)
diff --git a/libc/src/__support/CPP/string.h b/libc/src/__support/CPP/string.h
index a22e06e759572..261b8685059fd 100644
--- a/libc/src/__support/CPP/string.h
+++ b/libc/src/__support/CPP/string.h
@@ -19,6 +19,7 @@
#include "src/__support/macros/config.h"
#include "src/__support/macros/null_check.h"
#include "src/string/memory_utils/inline_memcpy.h"
+#include "src/string/memory_utils/inline_memmove.h"
#include "src/string/memory_utils/inline_memset.h"
#include "src/string/string_utils.h" // string_length
@@ -35,6 +36,8 @@ char *realloc_or_die(char *ptr, size_t size) {
return reinterpret_cast<char *>(new_ptr);
}
+char *malloc_or_die(size_t size) { return realloc_or_die(nullptr, size); }
+
} // namespace
// This class mimics std::string but does not intend to be a full fledged
@@ -63,13 +66,46 @@ class string {
buffer_[size_] = NULL_CHARACTER;
}
- // Whether ptr lies within this string's buffer.
- LIBC_INLINE bool addr_in_string_bounds(const char *ptr) {
- uintptr_t addr = reinterpret_cast<uintptr_t>(ptr);
- uintptr_t start = reinterpret_cast<uintptr_t>(data());
- uintptr_t end = start + capacity_;
+ // Assigns the new buffer, capacity, and size to this string,
+ // freeing the current internal buffer.
+ void move_assign_from_buffer(char *new_buffer, size_t new_capacity,
+ size_t new_size) {
+ if (buffer_ != get_empty_string())
+ ::free(buffer_);
+
+ buffer_ = new_buffer;
+ size_ = new_size;
+ capacity_ = new_capacity;
+ }
+
+ // Returns the capacity this string should have after growing to fit new_size.
+ LIBC_INLINE size_t capacity_needed_for_size(size_t new_size) {
+ size_t new_capacity = new_size + 1; // +1 for the terminating '\0'
+ if (new_capacity <= capacity_)
+ return capacity_;
+
+ // We extend the capacity to amortize buffer_ reallocations.
+ // We choose to augment the value by 11 / 8, this is about +40% and division
+ // by 8 is cheap. We guard the extension so the operation doesn't overflow.
+ if (new_capacity < SIZE_MAX / 11)
+ new_capacity = new_capacity * 11 / 8;
+ return new_capacity;
+ }
+
+ // Replaces the current buffer with a larger one.
+ // The first `keep_size` bytes of the current buffer will be copied over,
+ // after which `new_data` will be appended.
+ LIBC_INLINE void grow_and_replace(size_t keep_size,
+ cpp::string_view new_data) {
+ size_t new_size = keep_size + new_data.size();
+ size_t new_capacity = capacity_needed_for_size(new_size);
+ char *new_buffer = malloc_or_die(new_capacity);
+
+ inline_memcpy(new_buffer, buffer_, keep_size);
+ inline_memcpy(new_buffer + keep_size, new_data.data(), new_data.size());
- return start <= addr && addr < end;
+ move_assign_from_buffer(new_buffer, new_capacity, new_size);
+ set_size_and_add_null_character(new_size);
}
public:
@@ -93,32 +129,36 @@ class string {
inline_memset((void *)buffer_, static_cast<uint8_t>(value), size_);
}
- LIBC_INLINE string &operator=(const string &other) {
- return (*this) = string_view(other);
+ LIBC_INLINE string &assign(cpp::string_view view) {
+ if (view.empty()) {
+ set_size_and_add_null_character(0);
+ return *this;
+ }
+
+ if (capacity_ <= view.size()) {
+ grow_and_replace(0, view);
+ return *this;
+ }
+
+ inline_memmove(buffer_, view.data(), view.size());
+ set_size_and_add_null_character(view.size());
+ return *this;
}
+ LIBC_INLINE string &operator=(const string &other) { return assign(other); }
+
LIBC_INLINE string &operator=(char other) {
- return (*this) = string_view(&other, 1);
+ return assign(string_view(&other, 1));
}
- LIBC_INLINE string &operator=(string &&other) {
- if (buffer_ != get_empty_string())
- ::free(buffer_);
+ LIBC_INLINE string &operator=(string_view view) { return assign(view); }
- buffer_ = other.buffer_;
- size_ = other.size_;
- capacity_ = other.capacity_;
+ LIBC_INLINE string &operator=(string &&other) {
+ move_assign_from_buffer(other.buffer_, other.capacity_, other.size_);
other.reset_no_deallocate();
return *this;
}
- LIBC_INLINE string &operator=(string_view view) {
- LIBC_ASSERT(!addr_in_string_bounds(view.data()));
-
- resize(0);
- return (*this) += view;
- }
-
LIBC_INLINE ~string() {
if (buffer_ != get_empty_string())
::free(buffer_);
@@ -154,19 +194,18 @@ class string {
return string_view(buffer_, size_);
}
- LIBC_INLINE void reserve(size_t new_capacity) {
- ++new_capacity; // Accounting for the terminating '\0'
+ LIBC_INLINE void reserve(size_t new_size) {
+ size_t new_capacity = capacity_needed_for_size(new_size);
if (new_capacity <= capacity_)
return;
- // We extend the capacity to amortize buffer_ reallocations.
- // We choose to augment the value by 11 / 8, this is about +40% and division
- // by 8 is cheap. We guard the extension so the operation doesn't overflow.
- if (new_capacity < SIZE_MAX / 11)
- new_capacity = new_capacity * 11 / 8;
- buffer_ = realloc_or_die(buffer_ == get_empty_string() ? nullptr : buffer_,
- new_capacity);
+ bool is_empty_string = buffer_ == get_empty_string();
+ buffer_ = realloc_or_die(is_empty_string ? nullptr : buffer_, new_capacity);
capacity_ = new_capacity;
+
+ // Add null character if case we reallocated out of the empty buffer.
+ if (is_empty_string)
+ set_size_and_add_null_character(0);
}
LIBC_INLINE void resize(size_t size) {
@@ -191,7 +230,7 @@ class string {
if (buffer_ == get_empty_string()) {
// Ensure the buffer is heap allocated,
// so that it may later be passed to `free`.
- char *res = realloc_or_die(nullptr, 1);
+ char *res = malloc_or_die(1);
res[0] = '\0';
return res;
}
@@ -201,18 +240,25 @@ class string {
return res;
}
- LIBC_INLINE string &operator+=(string_view rhs) {
- LIBC_ASSERT(!addr_in_string_bounds(rhs.data()));
+ LIBC_INLINE string &append(cpp::string_view view) {
+ if (view.empty())
+ return *this;
- const size_t new_size = size_ + rhs.size();
- reserve(new_size);
- inline_memcpy(buffer_ + size_, rhs.data(), rhs.size());
+ if (capacity_ - size_ <= view.size()) {
+ grow_and_replace(size_, view);
+ return *this;
+ }
+
+ size_t new_size = size_ + view.size();
+ inline_memcpy(buffer_ + size_, view.data(), view.size());
set_size_and_add_null_character(new_size);
return *this;
}
+ LIBC_INLINE string &operator+=(string_view rhs) { return append(rhs); }
+
LIBC_INLINE string &operator+=(const char c) {
- return *this += string_view(&c, 1);
+ return append(string_view(&c, 1));
}
};
diff --git a/libc/test/src/__support/CPP/CMakeLists.txt b/libc/test/src/__support/CPP/CMakeLists.txt
index 71a0c94918dd4..ac0e8289e64bd 100644
--- a/libc/test/src/__support/CPP/CMakeLists.txt
+++ b/libc/test/src/__support/CPP/CMakeLists.txt
@@ -181,7 +181,6 @@ add_libc_test(
string_test.cpp
DEPENDS
libc.hdr.func.free
- libc.hdr.signal_macros
libc.src.__support.CPP.string
libc.src.__support.CPP.string_view
)
diff --git a/libc/test/src/__support/CPP/string_test.cpp b/libc/test/src/__support/CPP/string_test.cpp
index 7661db12e9bb6..add654438dc1e 100644
--- a/libc/test/src/__support/CPP/string_test.cpp
+++ b/libc/test/src/__support/CPP/string_test.cpp
@@ -7,7 +7,6 @@
//===----------------------------------------------------------------------===//
#include "hdr/func/free.h"
-#include "hdr/signal_macros.h"
#include "src/__support/CPP/string.h"
#include "test/UnitTest/Test.h"
@@ -261,14 +260,42 @@ TEST(LlvmLibcStringTest, ToString) {
}
}
-#if !defined(NDEBUG) && defined(ENABLE_SUBPROCESS_TESTS)
-TEST(LlvmLibcStringTest, SelfAssignDebugDeathTest) {
- string s("abc");
- ASSERT_DEATH([&]() { s = string_view(s).substr(2); }, WITH_SIGNAL(SIGABRT));
+TEST(LlvmLibcStringTest, SelfAssignTest) {
+ string s("abcdefg");
+ s = string_view(s).substr(1);
+ ASSERT_STREQ(s.c_str(), "bcdefg");
}
-TEST(LlvmLibcStringTest, SelfAppendDebugDeathTest) {
- string s("abc");
- ASSERT_DEATH([&]() { s += string_view(s).substr(2); }, WITH_SIGNAL(SIGABRT));
+TEST(LlvmLibcStringTest, SelfAssignAtCapacityTest) {
+ string s("aaa");
+
+ // Append until the string is at its capacity
+ // to exercise self-appending past capacity.
+ while (s.size() + 1 < s.capacity())
+ s += 'a';
+ ASSERT_EQ(s.capacity(), s.size() + 1);
+
+ string longer_string(s.size() + 1, 'b');
+
+ // Force a resize,
+ s = string_view(longer_string);
+ ASSERT_EQ(s, longer_string);
+}
+
+TEST(LlvmLibcStringTest, SelfAppendAtCapacityTest) {
+ string s("aaa");
+
+ // Append until the string is at its capacity
+ // to exercise self-appending past capacity.
+ while (s.size() + 1 < s.capacity())
+ s += 'a';
+ ASSERT_EQ(s.capacity(), s.size() + 1);
+
+ string_view view = string_view(s).substr(0, 3);
+ size_t expected_size = s.size() + view.size();
+
+ s += view;
+
+ string expected(expected_size, 'a');
+ ASSERT_EQ(s, expected);
}
-#endif
>From 132a04d30e0a256dcb19edda8e4d29857cd0be76 Mon Sep 17 00:00:00 2001
From: jtstogel <jtstogel at gmail.com>
Date: Tue, 21 Jul 2026 16:13:44 -0700
Subject: [PATCH 3/4] Remove now unused deps
---
libc/src/__support/CPP/CMakeLists.txt | 2 --
libc/src/__support/CPP/string.h | 35 ++++++++++---------
libc/test/src/__support/CPP/string_test.cpp | 19 +++++++---
.../llvm-project-overlay/libc/BUILD.bazel | 2 --
.../libc/test/src/__support/CPP/BUILD.bazel | 5 +--
5 files changed, 33 insertions(+), 30 deletions(-)
diff --git a/libc/src/__support/CPP/CMakeLists.txt b/libc/src/__support/CPP/CMakeLists.txt
index fb75a6f7fffa7..6ae9b44437e52 100644
--- a/libc/src/__support/CPP/CMakeLists.txt
+++ b/libc/src/__support/CPP/CMakeLists.txt
@@ -88,10 +88,8 @@ add_header_library(
libc.hdr.func.free
libc.hdr.func.malloc
libc.hdr.func.realloc
- libc.hdr.stdint_proxy
libc.src.__support.common
libc.src.__support.integer_to_string
- libc.src.__support.libc_assert
libc.src.__support.macros.null_check
libc.src.string.memory_utils.inline_memcpy
libc.src.string.memory_utils.inline_memmove
diff --git a/libc/src/__support/CPP/string.h b/libc/src/__support/CPP/string.h
index 261b8685059fd..d6298a93c5754 100644
--- a/libc/src/__support/CPP/string.h
+++ b/libc/src/__support/CPP/string.h
@@ -12,10 +12,8 @@
#include "hdr/func/free.h"
#include "hdr/func/malloc.h"
#include "hdr/func/realloc.h"
-#include "hdr/stdint_proxy.h"
#include "src/__support/CPP/string_view.h"
#include "src/__support/integer_to_string.h" // IntegerToString
-#include "src/__support/libc_assert.h"
#include "src/__support/macros/config.h"
#include "src/__support/macros/null_check.h"
#include "src/string/memory_utils/inline_memcpy.h"
@@ -78,7 +76,7 @@ class string {
capacity_ = new_capacity;
}
- // Returns the capacity this string should have after growing to fit new_size.
+ // Returns the capacity this string should grow to for a given size.
LIBC_INLINE size_t capacity_needed_for_size(size_t new_size) {
size_t new_capacity = new_size + 1; // +1 for the terminating '\0'
if (new_capacity <= capacity_)
@@ -92,17 +90,21 @@ class string {
return new_capacity;
}
- // Replaces the current buffer with a larger one.
- // The first `keep_size` bytes of the current buffer will be copied over,
- // after which `new_data` will be appended.
- LIBC_INLINE void grow_and_replace(size_t keep_size,
+ /**
+ * Replaces the current buffer with a new larger one.
+ *
+ * @param keep_prefix_size Prefix length from the current buffer to keep.
+ * @param new_data Data to append after the kept prefix.
+ */
+ LIBC_INLINE void grow_and_replace(size_t keep_prefix_size,
cpp::string_view new_data) {
- size_t new_size = keep_size + new_data.size();
+ size_t new_size = keep_prefix_size + new_data.size();
size_t new_capacity = capacity_needed_for_size(new_size);
char *new_buffer = malloc_or_die(new_capacity);
- inline_memcpy(new_buffer, buffer_, keep_size);
- inline_memcpy(new_buffer + keep_size, new_data.data(), new_data.size());
+ inline_memcpy(new_buffer, buffer_, keep_prefix_size);
+ inline_memcpy(new_buffer + keep_prefix_size, new_data.data(),
+ new_data.size());
move_assign_from_buffer(new_buffer, new_capacity, new_size);
set_size_and_add_null_character(new_size);
@@ -136,7 +138,7 @@ class string {
}
if (capacity_ <= view.size()) {
- grow_and_replace(0, view);
+ grow_and_replace(/* keep_prefix_size= */ 0, view);
return *this;
}
@@ -199,13 +201,12 @@ class string {
if (new_capacity <= capacity_)
return;
- bool is_empty_string = buffer_ == get_empty_string();
- buffer_ = realloc_or_die(is_empty_string ? nullptr : buffer_, new_capacity);
+ buffer_ = realloc_or_die(buffer_ == get_empty_string() ? nullptr : buffer_,
+ new_capacity);
capacity_ = new_capacity;
- // Add null character if case we reallocated out of the empty buffer.
- if (is_empty_string)
- set_size_and_add_null_character(0);
+ // Add null character in case we reallocated out of the empty buffer.
+ set_size_and_add_null_character(size_);
}
LIBC_INLINE void resize(size_t size) {
@@ -245,7 +246,7 @@ class string {
return *this;
if (capacity_ - size_ <= view.size()) {
- grow_and_replace(size_, view);
+ grow_and_replace(/* keep_prefix_size= */ size_, view);
return *this;
}
diff --git a/libc/test/src/__support/CPP/string_test.cpp b/libc/test/src/__support/CPP/string_test.cpp
index add654438dc1e..be999dd656899 100644
--- a/libc/test/src/__support/CPP/string_test.cpp
+++ b/libc/test/src/__support/CPP/string_test.cpp
@@ -261,24 +261,33 @@ TEST(LlvmLibcStringTest, ToString) {
}
TEST(LlvmLibcStringTest, SelfAssignTest) {
- string s("abcdefg");
+ string_view alphabet("abcdefghijklmnopqrstuvwxyz");
+
+ // Test with a string long enough to where memcpy'ing bytes internal
+ // to the string may fail.
+ string complicated_string;
+ for (size_t i = 0; i < 100; i++)
+ complicated_string += alphabet[i % alphabet.size()];
+
+ string s(complicated_string);
+
s = string_view(s).substr(1);
- ASSERT_STREQ(s.c_str(), "bcdefg");
+ ASSERT_EQ(string_view(s), string_view(complicated_string).substr(1));
}
TEST(LlvmLibcStringTest, SelfAssignAtCapacityTest) {
string s("aaa");
// Append until the string is at its capacity
- // to exercise self-appending past capacity.
+ // to exercise self-assigning past capacity.
while (s.size() + 1 < s.capacity())
s += 'a';
ASSERT_EQ(s.capacity(), s.size() + 1);
+ // Force a resize by assigning to a longer string.
string longer_string(s.size() + 1, 'b');
-
- // Force a resize,
s = string_view(longer_string);
+
ASSERT_EQ(s, longer_string);
}
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index 4b97a13d95c45..6020f511d0d3b 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -1207,13 +1207,11 @@ libc_support_library(
":__support_common",
":__support_cpp_string_view",
":__support_integer_to_string",
- ":__support_libc_assert",
":__support_macros_config",
":__support_macros_null_check",
":func_free",
":func_malloc",
":func_realloc",
- ":hdr_stdint_proxy",
":string_memory_utils",
":string_utils",
],
diff --git a/utils/bazel/llvm-project-overlay/libc/test/src/__support/CPP/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/src/__support/CPP/BUILD.bazel
index 9be61e1c27acd..399a88982f19a 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/src/__support/CPP/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/src/__support/CPP/BUILD.bazel
@@ -81,10 +81,7 @@ libc_test(
libc_test(
name = "string_test",
srcs = ["string_test.cpp"],
- deps = [
- "//libc:__support_cpp_string",
- "//libc:hdr_signal_macros",
- ],
+ deps = ["//libc:__support_cpp_string"],
)
libc_test(
>From fe1fea544971cd120cad78428f4381f4d27f0641 Mon Sep 17 00:00:00 2001
From: jtstogel <jtstogel at gmail.com>
Date: Tue, 21 Jul 2026 16:35:57 -0700
Subject: [PATCH 4/4] Handle self-move-assignment
---
libc/src/__support/CPP/string.h | 3 +++
libc/test/src/__support/CPP/string_test.cpp | 7 +++++++
2 files changed, 10 insertions(+)
diff --git a/libc/src/__support/CPP/string.h b/libc/src/__support/CPP/string.h
index d6298a93c5754..c546f0ca8a028 100644
--- a/libc/src/__support/CPP/string.h
+++ b/libc/src/__support/CPP/string.h
@@ -156,6 +156,9 @@ class string {
LIBC_INLINE string &operator=(string_view view) { return assign(view); }
LIBC_INLINE string &operator=(string &&other) {
+ if (this == &other)
+ return *this;
+
move_assign_from_buffer(other.buffer_, other.capacity_, other.size_);
other.reset_no_deallocate();
return *this;
diff --git a/libc/test/src/__support/CPP/string_test.cpp b/libc/test/src/__support/CPP/string_test.cpp
index be999dd656899..ca7cd4ebe86e8 100644
--- a/libc/test/src/__support/CPP/string_test.cpp
+++ b/libc/test/src/__support/CPP/string_test.cpp
@@ -291,6 +291,13 @@ TEST(LlvmLibcStringTest, SelfAssignAtCapacityTest) {
ASSERT_EQ(s, longer_string);
}
+TEST(LlvmLibcStringTest, SelfMoveAssign) {
+ string s("aaa");
+ s = move(s);
+
+ ASSERT_STREQ(s.c_str(), "aaa");
+}
+
TEST(LlvmLibcStringTest, SelfAppendAtCapacityTest) {
string s("aaa");
More information about the libc-commits
mailing list