[libc++] [P0966] [C++20] Fix bug PR45368 by correctly implementing P0966: string::reserve should not shrink.

[curdeius]

Summary

This patch fixes the implementation as well as the tests that didn't actually test the wanted behaviour.
You'll find all the details in the bug report.
It adds as well deprecation warning for reserve() (without argument) and adds a test.

http://wg21.link/P0966R1
https://bugs.llvm.org/show_bug.cgi?id=45368
https://reviews.llvm.org/D54992

[curdeius]

Oops. Remove spurious _NOEXCEPT.

[curdeius]

FYI, the code handling reallocation etc. that was previously in reserve hasn't been modified, only moved to __shrink_or_extend for reuse.

[curdeius]

Again, this change needs the use of C++20 when building libc++, because there are instantiations for char.

[curdeius]

Rebase on top of D91691.

[curdeius]

Rebase to rerun tests

[curdeius]

Ready for review!
On BuildKite there are some failures on Apple, but 3/4 concern libcxx/modules/stds_include.sh.cpp which seems (very) flaky.
The fourth one (on Apple back-deployment macosx10.9) looks severe, but I suspect that it doesn't compile src/ with -std=c++20 flag, and so uses instantiation of basic_string<char> as of C++17.

[curdeius]

I think I understood the problem with failing Apple back-deployment macosx10.9... I took a look at libcxx/utils/ci/run-buildbot that it invokes. It's not going to work in pre-merge test, because it uses libc++ binaries that it downloads from http://lab.llvm.org:8080/roots/libcxx-roots.tar.gz (obviously not compiled with changes from this patch).
@ldionne, any idea how this should be addressed?

[ldionne]

In D91778#2410947, @curdeius wrote:

I think I understood the problem with failing Apple back-deployment macosx10.9... I took a look at libcxx/utils/ci/run-buildbot that it invokes. It's not going to work in pre-merge test, because it uses libc++ binaries that it downloads from http://lab.llvm.org:8080/roots/libcxx-roots.tar.gz (obviously not compiled with changes from this patch).
@ldionne, any idea how this should be addressed?

The usual way of handling these test failures caused by a bug being fixed in the dylib is to mark the tests as XFAIL based on the system library being used. The following should work:

// This test relies on https://llvm.org/PR45368 being fixed, which isn't in
// older Apple dylibs
// 
// XFAIL: with_system_cxx_lib=macosx10.15
// XFAIL: with_system_cxx_lib=macosx10.14
// XFAIL: with_system_cxx_lib=macosx10.13
// XFAIL: with_system_cxx_lib=macosx10.12
// XFAIL: with_system_cxx_lib=macosx10.11
// XFAIL: with_system_cxx_lib=macosx10.10
// XFAIL: with_system_cxx_lib=macosx10.9

Regarding the other failures, they do seem flaky indeed. We need to figure out what's wrong with those modules related tests.

libcxx/include/string

3268

I'd be in favour of renaming this to something like __requested_capacity.

3289

Using something like __target_capacity here would convey more IMO.

libcxx/test/std/strings/basic.string/string.capacity/reserve_size.pass.cpp

29

Is there a reason why you're not checking invariants anymore?

libcxx/www/cxx2a_status.html

266 ↗

