Skip to content

Commit

Permalink
[libc++][string] Fixes shrink_to_fit. (#97961)
Browse files Browse the repository at this point in the history
Summary:
This ensures that shrink_to_fit does not increase the allocated size.

Partly addresses #95161

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251164
  • Loading branch information
mordante authored and yuxuanchen1997 committed Jul 25, 2024
1 parent 54fc532 commit 14d71b6
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 3 deletions.
17 changes: 14 additions & 3 deletions libcxx/include/string
Original file line number Diff line number Diff line change
Expand Up @@ -3358,23 +3358,34 @@ basic_string<_CharT, _Traits, _Allocator>::__shrink_or_extend(size_type __target
__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.
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
try {
#endif // _LIBCPP_HAS_NO_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 > __target_capacity) {
__alloc_traits::deallocate(__alloc(), __allocation.ptr, __allocation.count);
__annotate_new(__sz); // Undoes the __annotate_delete()
return;
}
__new_data = __allocation.ptr;
__target_capacity = __allocation.count - 1;
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
} catch (...) {
return;
}
#else // _LIBCPP_HAS_NO_EXCEPTIONS
if (__new_data == nullptr)
return;
#endif // _LIBCPP_HAS_NO_EXCEPTIONS
}
__begin_lifetime(__new_data, __target_capacity + 1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,49 @@ TEST_CONSTEXPR_CXX20 bool test() {
return true;
}

#if TEST_STD_VER >= 23
std::size_t min_bytes = 1000;

template <typename T>
struct increasing_allocator {
using value_type = T;
increasing_allocator() = default;
template <typename U>
increasing_allocator(const increasing_allocator<U>&) noexcept {}
std::allocation_result<T*> allocate_at_least(std::size_t n) {
std::size_t allocation_amount = n * sizeof(T);
if (allocation_amount < min_bytes)
allocation_amount = min_bytes;
min_bytes += 1000;
return {static_cast<T*>(::operator new(allocation_amount)), allocation_amount / sizeof(T)};
}
T* allocate(std::size_t n) { return allocate_at_least(n).ptr; }
void deallocate(T* p, std::size_t) noexcept { ::operator delete(static_cast<void*>(p)); }
};

template <typename T, typename U>
bool operator==(increasing_allocator<T>, increasing_allocator<U>) {
return true;
}

// https://github.com/llvm/llvm-project/issues/95161
void test_increasing_allocator() {
std::basic_string<char, std::char_traits<char>, increasing_allocator<char>> s{
"String does not fit in the internal buffer"};
std::size_t capacity = s.capacity();
std::size_t size = s.size();
s.shrink_to_fit();
assert(s.capacity() <= capacity);
assert(s.size() == size);
LIBCPP_ASSERT(is_string_asan_correct(s));
}
#endif // TEST_STD_VER >= 23

int main(int, char**) {
test();
#if TEST_STD_VER >= 23
test_increasing_allocator();
#endif
#if TEST_STD_VER > 17
static_assert(test());
#endif
Expand Down

0 comments on commit 14d71b6

Please sign in to comment.