[libcxx-commits] [libcxx] 0407ab4 - [libc++] Make sure basic_string::reserve(n) never shrinks in all Standard modes
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Jan 24 12:43:20 PST 2022
Author: Louis Dionne
Date: 2022-01-24T15:43:13-05:00
New Revision: 0407ab4114dbdbd1845df712639bdbc84ec6df2c
URL: https://github.com/llvm/llvm-project/commit/0407ab4114dbdbd1845df712639bdbc84ec6df2c
DIFF: https://github.com/llvm/llvm-project/commit/0407ab4114dbdbd1845df712639bdbc84ec6df2c.diff
LOG: [libc++] Make sure basic_string::reserve(n) never shrinks in all Standard modes
Since basic_string::reserve(n) is instantiated in the shared library but also
available to the compiler for inlining, its definition should not depend on
things like the Standard mode in use. Indeed, that flag may not match between
how the shared library is compiled and how users are compiling their own code,
resulting in ODR violations.
However, note that we retain the behavior of basic_string::reserve() to
shrink the string for backwards compatibility reasons. While it would
technically be conforming to not shrink, we believe user expectation is
for it to shrink, and so existing code might have been written based on
that assumption. We prefer to not break such code, even though that makes
basic_string::reserve() and basic_string::reserve(0) not equivalent anymore.
Fixes llvm-project#53170
Differential Revision: https://reviews.llvm.org/D117332
Added:
libcxx/test/libcxx/strings/basic.string/string.capacity/PR53170.pass.cpp
Modified:
libcxx/docs/ReleaseNotes.rst
libcxx/include/string
Removed:
libcxx/test/libcxx/strings/basic.string/string.capacity/reserve.pass.cpp
################################################################################
diff --git a/libcxx/docs/ReleaseNotes.rst b/libcxx/docs/ReleaseNotes.rst
index b35c8a3329515..7210e1971b10f 100644
--- a/libcxx/docs/ReleaseNotes.rst
+++ b/libcxx/docs/ReleaseNotes.rst
@@ -115,6 +115,18 @@ API Changes
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
+
diff erent 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
index b2eef646f9827..6f22f02afa0ba 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -3265,10 +3265,11 @@ basic_string<_CharT, _Traits, _Allocator>::reserve(size_type __requested_capacit
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
index 0000000000000..1be3dc0af2675
--- /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
+//
+//===----------------------------------------------------------------------===//
+
+// <string>
+
+// 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 <string>
+#include <stdexcept>
+#include <cassert>
+
+#include "test_macros.h"
+#include "min_allocator.h"
+
+template <class S>
+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<std::string>();
+
+#if TEST_STD_VER >= 11
+ test<std::basic_string<char, std::char_traits<char>, min_allocator<char> > >();
+#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
index 358f51fd6e4cf..0000000000000
--- 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
-//
-//===----------------------------------------------------------------------===//
-
-// <string>
-
-// void reserve(); // Deprecated in C++20.
-
-// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
-
-#include <string>
-#include <stdexcept>
-#include <cassert>
-
-#include "test_macros.h"
-#include "min_allocator.h"
-
-template <class S>
-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<S>();
- }
-#if TEST_STD_VER >= 11
- {
- typedef min_allocator<char> A;
- typedef std::basic_string<char, std::char_traits<char>, A> S;
- test<S>();
- }
-#endif
-
- return 0;
-}
More information about the libcxx-commits
mailing list