llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: Nikolas Klauser (philnik777)
<details>
<summary>Changes</summary>
When doing amortized growth we currently have separate functions for calculating the new capacity and allocating. However, we set the size to the calculated capacity that way instead of the actually required size. This makes the interface quite confusing, since the size has to be set manually. Instead, this patch refactors the function to get the amortized growth capacity to instead allocate and set the size correctly.
---
Full diff: https://github.com/llvm/llvm-project/pull/171117.diff
1 Files Affected:
- (modified) libcxx/include/string (+21-15)
``````````diff
diff --git a/libcxx/include/string b/libcxx/include/string
index 34af7efb56659..25595605bc111 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -2285,6 +2285,25 @@ private:
return __long(__buffer, __capacity);
}
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __long
+ __allocate_long_buffer_for_amortized_growth(size_type __required_size) {
+ auto __new_cap = [&] {
+ size_type __max_size = max_size();
+ if (__required_size > __max_size)
+ __throw_length_error();
+ size_type __current_cap = capacity();
+ _LIBCPP_ASSERT_INTERNAL(
+ __current_cap < __required_size, "Trying to grow string even though there is enough capacity already?");
+ if (__current_cap > __max_size / 2 - __alignment)
+ return __max_size;
+ return std::max(__required_size, 2 * __current_cap);
+ }();
+
+ __long __buffer = __allocate_long_buffer(__alloc_, __new_cap);
+ __buffer.__size_ = __required_size;
+ return __buffer;
+ }
+
// Replace the current buffer with __new_rep. Deallocate the old long buffer if it exists.
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __reset_internal_buffer(__rep __new_rep = __short()) {
__annotate_delete();
@@ -2384,19 +2403,6 @@ private:
return __guess;
}
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type
- __get_amortized_growth_capacity(size_type __required_capacity) {
- size_type __max_size = max_size();
- if (__required_capacity > __max_size)
- __throw_length_error();
- size_type __current_cap = capacity();
- _LIBCPP_ASSERT_INTERNAL(
- __current_cap < __required_capacity, "Trying to grow string even though there is enough capacity already?");
- if (__current_cap > __max_size / 2 - __alignment)
- return __max_size;
- return std::max(__required_capacity, 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);
@@ -2728,7 +2734,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::__
size_type __n_del,
size_type __n_add,
const value_type* __p_new_stuff) {
- __long __buffer = __allocate_long_buffer(__alloc_, __get_amortized_growth_capacity(__old_cap + __delta_cap));
+ __long __buffer = __allocate_long_buffer_for_amortized_growth(__old_cap + __delta_cap);
pointer __old_p = __get_pointer();
__annotate_delete();
auto __guard = std::__make_scope_guard(__annotate_new_size(*this));
@@ -2761,7 +2767,7 @@ _LIBCPP_DEPRECATED_("use __grow_by_without_replace") basic_string<_CharT, _Trait
size_type __n_copy,
size_type __n_del,
size_type __n_add) {
- __long __buffer = __allocate_long_buffer(__alloc_, __get_amortized_growth_capacity(__old_cap + __delta_cap));
+ __long __buffer = __allocate_long_buffer_for_amortized_growth(__old_cap + __delta_cap);
pointer __old_p = __get_pointer();
if (__n_copy != 0)
traits_type::copy(std::__to_address(__buffer.__data_), std::__to_address(__old_p), __n_copy);
``````````
</details>
https://github.com/llvm/llvm-project/pull/171117