(On Diff #306971)

Typo!

[curdeius]

• Fix typo and use permalink.
• Check invariants.
• Rename.
• Mark test with XFAIL for macOS.

[curdeius]

libcxx/test/std/strings/basic.string/string.capacity/reserve_size.pass.cpp

29

I just moved the test for reserve() (without size argument) to the file reserve.pass.cpp and it still checks invariants.
This test (for reserve(size)) didn't check invariants, but I can add it in this test, that seems judicious.

[rsmith]

libcxx/include/string

3273–3276

This #if doesn't seem to work: because reserve(size_type) is not inline, we always use the version from the libc++ library, which means we get the C++17 behavior if the library was built in C++17 mode, and we get the C++20 behavior if the library was built in C++20 mode, regardless of the standard mode used by the compilation using libc++.

[ldionne]

libcxx/include/string

3273–3276

Oh crap, that's right. Nice catch. But I'm not seeing a way to fix this except to make this function inline, and to keep a definition in the dylib for backwards compatibility (but nobody except already-compiled code would call it). Do you see other ways?

[rsmith]

libcxx/include/string

3273–3276

If we could somehow split it into two functions for C++17 and C++20 calls, that might work.

In principle, if we could rely on concepts in C++20 mode, then I think that making the "never shrinks" C++20 overload be requires true + inline and leaving the C++17 version unconstrained and non-inline should do the trick with no ABI impact. But that doesn't compile in Clang and crashes GCC: https://godbolt.org/z/3WEKMa -- looks like neither compiler properly mangles constrained functions yet.

So how about something like this: https://godbolt.org/z/M69EP1

[rsmith]

libcxx/include/string

3273–3276

Looks like this was landed without fixing this problem. See https://github.com/llvm/llvm-project/issues/53170

libcxx/docs/Cxx2aStatus.rst

 .. csv-table::
    :file: Cxx2aStatusPaperStatus.csv
    :widths: auto
 
 .. note::
 
    .. [#note-P0202] P0202: The missing bits in P0202 are in ``copy`` and ``copy_backwards`` (and the ones that call them: ``copy_n``, ``set_union``, ``set_difference``, and ``set_symmetric_difference``). This is because the first two algorithms have specializations that call ``memmove`` which is not constexpr. See `Bug 25165 <https://bugs.llvm.org/show_bug.cgi?id=25165>`__
    .. [#note-P0600] P0600: The missing bits in P0600 are in |sect|\ [mem.res.class], |sect|\ [mem.poly.allocator.class], and |sect|\ [container.node.overview].
+   .. [#note-P0966] P0966: It was previously erroneously marked as complete in version 8.0. See `bug 45368 <https://llvm.org/PR45368>`__.
 
    .. [#note-P0619] P0619: Only ``std::allocator`` part is implemented.
 
 
 .. _issues-status-cxx2a:
 
 Library Working Group Issues Status
 ====================================
 
 .. csv-table::
    :file: Cxx2aStatusIssuesStatus.csv
    :widths: auto
 
 Last Updated: 24-Nov-2020

libcxx/docs/Cxx2aStatusPaperStatus.csv

 "`P0122R7 <https://wg21.link/P0122R7>`__","LWG","<span>","Jacksonville","|Complete|","7.0"
 "`P0355R7 <https://wg21.link/P0355R7>`__","LWG","Extending chrono to Calendars and Time Zones","Jacksonville","|In Progress|",""
 "`P0551R3 <https://wg21.link/P0551R3>`__","LWG","Thou Shalt Not Specialize ``std``\  Function Templates!","Jacksonville","|Complete|","11.0"
 "`P0753R2 <https://wg21.link/P0753R2>`__","LWG","Manipulators for C++ Synchronized Buffered Ostream","Jacksonville","",""
 "`P0754R2 <https://wg21.link/P0754R2>`__","LWG","<version>","Jacksonville","|Complete|","7.0"
 "`P0809R0 <https://wg21.link/P0809R0>`__","LWG","Comparing Unordered Containers","Jacksonville","",""
 "`P0858R0 <https://wg21.link/P0858R0>`__","LWG","Constexpr iterator requirements","Jacksonville","",""
 "`P0905R1 <https://wg21.link/P0905R1>`__","CWG","Symmetry for spaceship","Jacksonville","",""
-"`P0966R1 <https://wg21.link/P0966R1>`__","LWG","``string::reserve``\  Should Not Shrink","Jacksonville","|Complete|","8.0"
+"`P0966R1 <https://wg21.link/P0966R1>`__","LWG","``string::reserve``\  Should Not Shrink","Jacksonville","|Complete| [#note-P0966]_","12.0"
 "","","","","",""
 "`P0019R8 <https://wg21.link/P0019R8>`__","LWG","Atomic Ref","Rapperswil","",""
 "`P0458R2 <https://wg21.link/P0458R2>`__","LWG","Checking for Existence of an Element in Associative Containers","Rapperswil","|Complete|",""
 "`P0475R1 <https://wg21.link/P0475R1>`__","LWG","LWG 2511: guaranteed copy elision for piecewise construction","Rapperswil","",""
 "`P0476R2 <https://wg21.link/P0476R2>`__","LWG","Bit-casting object representations","Rapperswil","",""
 "`P0528R3 <https://wg21.link/P0528R3>`__","CWG","The Curious Case of Padding Bits, Featuring Atomic Compare-and-Exchange","Rapperswil","",""
 "`P0542R5 <https://wg21.link/P0542R5>`__","CWG","Support for contract based programming in C++","Rapperswil","",""
 "`P0556R3 <https://wg21.link/P0556R3>`__","LWG","Integral power-of-2 operations","Rapperswil","|Complete|","9.0"

libcxx/include/__config

 #endif
 
 #if _LIBCPP_STD_VER >= 17
 #  define _LIBCPP_DEPRECATED_IN_CXX17 _LIBCPP_DEPRECATED
 #else
 #  define _LIBCPP_DEPRECATED_IN_CXX17
 #endif
 
+#if _LIBCPP_STD_VER > 17
+#  define _LIBCPP_DEPRECATED_IN_CXX20 _LIBCPP_DEPRECATED
+#else
+#  define _LIBCPP_DEPRECATED_IN_CXX20
+#endif
+
 // Macros to enter and leave a state where deprecation warnings are suppressed.
 #if !defined(_LIBCPP_SUPPRESS_DEPRECATED_PUSH) && \
     (defined(_LIBCPP_COMPILER_CLANG) || defined(_LIBCPP_COMPILER_GCC))
 #  define _LIBCPP_SUPPRESS_DEPRECATED_PUSH \
     _Pragma("GCC diagnostic push") \
     _Pragma("GCC diagnostic ignored \"-Wdeprecated\"")
 #  define _LIBCPP_SUPPRESS_DEPRECATED_POP \
     _Pragma("GCC diagnostic pop")

libcxx/include/string

// types:
     size_type size() const noexcept;
     size_type length() const noexcept;
     size_type max_size() const noexcept;
     size_type capacity() const noexcept;
 
     void resize(size_type n, value_type c);
     void resize(size_type n);
 
-    void reserve(size_type res_arg = 0);
+    void reserve(size_type res_arg);
+    void reserve(); // deprecated in C++20
     void shrink_to_fit();
     void clear() noexcept;
     bool empty() const noexcept;
 
     const_reference operator[](size_type pos) const;
     reference       operator[](size_type pos);
 
     const_reference at(size_type n) const;
#endif  // _LIBCPP_DEBUG_LEVEL == 2
     _LIBCPP_INLINE_VISIBILITY size_type max_size() const _NOEXCEPT;
     _LIBCPP_INLINE_VISIBILITY size_type capacity() const _NOEXCEPT
         {return (__is_long() ? __get_long_cap()
                              : static_cast<size_type>(__min_cap)) - 1;}
 
     void resize(size_type __n, value_type __c);
     _LIBCPP_INLINE_VISIBILITY void resize(size_type __n) {resize(__n, value_type());}
 
-    void reserve(size_type __res_arg);
+    void reserve(size_type __requested_capacity);
     _LIBCPP_INLINE_VISIBILITY void __resize_default_init(size_type __n);
 
+    _LIBCPP_DEPRECATED_IN_CXX20 _LIBCPP_INLINE_VISIBILITY
+    void reserve() _NOEXCEPT {shrink_to_fit();}
     _LIBCPP_INLINE_VISIBILITY
-    void reserve() _NOEXCEPT {reserve(0);}
-    _LIBCPP_INLINE_VISIBILITY
-    void shrink_to_fit() _NOEXCEPT {reserve();}
+    void shrink_to_fit() _NOEXCEPT;
     _LIBCPP_INLINE_VISIBILITY
     void clear() _NOEXCEPT;
     _LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_INLINE_VISIBILITY
     bool empty() const _NOEXCEPT {return size() == 0;}
 
     _LIBCPP_INLINE_VISIBILITY const_reference operator[](size_type __pos) const _NOEXCEPT;
     _LIBCPP_INLINE_VISIBILITY reference       operator[](size_type __pos)       _NOEXCEPT;
 
#if _LIBCPP_STD_VER > 17
     bool ends_with(const value_type* __s) const _NOEXCEPT
     { return ends_with(__self_view(__s)); }
 #endif
 
     _LIBCPP_INLINE_VISIBILITY bool __invariants() const;
 
     _LIBCPP_INLINE_VISIBILITY void __clear_and_shrink() _NOEXCEPT;
 
+    _LIBCPP_INLINE_VISIBILITY void __shrink_or_extend(size_type __target_capacity);
+
     _LIBCPP_INLINE_VISIBILITY
     bool __is_long() const _NOEXCEPT
         {return bool(__r_.first().__s.__size_ & __short_mask);}
 
 #if _LIBCPP_DEBUG_LEVEL == 2
 
     bool __dereferenceable(const const_iterator* __i) const;
     bool __decrementable(const const_iterator* __i) const;
#ifdef _LIBCPP_BIG_ENDIAN
     return (__m <= ~__long_mask ? __m : __m/2) - __alignment;
 #else
     return __m - __alignment;
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 void
-basic_string<_CharT, _Traits, _Allocator>::reserve(size_type __res_arg)
+basic_string<_CharT, _Traits, _Allocator>::reserve(size_type __requested_capacity)
 {
-    if (__res_arg > max_size())
+    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
+
+    size_type __target_capacity = _VSTD::max(__requested_capacity, size());
+    __target_capacity = __recommend(__target_capacity);
+    if (__target_capacity == capacity()) return;
+
+    __shrink_or_extend(__target_capacity);
+}
+
+template <class _CharT, class _Traits, class _Allocator>
+void
+basic_string<_CharT, _Traits, _Allocator>::shrink_to_fit() _NOEXCEPT
+{
+    size_type __target_capacity = __recommend(size());
+    if (__target_capacity == capacity()) return;
+
+    __shrink_or_extend(__target_capacity);
+}
+
+template <class _CharT, class _Traits, class _Allocator>
+void
+basic_string<_CharT, _Traits, _Allocator>::__shrink_or_extend(size_type __target_capacity)
+{
     size_type __cap = capacity();
     size_type __sz = size();
-    __res_arg = _VSTD::max(__res_arg, __sz);
-    __res_arg = __recommend(__res_arg);
-    if (__res_arg != __cap)
-    {
-        pointer __new_data, __p;
-        bool __was_long, __now_long;
-        if (__res_arg == __min_cap - 1)
-        {
-            __was_long = true;
-            __now_long = false;
-            __new_data = __get_short_pointer();
-            __p = __get_long_pointer();
-        }
-        else
-        {
-            if (__res_arg > __cap)
-                __new_data = __alloc_traits::allocate(__alloc(), __res_arg+1);
-            else
-            {
-            #ifndef _LIBCPP_NO_EXCEPTIONS
-                try
-                {
-            #endif  // _LIBCPP_NO_EXCEPTIONS
-                    __new_data = __alloc_traits::allocate(__alloc(), __res_arg+1);
-            #ifndef _LIBCPP_NO_EXCEPTIONS
-                }
-                catch (...)
-                {
-                    return;
-                }
-            #else  // _LIBCPP_NO_EXCEPTIONS
-                if (__new_data == nullptr)
-                    return;
-            #endif  // _LIBCPP_NO_EXCEPTIONS
-            }
-            __now_long = true;
-            __was_long = __is_long();
-            __p = __get_pointer();
-        }
-        traits_type::copy(_VSTD::__to_address(__new_data),
-                          _VSTD::__to_address(__p), size()+1);
-        if (__was_long)
-            __alloc_traits::deallocate(__alloc(), __p, __cap+1);
-        if (__now_long)
-        {
-            __set_long_cap(__res_arg+1);
-            __set_long_size(__sz);
-            __set_long_pointer(__new_data);
-        }
-        else
-            __set_short_size(__sz);
-        __invalidate_all_iterators();
-    }
-}
+
+    if (__target_capacity == __min_cap - 1)
+        if (__target_capacity > __cap)
+            __new_data = __alloc_traits::allocate(__alloc(), __target_capacity+1);
+                __new_data = __alloc_traits::allocate(__alloc(), __target_capacity+1);
+        __set_long_cap(__target_capacity+1);
 
 template <class _CharT, class _Traits, class _Allocator>
 inline
 typename basic_string<_CharT, _Traits, _Allocator>::const_reference
 basic_string<_CharT, _Traits, _Allocator>::operator[](size_type __pos) const _NOEXCEPT
 {
     _LIBCPP_ASSERT(__pos <= size(), "string index out of bounds");
     return *(data() + __pos);

libcxx/test/libcxx/strings/basic.string/string.capacity/reserve.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// 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;
+}

libcxx/test/std/strings/basic.string/string.capacity/reserve.deprecated_in_cxx20.verify.cpp

+//===----------------------------------------------------------------------===//
+//
+// 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
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+#include <string>
+
+int main(int, char**)
+{
+    std::string s;
+    s.reserve(); // expected-warning {{'reserve' is deprecated}}
+    return 0;
+}

libcxx/test/std/strings/basic.string/string.capacity/reserve.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // 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>
 
-// Split into two calls for C++20
-// void reserve();
-// void reserve(size_type res_arg);
+// 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(S s)
+test(typename S::size_type min_cap, typename S::size_type erased_index)
 {
+    S s(min_cap, 'a');
+    s.erase(erased_index);
+    assert(s.size() == erased_index);
+    assert(s.capacity() >= min_cap); // Check that we really have at least this capacity.
+
     typename S::size_type old_cap = s.capacity();
     S s0 = s;
     s.reserve();
     LIBCPP_ASSERT(s.__invariants());
     assert(s == s0);
     assert(s.capacity() <= old_cap);
     assert(s.capacity() >= s.size());
 }
 
-template <class S>
-void
-test(S s, typename S::size_type res_arg)
-{
-    typename S::size_type old_cap = s.capacity();
-    ((void)old_cap); // Prevent unused warning
-    S s0 = s;
-    if (res_arg <= s.max_size())
-    {
-        s.reserve(res_arg);
-        assert(s == s0);
-        assert(s.capacity() >= res_arg);
-        assert(s.capacity() >= s.size());
-#if TEST_STD_VER > 17
-        assert(s.capacity() >= old_cap); // resize never shrinks as of P0966
-#endif
-    }
-#ifndef TEST_HAS_NO_EXCEPTIONS
-    else
-    {
-        try
-        {
-            s.reserve(res_arg);
-            assert(false);
-        }
-        catch (std::length_error&)
-        {
-            assert(res_arg > s.max_size());
-        }
-    }
-#endif
-}
-
 int main(int, char**)
 {
     {
     typedef std::string S;
     {
-    S s;
-    test(s);
-
-    s.assign(10, 'a');
-    s.erase(5);
-    test(s);
-
-    s.assign(100, 'a');
-    s.erase(50);
-    test(s);
-    }
-    {
-    S s;
-    test(s, 5);
-    test(s, 10);
-    test(s, 50);
-    }
-    {
-    S s(100, 'a');
-    s.erase(50);
-    test(s, 5);
-    test(s, 10);
-    test(s, 50);
-    test(s, 100);
-    test(s, 1000);
-    test(s, S::npos);
+    test<S>(0, 0);
+    test<S>(10, 5);
+    test<S>(100, 50);
     }
     }
 #if TEST_STD_VER >= 11
     {
     typedef std::basic_string<char, std::char_traits<char>, min_allocator<char>> S;
     {
-    S s;
-    test(s);
-
-    s.assign(10, 'a');
-    s.erase(5);
-    test(s);
-
-    s.assign(100, 'a');
-    s.erase(50);
-    test(s);
-    }
-    {
-    S s;
-    test(s, 5);
-    test(s, 10);
-    test(s, 50);
-    }
-    {
-    S s(100, 'a');
-    s.erase(50);
-    test(s, 5);
-    test(s, 10);
-    test(s, 50);
-    test(s, 100);
-    test(s, 1000);
-    test(s, S::npos);
+    test<S>(0, 0);
+    test<S>(10, 5);
+    test<S>(100, 50);
     }
     }
 #endif
 
   return 0;
 }

libcxx/test/std/strings/basic.string/string.capacity/reserve_size.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // 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>
 
-// Split into two calls for C++20
-// void reserve();
 // void reserve(size_type res_arg);
 
+// This test relies on https://llvm.org/PR45368 being fixed, which isn't in
+// older Apple dylibs
+//
+// XFAIL: with_system_cxx_lib=macosx10.15
+// XFAIL: with_system_cxx_lib=macosx10.14
+// XFAIL: with_system_cxx_lib=macosx10.13
+// XFAIL: with_system_cxx_lib=macosx10.12
+// XFAIL: with_system_cxx_lib=macosx10.11
+// XFAIL: with_system_cxx_lib=macosx10.10
+// XFAIL: with_system_cxx_lib=macosx10.9
+
 #include <string>
 #include <stdexcept>
 #include <cassert>
 
 #include "test_macros.h"
 #include "min_allocator.h"
 
 template <class S>
 void
-test(S s)
+test(typename S::size_type min_cap, typename S::size_type erased_index, typename S::size_type res_arg)
 {
-    typename S::size_type old_cap = s.capacity();
-    S s0 = s;
-    s.reserve();
-    LIBCPP_ASSERT(s.__invariants());
-    assert(s == s0);
-    assert(s.capacity() <= old_cap);
-    assert(s.capacity() >= s.size());
-}
+    S s(min_cap, 'a');
+    s.erase(erased_index);
+    assert(s.size() == erased_index);
+    assert(s.capacity() >= min_cap); // Check that we really have at least this capacity.
 
-template <class S>
-void
-test(S s, typename S::size_type res_arg)
-{
+#if TEST_STD_VER > 17
     typename S::size_type old_cap = s.capacity();
-    ((void)old_cap); // Prevent unused warning
+#endif
     S s0 = s;
     if (res_arg <= s.max_size())
     {
         s.reserve(res_arg);
+        LIBCPP_ASSERT(s.__invariants());
         assert(s == s0);
         assert(s.capacity() >= res_arg);
         assert(s.capacity() >= s.size());
 #if TEST_STD_VER > 17
-        assert(s.capacity() >= old_cap); // resize never shrinks as of P0966
+        assert(s.capacity() >= old_cap); // reserve never shrinks as of P0966 (C++20)
 #endif
     }
 #ifndef TEST_HAS_NO_EXCEPTIONS
     else
     {
         try
         {
             s.reserve(res_arg);
+            LIBCPP_ASSERT(s.__invariants());
             assert(false);
         }
         catch (std::length_error&)
         {
             assert(res_arg > s.max_size());
         }
     }
 #endif
 }
 
 int main(int, char**)
 {
     {
     typedef std::string S;
     {
-    S s;
-    test(s);
-
-    s.assign(10, 'a');
-    s.erase(5);
-    test(s);
-
-    s.assign(100, 'a');
-    s.erase(50);
-    test(s);
-    }
-    {
-    S s;
-    test(s, 5);
-    test(s, 10);
-    test(s, 50);
-    }
-    {
-    S s(100, 'a');
-    s.erase(50);
-    test(s, 5);
-    test(s, 10);
-    test(s, 50);
-    test(s, 100);
-    test(s, 1000);
-    test(s, S::npos);
+    test<S>(0, 0, 5);
+    test<S>(0, 0, 10);
+    test<S>(0, 0, 50);
+    }
+    {
+    test<S>(100, 50, 5);
+    test<S>(100, 50, 10);
+    test<S>(100, 50, 50);
+    test<S>(100, 50, 100);
+    test<S>(100, 50, 1000);
+    test<S>(100, 50, S::npos);
     }
     }
 #if TEST_STD_VER >= 11
     {
     typedef std::basic_string<char, std::char_traits<char>, min_allocator<char>> S;
     {
-    S s;
-    test(s);
-
-    s.assign(10, 'a');
-    s.erase(5);
-    test(s);
-
-    s.assign(100, 'a');
-    s.erase(50);
-    test(s);
-    }
-    {
-    S s;
-    test(s, 5);
-    test(s, 10);
-    test(s, 50);
-    }
-    {
-    S s(100, 'a');
-    s.erase(50);
-    test(s, 5);
-    test(s, 10);
-    test(s, 50);
-    test(s, 100);
-    test(s, 1000);
-    test(s, S::npos);
+    test<S>(0, 0, 5);
+    test<S>(0, 0, 10);
+    test<S>(0, 0, 50);
+    }
+    {
+    test<S>(100, 50, 5);
+    test<S>(100, 50, 10);
+    test<S>(100, 50, 50);
+    test<S>(100, 50, 100);
+    test<S>(100, 50, 1000);
+    test<S>(100, 50, S::npos);
     }
     }
 #endif
 
   return 0;
 }