[libcxx-commits] [libcxx] Reapply "[libc++] Simplify the implementation of reserve() and shrink_to_fit() (#113453)" (PR #125888)
via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Feb 6 00:36:42 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: Nikolas Klauser (philnik777)
<details>
<summary>Changes</summary>
The capacity is now passed correctly and a test for this path is added.
Since we changed the implementation of `reserve(size_type)` to only ever extend,
it doesn't make a ton of sense anymore to have `__shrink_or_extend`, since the code
paths of `reserve` and `shrink_to_fit` are now almost completely separate.
This patch splits up `__shrink_or_extend` so that the individual parts are in `reserve`
and `shrink_to_fit` depending on where they are needed.
This reverts commit 59f57be94f38758616b1339b293b43af845571af.
---
Full diff: https://github.com/llvm/llvm-project/pull/125888.diff
2 Files Affected:
- (modified) libcxx/include/string (+59-60)
- (modified) libcxx/test/std/strings/basic.string/string.capacity/reserve_size.pass.cpp (+9)
``````````diff
diff --git a/libcxx/include/string b/libcxx/include/string
index fdd8085106dcc60..b7f2d1226946392 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1882,8 +1882,6 @@ public:
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __clear_and_shrink() _NOEXCEPT;
private:
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __shrink_or_extend(size_type __target_capacity);
-
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_STRING_INTERNAL_MEMORY_ACCESS bool
__is_long() const _NOEXCEPT {
if (__libcpp_is_constant_evaluated() && __builtin_constant_p(__rep_.__l.__is_long_)) {
@@ -2079,6 +2077,21 @@ private:
# endif
}
+ // Disable ASan annotations and enable them again when going out of scope. It is assumed that the string is in a valid
+ // state at that point, so `size()` can be called safely.
+ struct [[__nodiscard__]] __annotation_guard {
+ __annotation_guard(const __annotation_guard&) = delete;
+ __annotation_guard& operator=(const __annotation_guard&) = delete;
+
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __annotation_guard(basic_string& __str) : __str_(__str) {
+ __str_.__annotate_delete();
+ }
+
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 ~__annotation_guard() { __str_.__annotate_new(__str_.size()); }
+
+ basic_string& __str_;
+ };
+
template <size_type __a>
static _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type __align_it(size_type __s) _NOEXCEPT {
return (__s + (__a - 1)) & ~(__a - 1);
@@ -3357,7 +3370,16 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::re
if (__requested_capacity <= capacity())
return;
- __shrink_or_extend(__recommend(__requested_capacity));
+ __annotation_guard __g(*this);
+ auto __allocation = std::__allocate_at_least(__alloc_, __recommend(__requested_capacity) + 1);
+ auto __size = size();
+ __begin_lifetime(__allocation.ptr, __allocation.count);
+ traits_type::copy(std::__to_address(__allocation.ptr), data(), __size + 1);
+ if (__is_long())
+ __alloc_traits::deallocate(__alloc_, __get_long_pointer(), __get_long_cap());
+ __set_long_cap(__allocation.count);
+ __set_long_size(__size);
+ __set_long_pointer(__allocation.ptr);
}
template <class _CharT, class _Traits, class _Allocator>
@@ -3366,69 +3388,46 @@ inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocat
if (__target_capacity == capacity())
return;
- __shrink_or_extend(__target_capacity);
-}
+ _LIBCPP_ASSERT_INTERNAL(__is_long(), "Trying to shrink small string");
-template <class _CharT, class _Traits, class _Allocator>
-inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void
-basic_string<_CharT, _Traits, _Allocator>::__shrink_or_extend(size_type __target_capacity) {
- __annotate_delete();
- auto __guard = std::__make_scope_guard(__annotate_new_size(*this));
- size_type __cap = capacity();
- size_type __sz = size();
-
- pointer __new_data, __p;
- bool __was_long, __now_long;
+ // We're a long string and we're shrinking into the small buffer.
if (__fits_in_sso(__target_capacity)) {
- __was_long = true;
- __now_long = false;
- __new_data = __get_short_pointer();
- __p = __get_long_pointer();
- } else {
- if (__target_capacity > __cap) {
- // Extend
- // - called from reserve should propagate the exception thrown.
- auto __allocation = std::__allocate_at_least(__alloc_, __target_capacity + 1);
- __new_data = __allocation.ptr;
- __target_capacity = __allocation.count - 1;
- } else {
- // Shrink
- // - called from shrink_to_fit should not throw.
- // - called from reserve may throw but is not required to.
+ __annotation_guard __g(*this);
+ auto __ptr = __get_long_pointer();
+ auto __size = __get_long_size();
+ auto __cap = __get_long_cap();
+ traits_type::copy(std::__to_address(__get_short_pointer()), std::__to_address(__ptr), __size + 1);
+ __set_short_size(__size);
+ __alloc_traits::deallocate(__alloc_, __ptr, __cap);
+ return;
+ }
+
# if _LIBCPP_HAS_EXCEPTIONS
- try {
+ try {
# endif // _LIBCPP_HAS_EXCEPTIONS
- auto __allocation = std::__allocate_at_least(__alloc_, __target_capacity + 1);
-
- // The Standard mandates shrink_to_fit() does not increase the capacity.
- // With equal capacity keep the existing buffer. This avoids extra work
- // due to swapping the elements.
- if (__allocation.count - 1 > capacity()) {
- __alloc_traits::deallocate(__alloc_, __allocation.ptr, __allocation.count);
- return;
- }
- __new_data = __allocation.ptr;
- __target_capacity = __allocation.count - 1;
+ __annotation_guard __g(*this);
+ auto __size = size();
+ auto __allocation = std::__allocate_at_least(__alloc_, __target_capacity + 1);
+
+ // The Standard mandates shrink_to_fit() does not increase the capacity.
+ // With equal capacity keep the existing buffer. This avoids extra work
+ // due to swapping the elements.
+ if (__allocation.count - 1 > capacity()) {
+ __alloc_traits::deallocate(__alloc_, __allocation.ptr, __allocation.count);
+ return;
+ }
+
+ __begin_lifetime(__allocation.ptr, __allocation.count);
+ auto __ptr = __get_long_pointer();
+ traits_type::copy(std::__to_address(__allocation.ptr), std::__to_address(__ptr), __size + 1);
+ __alloc_traits::deallocate(__alloc_, __ptr, __get_long_cap());
+ __set_long_cap(__allocation.count);
+ __set_long_pointer(__allocation.ptr);
# if _LIBCPP_HAS_EXCEPTIONS
- } catch (...) {
- return;
- }
+ } catch (...) {
+ return;
+ }
# endif // _LIBCPP_HAS_EXCEPTIONS
- }
- __begin_lifetime(__new_data, __target_capacity + 1);
- __now_long = true;
- __was_long = __is_long();
- __p = __get_pointer();
- }
- traits_type::copy(std::__to_address(__new_data), std::__to_address(__p), size() + 1);
- if (__was_long)
- __alloc_traits::deallocate(__alloc_, __p, __cap + 1);
- if (__now_long) {
- __set_long_cap(__target_capacity + 1);
- __set_long_size(__sz);
- __set_long_pointer(__new_data);
- } else
- __set_short_size(__sz);
}
template <class _CharT, class _Traits, class _Allocator>
diff --git a/libcxx/test/std/strings/basic.string/string.capacity/reserve_size.pass.cpp b/libcxx/test/std/strings/basic.string/string.capacity/reserve_size.pass.cpp
index 6196fd6836e7d16..cb1e6b18ed450d6 100644
--- a/libcxx/test/std/strings/basic.string/string.capacity/reserve_size.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.capacity/reserve_size.pass.cpp
@@ -75,6 +75,15 @@ TEST_CONSTEXPR_CXX20 void test_string() {
test<S>(100, 50, 1000);
test<S>(100, 50, S::npos);
}
+
+ { // Check that growing twice works as expected
+ S str;
+ str.reserve(50);
+ assert(str.capacity() >= 50);
+ size_t old_cap = str.capacity();
+ str.reserve(str.capacity() + 1);
+ assert(str.capacity() > old_cap);
+ }
}
TEST_CONSTEXPR_CXX20 bool test() {
``````````
</details>
https://github.com/llvm/llvm-project/pull/125888
More information about the libcxx-commits
mailing list