diff --git a/libcxx/docs/ReleaseNotes.rst b/libcxx/docs/ReleaseNotes.rst --- a/libcxx/docs/ReleaseNotes.rst +++ b/libcxx/docs/ReleaseNotes.rst @@ -115,6 +115,18 @@ You must now explicitly initialize with a ``chrono::month`` and ``chrono::weekday_indexed`` instead of "meh, whenever". +- C++20 requires that ``std::basic_string::reserve(n)`` never reduce the capacity + of the string. (For that, use ``shrink_to_fit()``.) Prior to this release, libc++'s + ``std::basic_string::reserve(n)`` could reduce capacity in C++17 and before, but + not in C++20 and later. This caused ODR violations when mixing code compiled under + different Standard modes. After this change, libc++'s ``std::basic_string::reserve(n)`` + never reduces capacity, even in C++17 and before. + C++20 deprecates the zero-argument overload of ``std::basic_string::reserve()``, + but specifically permits it to reduce capacity. To avoid breaking existing code + assuming that ``std::basic_string::reserve()`` will shrink, libc++ maintains + the behavior to shrink, even though that makes ``std::basic_string::reserve()`` not + a synonym for ``std::basic_string::reserve(0)`` in any Standard mode anymore. + ABI Changes ----------- diff --git a/libcxx/include/string b/libcxx/include/string --- a/libcxx/include/string +++ b/libcxx/include/string @@ -3265,10 +3265,11 @@ if (__requested_capacity > max_size()) this->__throw_length_error(); -#if _LIBCPP_STD_VER > 17 - // Reserve never shrinks as of C++20. - if (__requested_capacity <= capacity()) return; -#endif + // Make sure reserve(n) never shrinks. This is technically only required in C++20 + // and later (since P0966R1), however we provide consistent behavior in all Standard + // modes because this function is instantiated in the shared library. + if (__requested_capacity <= capacity()) + return; size_type __target_capacity = _VSTD::max(__requested_capacity, size()); __target_capacity = __recommend(__target_capacity); diff --git a/libcxx/test/libcxx/strings/basic.string/string.capacity/PR53170.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.capacity/PR53170.pass.cpp new file mode 100644 --- /dev/null +++ b/libcxx/test/libcxx/strings/basic.string/string.capacity/PR53170.pass.cpp @@ -0,0 +1,79 @@ +//===----------------------------------------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +// + +// void reserve(); // Deprecated in C++20. +// void reserve(size_type); + +// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS + +// This test ensures that libc++ implements https://wg21.link/P0966R1 (reserve never shrinks) +// even before C++20. This is required in order to avoid ODR violations because basic_string::reserve(size) +// is compiled into the shared library. Hence, it needs to have the same definition in all Standard modes. +// +// However, note that reserve() does shrink, and it does so in all Standard modes. +// +// Reported as https://llvm.org/PR53170. + +// reserve(n) used to shrink the string until https://llvm.org/D117332 was shipped. +// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11|12|13|14|15}} +// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx{{11|12}} + +#include +#include +#include + +#include "test_macros.h" +#include "min_allocator.h" + +template +void test() { + // Test that a call to reserve() does shrink the string. + { + S s(1000, 'a'); + typename S::size_type old_cap = s.capacity(); + s.resize(20); + assert(s.capacity() == old_cap); + + s.reserve(); + assert(s.capacity() < old_cap); + } + + // Test that a call to reserve(smaller-than-capacity) never shrinks the string. + { + S s(1000, 'a'); + typename S::size_type old_cap = s.capacity(); + s.resize(20); + assert(s.capacity() == old_cap); + + s.reserve(10); + assert(s.capacity() == old_cap); + } + + // In particular, test that reserve(0) does NOT shrink the string. + { + S s(1000, 'a'); + typename S::size_type old_cap = s.capacity(); + s.resize(20); + assert(s.capacity() == old_cap); + + s.reserve(0); + assert(s.capacity() == old_cap); + } +} + +int main(int, char**) { + test(); + +#if TEST_STD_VER >= 11 + test, min_allocator > >(); +#endif + + return 0; +} diff --git a/libcxx/test/libcxx/strings/basic.string/string.capacity/reserve.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.capacity/reserve.pass.cpp deleted file mode 100644 --- a/libcxx/test/libcxx/strings/basic.string/string.capacity/reserve.pass.cpp +++ /dev/null @@ -1,50 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// 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 -// -//===----------------------------------------------------------------------===// - -// - -// void reserve(); // Deprecated in C++20. - -// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS - -#include -#include -#include - -#include "test_macros.h" -#include "min_allocator.h" - -template -void -test() -{ - // Tests that a call to reserve() on a long string is equivalent to shrink_to_fit(). - S s(1000, 'a'); - typename S::size_type old_cap = s.capacity(); - s.resize(20); - assert(s.capacity() == old_cap); - s.reserve(); - assert(s.capacity() < old_cap); -} - -int main(int, char**) -{ - { - typedef std::string S; - test(); - } -#if TEST_STD_VER >= 11 - { - typedef min_allocator A; - typedef std::basic_string, A> S; - test(); - } -#endif - - return 0; -}