From 140cf935cd118f7208b7c3826a8b9d50936242f0 Mon Sep 17 00:00:00 2001 From: Andrew Luo Date: Thu, 6 Aug 2020 19:35:43 +0100 Subject: [PATCH] libstdc++: Implement P0966 std::string::reserve should not shrink Remove ability for reserve(n) to reduce a string's capacity. Add a new reserve() overload that makes a shrink-to-fit request, and make shrink_to_fit() use that. libstdc++-v3/ChangeLog: 2020-07-30 Andrew Luo Jonathan Wakely * config/abi/pre/gnu.ver (GLIBCXX_3.4): Use less greedy patterns for basic_string members. (GLIBCXX_3.4.29): Export new basic_string::reserve symbols. * doc/xml/manual/status_cxx2020.xml: Update P0966 status. * include/bits/basic_string.h (shrink_to_fit()): Call reserve(). (reserve(size_type)): Remove default argument. (reserve()): Declare new overload. [!_GLIBCXX_USE_CXX11_ABI] (shrink_to_fit, reserve): Likewise. * include/bits/basic_string.tcc (reserve(size_type)): Remove support for shrinking capacity. (reserve()): Perform shrink-to-fit operation. [!_GLIBCXX_USE_CXX11_ABI] (reserve): Likewise. * testsuite/21_strings/basic_string/capacity/1.cc: Adjust to reflect new behavior. * testsuite/21_strings/basic_string/capacity/char/1.cc: Likewise. * testsuite/21_strings/basic_string/capacity/char/18654.cc: Likewise. * testsuite/21_strings/basic_string/capacity/char/2.cc: Likewise. * testsuite/21_strings/basic_string/capacity/wchar_t/1.cc: Likewise. * testsuite/21_strings/basic_string/capacity/wchar_t/18654.cc: Likewise. * testsuite/21_strings/basic_string/capacity/wchar_t/2.cc: Likewise. --- libstdc++-v3/config/abi/pre/gnu.ver | 17 ++- .../doc/xml/manual/status_cxx2020.xml | 3 +- libstdc++-v3/include/bits/basic_string.h | 52 ++++---- libstdc++-v3/include/bits/basic_string.tcc | 119 +++++++++++++----- .../21_strings/basic_string/capacity/1.cc | 14 ++- .../basic_string/capacity/char/1.cc | 14 ++- .../basic_string/capacity/char/18654.cc | 8 +- .../basic_string/capacity/char/2.cc | 6 +- .../basic_string/capacity/wchar_t/1.cc | 14 ++- .../basic_string/capacity/wchar_t/18654.cc | 8 +- .../basic_string/capacity/wchar_t/2.cc | 6 +- 11 files changed, 182 insertions(+), 79 deletions(-) diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver index b6ce76c1f20..b582f53e363 100644 --- a/libstdc++-v3/config/abi/pre/gnu.ver +++ b/libstdc++-v3/config/abi/pre/gnu.ver @@ -223,7 +223,10 @@ GLIBCXX_3.4 { _ZNSs6assignE[PRcjmvy]*; _ZNSs6insertE[PRcjmvy]*; _ZNSs6insertEN9__gnu_cxx17__normal_iteratorIPcSsEE[PRcjmvy]*; - _ZNSs[67][j-z]*E[PRcjmvy]*; + _ZNSs6rbeginEv; + _ZNSs6resizeE[jmy]*; + _ZNSs7replaceE[jmy]*; + _ZNSs7reserveE[jmy]; _ZNSs7[a-z]*EES2_[NPRjmy]*; _ZNSs7[a-z]*EES2_S[12]*; _ZNSs12_Alloc_hiderC*; @@ -290,7 +293,10 @@ GLIBCXX_3.4 { _ZNSbIwSt11char_traitsIwESaIwEE6assignE[PRwjmvy]*; _ZNSbIwSt11char_traitsIwESaIwEE6insertE[PRwjmvy]*; _ZNSbIwSt11char_traitsIwESaIwEE6insertEN9__gnu_cxx17__normal_iteratorIPwS2_EE[PRwjmvy]*; - _ZNSbIwSt11char_traitsIwESaIwEE[67][j-z]*E[PRwjmvy]*; + _ZNSbIwSt11char_traitsIwESaIwEE6rbeginEv; + _ZNSbIwSt11char_traitsIwESaIwEE7replaceEmm[PRm]*; + _ZNSbIwSt11char_traitsIwESaIwEE6resizeEm*; + _ZNSbIwSt11char_traitsIwESaIwEE7reserveEm; _ZNSbIwSt11char_traitsIwESaIwEE7[a-z]*EES6_[NPRjmy]*; _ZNSbIwSt11char_traitsIwESaIwEE7[a-z]*EES6_S[56]*; _ZNSbIwSt11char_traitsIwESaIwEE12_Alloc_hiderC*; @@ -1740,7 +1746,7 @@ GLIBCXX_3.4.21 { _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE6rbegin*; _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE6resize*; _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE7replace*; - _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE7reserve*; + _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE7reserveE[jmy]; _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE8pop_back*; _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE9push_back*; _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE[7-9]_[MS]_*; @@ -2309,6 +2315,11 @@ GLIBCXX_3.4.29 { # std::__istream_extract(wistream&, wchar_t*, streamsize) _ZSt17__istream_extractIwSt11char_traitsIwEEvRSt13basic_istreamIT_T0_EPS3_[ilx]; + # basic_string::reserve() + _ZNSs7reserveEv; + _ZNSbIwSt11char_traitsIwESaIwEE7reserveEv; + _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE7reserveEv; + } GLIBCXX_3.4.28; # Symbols in the support library (libsupc++) have their own tag. diff --git a/libstdc++-v3/doc/xml/manual/status_cxx2020.xml b/libstdc++-v3/doc/xml/manual/status_cxx2020.xml index ade77cbb80b..b9ad03c720f 100644 --- a/libstdc++-v3/doc/xml/manual/status_cxx2020.xml +++ b/libstdc++-v3/doc/xml/manual/status_cxx2020.xml @@ -635,13 +635,12 @@ or any notes about the implementation. - string::reserve Should Not Shrink P0966R1 - + 11 diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h index 52cecdd7ca1..e34b5c1fca3 100644 --- a/libstdc++-v3/include/bits/basic_string.h +++ b/libstdc++-v3/include/bits/basic_string.h @@ -940,20 +940,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 { this->resize(__n, _CharT()); } #if __cplusplus >= 201103L +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" /// A non-binding request to reduce capacity() to size(). void shrink_to_fit() noexcept - { -#if __cpp_exceptions - if (capacity() > size()) - { - try - { reserve(0); } - catch(...) - { } - } -#endif - } + { reserve(); } +#pragma GCC diagnostic pop #endif /** @@ -985,7 +978,16 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 * data. */ void - reserve(size_type __res_arg = 0); + reserve(size_type __res_arg); + + /** + * Equivalent to shrink_to_fit(). + */ +#if __cplusplus > 201703L + [[deprecated("use shrink_to_fit() instead")]] +#endif + void + reserve(); /** * Erases the string, making it empty. @@ -3942,20 +3944,13 @@ _GLIBCXX_END_NAMESPACE_CXX11 { this->resize(__n, _CharT()); } #if __cplusplus >= 201103L +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" /// A non-binding request to reduce capacity() to size(). void - shrink_to_fit() _GLIBCXX_NOEXCEPT - { -#if __cpp_exceptions - if (capacity() > size()) - { - try - { reserve(0); } - catch(...) - { } - } -#endif - } + shrink_to_fit() noexcept + { reserve(); } +#pragma GCC diagnostic pop #endif /** @@ -3984,7 +3979,14 @@ _GLIBCXX_END_NAMESPACE_CXX11 * data. */ void - reserve(size_type __res_arg = 0); + reserve(size_type __res_arg); + + /// Equivalent to shrink_to_fit(). +#if __cplusplus > 201703L + [[deprecated("use shrink_to_fit() instead")]] +#endif + void + reserve(); /** * Erases the string, making it empty. diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc index 75218a40610..a64b63a37fb 100644 --- a/libstdc++-v3/include/bits/basic_string.tcc +++ b/libstdc++-v3/include/bits/basic_string.tcc @@ -280,29 +280,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION basic_string<_CharT, _Traits, _Alloc>:: reserve(size_type __res) { - // Make sure we don't shrink below the current size. - if (__res < length()) - __res = length(); - const size_type __capacity = capacity(); - if (__res != __capacity) - { - if (__res > __capacity - || __res > size_type(_S_local_capacity)) - { - pointer __tmp = _M_create(__res, __capacity); - this->_S_copy(__tmp, _M_data(), length() + 1); - _M_dispose(); - _M_data(__tmp); - _M_capacity(__res); - } - else if (!_M_is_local()) - { - this->_S_copy(_M_local_data(), _M_data(), length() + 1); - _M_destroy(__capacity); - _M_data(_M_local_data()); - } - } + // _GLIBCXX_RESOLVE_LIB_DEFECTS + // 2968. Inconsistencies between basic_string reserve and + // vector/unordered_map/unordered_set reserve functions + // P0966 reserve should not shrink + if (__res <= __capacity) + return; + + pointer __tmp = _M_create(__res, __capacity); + this->_S_copy(__tmp, _M_data(), length() + 1); + _M_dispose(); + _M_data(__tmp); + _M_capacity(__res); } template @@ -342,6 +332,41 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _M_set_length(length() - __n); } + template + void + basic_string<_CharT, _Traits, _Alloc>:: + reserve() + { + if (_M_is_local()) + return; + + const size_type __length = length(); + const size_type __capacity = _M_allocated_capacity; + + if (__length <= size_type(_S_local_capacity)) + { + this->_S_copy(_M_local_data(), _M_data(), __length + 1); + _M_destroy(__capacity); + _M_data(_M_local_data()); + } +#if __cpp_exceptions + else if (__length < __capacity) + try + { + pointer __tmp + = _Alloc_traits::allocate(_M_get_allocator(), __length + 1); + this->_S_copy(__tmp, _M_data(), __length + 1); + _M_dispose(); + _M_data(__tmp); + _M_capacity(__length); + } + catch (const __cxxabiv1::__forced_unwind&) + { throw; } + catch (...) + { /* swallow the exception */ } +#endif + } + template void basic_string<_CharT, _Traits, _Alloc>:: @@ -953,16 +978,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION basic_string<_CharT, _Traits, _Alloc>:: reserve(size_type __res) { - if (__res != this->capacity() || _M_rep()->_M_is_shared()) - { - // Make sure we don't shrink below the current size - if (__res < this->size()) - __res = this->size(); - const allocator_type __a = get_allocator(); - _CharT* __tmp = _M_rep()->_M_clone(__a, __res - this->size()); - _M_rep()->_M_dispose(__a); - _M_data(__tmp); - } + const size_type __capacity = capacity(); + + // _GLIBCXX_RESOLVE_LIB_DEFECTS + // 2968. Inconsistencies between basic_string reserve and + // vector/unordered_map/unordered_set reserve functions + // P0966 reserve should not shrink + if (__res <= __capacity) + { + if (!_M_rep()->_M_is_shared()) + return; + + // unshare, but keep same capacity + __res = __capacity; + } + + const allocator_type __a = get_allocator(); + _CharT* __tmp = _M_rep()->_M_clone(__a, __res - this->size()); + _M_rep()->_M_dispose(__a); + _M_data(__tmp); } template @@ -1006,7 +1040,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // The standard places no restriction on allocating more memory // than is strictly needed within this layer at the moment or as - // requested by an explicit application call to reserve(). + // requested by an explicit application call to reserve(n). // Many malloc implementations perform quite poorly when an // application attempts to allocate memory in a stepwise fashion @@ -1140,6 +1174,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return *this; } + template + void + basic_string<_CharT, _Traits, _Alloc>:: + reserve() + { + if (length() < capacity() || _M_rep()->_M_is_shared()) + try + { + const allocator_type __a = get_allocator(); + _CharT* __tmp = _M_rep()->_M_clone(__a); + _M_rep()->_M_dispose(__a); + _M_data(__tmp); + } + catch (const __cxxabiv1::__forced_unwind&) + { throw; } + catch (...) + { /* swallow the exception */ } + } + template typename basic_string<_CharT, _Traits, _Alloc>::size_type basic_string<_CharT, _Traits, _Alloc>:: diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/1.cc b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/1.cc index 02f6bb2629e..a60f2d62da8 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/1.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/1.cc @@ -136,13 +136,19 @@ void test01() sz04 = str02.capacity(); VERIFY( sz04 >= sz03 ); VERIFY( sz04 >= 100 ); +#if __cplusplus <= 201703L str02.reserve(); - sz03 = str02.capacity(); -#if _GLIBCXX_USE_CXX11_ABI - VERIFY( sz03 < 100); #else - VERIFY( sz03 == 0 ); + str02.shrink_to_fit(); // reserve is deprecated in C++20 #endif + sz03 = str02.capacity(); + VERIFY( sz03 < sz04 ); + + // P0966: reserve should not shrink + str02.reserve(100); + sz03 = str02.capacity(); + str02.reserve(sz03 - 1); + VERIFY( str02.capacity() == sz03 ); sz03 = str02.size() + 5; str02.resize(sz03); diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/char/1.cc b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/char/1.cc index 854c2902e6b..cde253e1142 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/char/1.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/char/1.cc @@ -33,13 +33,19 @@ void test01() size_type_s sz02 = str01.capacity(); VERIFY( sz02 >= sz01 ); VERIFY( sz02 >= 100 ); +#if __cplusplus <= 201703L str01.reserve(); - sz01 = str01.capacity(); -#if _GLIBCXX_USE_CXX11_ABI - VERIFY( sz01 < 100); #else - VERIFY( sz01 == 0 ); + str01.shrink_to_fit(); // reserve is deprecated in C++20 #endif + sz01 = str01.capacity(); + VERIFY( sz01 < sz02 ); + + // P0966: reserve should not shrink + str01.reserve(100); + sz01 = str01.capacity(); + str01.reserve(sz01 - 1); + VERIFY( str01.capacity() == sz01 ); sz01 = str01.size() + 5; str01.resize(sz01); diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/char/18654.cc b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/char/18654.cc index e9dbf7f55f6..13f65f57133 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/char/18654.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/char/18654.cc @@ -47,11 +47,17 @@ void test01() { string str(i, 'x'); str.reserve(3 * i); + const size_type cap = str.capacity(); + VERIFY( cap >= 3 * i ); str.reserve(2 * i); - VERIFY( str.capacity() == 2 * i ); + VERIFY( str.capacity() == cap ); +#if __cplusplus <= 201703L str.reserve(); +#else + str.shrink_to_fit(); // reserve is deprecated in C++20 +#endif VERIFY( str.capacity() == i ); } } diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/char/2.cc b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/char/2.cc index 43ae4c6cd9d..1e87dde6d2e 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/char/2.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/char/2.cc @@ -29,7 +29,11 @@ void test02() std::string str01 = "twelve chars"; // str01 becomes shared std::string str02 = str01; - str01.reserve(1); +#if __cplusplus <= 201703L + str01.reserve(); +#else + str01.shrink_to_fit(); // reserve is deprecated in C++20 +#endif VERIFY( str01.capacity() >= 12 ); } diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/wchar_t/1.cc b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/wchar_t/1.cc index 436ed59a3b5..fa1584cc134 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/wchar_t/1.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/wchar_t/1.cc @@ -33,13 +33,19 @@ void test01() size_type_s sz02 = str01.capacity(); VERIFY( sz02 >= sz01 ); VERIFY( sz02 >= 100 ); +#if __cplusplus <= 201703L str01.reserve(); - sz01 = str01.capacity(); -#if _GLIBCXX_USE_CXX11_ABI - VERIFY( sz01 < 100); #else - VERIFY( sz01 == 0 ); + str01.shrink_to_fit(); // reserve is deprecated in C++20 #endif + sz01 = str01.capacity(); + VERIFY( sz01 < sz02 ); + + // P0966: reserve should not shrink + str01.reserve(100); + sz01 = str01.capacity(); + str01.reserve(sz01 - 1); + VERIFY( str01.capacity() == sz01 ); sz01 = str01.size() + 5; str01.resize(sz01); diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/wchar_t/18654.cc b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/wchar_t/18654.cc index 374c27ea43d..fcdcd6abb14 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/wchar_t/18654.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/wchar_t/18654.cc @@ -47,11 +47,17 @@ void test01() { wstring str(i, L'x'); str.reserve(3 * i); + const size_type cap = str.capacity(); + VERIFY( cap >= 3 * i ); str.reserve(2 * i); - VERIFY( str.capacity() == 2 * i ); + VERIFY( str.capacity() == cap ); +#if __cplusplus <= 201703L str.reserve(); +#else + str.shrink_to_fit(); // reserve is deprecated in C++20 +#endif VERIFY( str.capacity() == i ); } } diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/wchar_t/2.cc b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/wchar_t/2.cc index fac8adffcf4..1ec140e61bf 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/capacity/wchar_t/2.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/capacity/wchar_t/2.cc @@ -29,7 +29,11 @@ void test02() std::wstring str01 = L"twelve chars"; // str01 becomes shared std::wstring str02 = str01; - str01.reserve(1); +#if __cplusplus <= 201703L + str01.reserve(); +#else + str01.shrink_to_fit(); // reserve is deprecated in C++20 +#endif VERIFY( str01.capacity() == 12 ); } -- 2.30.2