Skip to content

Conversation

@philnik777
Copy link
Contributor

Introducing this utility makes the __grow_by{,_and_replace} significantly easier to understand and allows us to migrate away from these functions in the future.

@philnik777 philnik777 requested a review from a team as a code owner October 9, 2025 10:39
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Oct 9, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 9, 2025

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

Introducing this utility makes the __grow_by{,_and_replace} significantly easier to understand and allows us to migrate away from these functions in the future.


Full diff: https://github.com/llvm/llvm-project/pull/162633.diff

1 Files Affected:

  • (modified) libcxx/include/string (+12-12)
diff --git a/libcxx/include/string b/libcxx/include/string
index 363f27a51648c..a7b17e376675d 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -2357,6 +2357,16 @@ private:
     return __guess;
   }
 
+  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type __recommend_growing(size_type __required_size) {
+    size_type __max_size = max_size();
+    if (__required_size > __max_size)
+      __throw_length_error();
+    size_type __current_cap = capacity();
+    if (__current_cap > __max_size / 2 - __alignment)
+      return __max_size;
+    return std::max(__required_size, 2 * __current_cap);
+  }
+
   inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void __init(const value_type* __s, size_type __sz);
   inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void __init(size_type __n, value_type __c);
 
@@ -2701,15 +2711,10 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::__
     size_type __n_del,
     size_type __n_add,
     const value_type* __p_new_stuff) {
-  size_type __ms = max_size();
-  if (__delta_cap > __ms - __old_cap)
-    __throw_length_error();
+  __long __buffer = __allocate_long_buffer(__alloc_, __recommend_growing(__old_cap + __delta_cap));
   pointer __old_p = __get_pointer();
-  size_type __cap =
-      __old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms;
   __annotate_delete();
   auto __guard    = std::__make_scope_guard(__annotate_new_size(*this));
-  __long __buffer = __allocate_long_buffer(__alloc_, __cap);
   if (__n_copy != 0)
     traits_type::copy(std::__to_address(__buffer.__data_), std::__to_address(__old_p), __n_copy);
   if (__n_add != 0)
@@ -2739,13 +2744,8 @@ _LIBCPP_DEPRECATED_("use __grow_by_without_replace") basic_string<_CharT, _Trait
     size_type __n_copy,
     size_type __n_del,
     size_type __n_add) {
-  size_type __ms = max_size();
-  if (__delta_cap > __ms - __old_cap)
-    this->__throw_length_error();
+  __long __buffer = __allocate_long_buffer(__alloc_, __recommend_growing(__old_cap + __delta_cap));
   pointer __old_p = __get_pointer();
-  size_type __cap =
-      __old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms;
-  __long __buffer = __allocate_long_buffer(__alloc_, __cap);
   if (__n_copy != 0)
     traits_type::copy(std::__to_address(__buffer.__data_), std::__to_address(__old_p), __n_copy);
   size_type __sec_cp_sz = __old_sz - __n_del - __n_copy;

@github-actions
Copy link

github-actions bot commented Oct 9, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@philnik777 philnik777 force-pushed the string_introduce_recommend_growing branch 2 times, most recently from 7b1e194 to 99381cd Compare November 17, 2025 15:04
Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM w/ small comments and assuming no regression in the string benchmarks and SPEC.

@philnik777 philnik777 force-pushed the string_introduce_recommend_growing branch from 99381cd to 4d1a441 Compare November 18, 2025 11:31
@philnik777 philnik777 changed the title [libc++] Introduce basic_string::__recommend_growing [libc++] Introduce basic_string::__allocate_long_buffer_for_growing Nov 18, 2025
Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with suggestion. Please run the benchmarks to ensure there's no regression, this code is sensitive.

@philnik777 philnik777 force-pushed the string_introduce_recommend_growing branch from 4d1a441 to 465e000 Compare November 19, 2025 18:11
@philnik777 philnik777 merged commit 121e2e9 into llvm:main Nov 24, 2025
79 of 80 checks passed
@philnik777 philnik777 deleted the string_introduce_recommend_growing branch November 24, 2025 10:09
aadeshps-mcw pushed a commit to aadeshps-mcw/llvm-project that referenced this pull request Nov 26, 2025
…lvm#162633)

Introducing this utility makes the `__grow_by{,_and_replace}`
significantly easier to understand and allows us to migrate away from
these functions in the future.
Priyanshu3820 pushed a commit to Priyanshu3820/llvm-project that referenced this pull request Nov 26, 2025
…lvm#162633)

Introducing this utility makes the `__grow_by{,_and_replace}`
significantly easier to understand and allows us to migrate away from
these functions in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants