Skip to content

[libc++] Fix throwing away smaller allocations in string::shrink_to_fit #115659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libcxx/include/string
Original file line number Diff line number Diff line change
Expand Up @@ -3393,7 +3393,7 @@ basic_string<_CharT, _Traits, _Allocator>::__shrink_or_extend(size_type __target
// 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) {
if (__allocation.count - 1 > capacity()) {
__alloc_traits::deallocate(__alloc_, __allocation.ptr, __allocation.count);
__annotate_new(__sz); // Undoes the __annotate_delete()
return;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20

// <string>

// void shrink_to_fit(); // constexpr since C++20

// Make sure we use an allocation returned by allocate_at_least if it is smaller than the current allocation
// even if it contains more bytes than we requested
Comment on lines +15 to +16
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this something we can check for all implementations? I don't think this test is libc++ specific?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implementations aren't required to do anything when shrink_to_fit() is called. void shrink_to_fit() {} is a valid implementation: http://eel.is/c++draft/basic.string#string.capacity-16


#include <cassert>
#include <string>

template <typename T>
struct oversizing_allocator {
using value_type = T;
oversizing_allocator() = default;
template <typename U>
oversizing_allocator(const oversizing_allocator<U>&) noexcept {}
std::allocation_result<T*> allocate_at_least(std::size_t n) {
++n;
return {static_cast<T*>(::operator new(n * sizeof(T))), n};
}
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==(oversizing_allocator<T>, oversizing_allocator<U>) {
return true;
}

void test_oversizing_allocator() {
std::basic_string<char, std::char_traits<char>, oversizing_allocator<char>> s{
"String does not fit in the internal buffer and is a bit longer"};
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);
}

int main(int, char**) {
test_oversizing_allocator();

return 0;
}
Loading