[libc-commits] [libc] [libc][cpp::string] Do not count null terminator in capacity() (PR #211918)

[Jackson Stogel via libc-commits]

https://github.com/jtstogel updated https://github.com/llvm/llvm-project/pull/211918

>From 9e5db7d6dab2f1d143fdc32dbd5fcb67eeb96202 Mon Sep 17 00:00:00 2001
From: jtstogel <jtstogel at gmail.com>
Date: Fri, 24 Jul 2026 13:22:23 -0700
Subject: [PATCH] [libc][cpp::string] Do not include null terminator in
 capacity()

Currently, `capacity()` includes the null terminator of the backing buffer, which does not align with C++20's `std::string`. That is, `s.reserve(s.capacity())` should be a no-op. While there's no requirement these be in-sync, it is less surprising to align its behavior.

Also, this fixes the case where `reserve()` is called on an empty string, where previously the null terminator was not set.
---
 libc/src/__support/CPP/string.h             | 38 +++++++++++++++------
 libc/test/src/__support/CPP/string_test.cpp | 24 +++++++++++--
 2 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/libc/src/__support/CPP/string.h b/libc/src/__support/CPP/string.h
index 274c6b67cb4d8..0de0e11a583bf 100644
--- a/libc/src/__support/CPP/string.h
+++ b/libc/src/__support/CPP/string.h
@@ -45,8 +45,14 @@ class string {
     return const_cast<char *>(&NULL_CHARACTER);
   }
 
+  // Backing data for the represented string.
+  // This may point to NULL_CHARACTER or a heap-allocated buffer.
   char *buffer_ = get_empty_string();
+
+  // Size of the string represented in buffer_.
   size_t size_ = 0;
+
+  // The size of buffer_.
   size_t capacity_ = 0;
 
   constexpr void reset_no_deallocate() {
@@ -112,7 +118,13 @@ class string {
       ::free(buffer_);
   }
 
-  LIBC_INLINE constexpr size_t capacity() const { return capacity_; }
+  // Returns the number of writable bytes in this string.
+  // Does not include the string-managed null terminator.
+  LIBC_INLINE constexpr size_t capacity() const {
+    if (capacity_ == 0)
+      return 0;
+    return capacity_ - 1;
+  }
   LIBC_INLINE constexpr size_t size() const { return size_; }
   LIBC_INLINE constexpr bool empty() const { return size_ == 0; }
 
@@ -142,19 +154,25 @@ class string {
     return string_view(buffer_, size_);
   }
 
-  LIBC_INLINE void reserve(size_t new_capacity) {
-    ++new_capacity; // Accounting for the terminating '\0'
-    if (new_capacity <= capacity_)
+  LIBC_INLINE void reserve(size_t new_cap) {
+    size_t allocation_size = new_cap + 1; // +1 for terminating '\0'
+    if (allocation_size <= 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;
+    if (allocation_size < SIZE_MAX / 11)
+      allocation_size = allocation_size * 11 / 8;
+
+    if (buffer_ == get_empty_string()) {
+      buffer_ = realloc_or_die(nullptr, allocation_size);
+      buffer_[0] = NULL_CHARACTER;
+    } else {
+      buffer_ = realloc_or_die(buffer_, allocation_size);
+    }
 
-    buffer_ = realloc_or_die(buffer_ == get_empty_string() ? nullptr : buffer_,
-                             new_capacity);
-    capacity_ = new_capacity;
+    capacity_ = allocation_size;
   }
 
   LIBC_INLINE void resize(size_t size) {
@@ -163,7 +181,7 @@ class string {
     if (size == size_)
       return;
 
-    if (size >= capacity_) {
+    if (size > capacity()) {
       reserve(size);
       const size_t size_extension = size - size_;
       inline_memset(data() + size_, '\0', size_extension);
diff --git a/libc/test/src/__support/CPP/string_test.cpp b/libc/test/src/__support/CPP/string_test.cpp
index 4a8c043d5d7a8..ba1992dd2da39 100644
--- a/libc/test/src/__support/CPP/string_test.cpp
+++ b/libc/test/src/__support/CPP/string_test.cpp
@@ -173,12 +173,12 @@ TEST(LlvmLibcStringTest, ResizeCapacityAndNullTermination) {
   // One char
   a.resize(1);
   ASSERT_EQ(a.size(), size_t(1));
-  ASSERT_GE(a.capacity(), size_t(2));
+  ASSERT_GE(a.capacity(), size_t(1));
   ASSERT_EQ(a.data()[1], '\0');
   // Clear
   a.resize(0);
   ASSERT_EQ(a.size(), size_t(0));
-  ASSERT_GE(a.capacity(), size_t(2));
+  ASSERT_GE(a.capacity(), size_t(1));
   ASSERT_EQ(a.data()[0], '\0');
   // Resize and check zero initialized
   a.resize(10);
@@ -193,10 +193,28 @@ TEST(LlvmLibcStringTest, ResizeWithCapacityPlus1) {
   a.resize(32);
 
   size_t previous_capacity = a.capacity();
-  a.resize(previous_capacity);
+  a.resize(previous_capacity + 1);
   ASSERT_GT(a.capacity(), previous_capacity);
 }
 
+TEST(LlvmLibcStringTest, ReserveWithSameCapacityIsNop) {
+  string a;
+  a.resize(32);
+
+  size_t previous_capacity = a.capacity();
+  // Since C++20, calling reserve with capacity less than or equal
+  // to the current capacity should have no effect.
+  a.reserve(previous_capacity);
+  ASSERT_EQ(a.capacity(), previous_capacity);
+}
+
+TEST(LlvmLibcStringTest, ReserveOnEmptyStringKeepsNullTerminator) {
+  string s;
+  s.reserve(10);
+  ASSERT_EQ(s.size(), size_t(0));
+  ASSERT_EQ(s[0], '\0');
+}
+
 TEST(LlvmLibcStringTest, ConcatWithCString) {
   ASSERT_STREQ((string("a") + string("b")).c_str(), "ab");
   ASSERT_STREQ((string("a") + "b").c_str(), "ab");