From 11d10beb5762c48de90a004368df2c2863b33d7a Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Wed, 1 Feb 2017 11:41:48 +0000 Subject: [PATCH] PR libstdc++/79254 simplify exception-safety in copy assignment PR libstdc++/79254 * config/abi/pre/gnu.ver: Remove recently added symbols. * include/bits/basic_string.h [_GLIBCXX_USE_CXX11_ABI] (basic_string::_M_copy_assign): Remove. (basic_string::operator=(const basic_string&)): Don't dispatch to _M_copy_assign. If source object is small just deallocate, otherwise perform new allocation before making any changes. * include/bits/basic_string.tcc [_GLIBCXX_USE_CXX11_ABI] (basic_string::_M_copy_assign(const basic_string&, true_type)): Remove. * testsuite/21_strings/basic_string/allocator/char/copy_assign.cc: Test cases where the allocators are equal or the string is small. * testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc: Likewise. From-SVN: r245085 --- libstdc++-v3/ChangeLog | 17 +++++ libstdc++-v3/config/abi/pre/gnu.ver | 3 - libstdc++-v3/include/bits/basic_string.h | 42 ++++++++---- libstdc++-v3/include/bits/basic_string.tcc | 64 ------------------- .../allocator/char/copy_assign.cc | 10 +++ .../allocator/wchar_t/copy_assign.cc | 10 +++ 6 files changed, 65 insertions(+), 81 deletions(-) diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index fb6255e02f9..979ded4aeab 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,20 @@ +2017-02-01 Jonathan Wakely + + PR libstdc++/79254 + * config/abi/pre/gnu.ver: Remove recently added symbols. + * include/bits/basic_string.h [_GLIBCXX_USE_CXX11_ABI] + (basic_string::_M_copy_assign): Remove. + (basic_string::operator=(const basic_string&)): Don't dispatch to + _M_copy_assign. If source object is small just deallocate, otherwise + perform new allocation before making any changes. + * include/bits/basic_string.tcc [_GLIBCXX_USE_CXX11_ABI] + (basic_string::_M_copy_assign(const basic_string&, true_type)): + Remove. + * testsuite/21_strings/basic_string/allocator/char/copy_assign.cc: + Test cases where the allocators are equal or the string is small. + * testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc: + Likewise. + 2017-01-30 Ville Voutilainen Implement LWG 2825, LWG 2756 breaks class template argument diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver index 1bea4b4d7fe..268fb9479a9 100644 --- a/libstdc++-v3/config/abi/pre/gnu.ver +++ b/libstdc++-v3/config/abi/pre/gnu.ver @@ -1955,9 +1955,6 @@ GLIBCXX_3.4.23 { _ZNSsC[12]ERKSs[jmy]RKSaIcE; _ZNSbIwSt11char_traitsIwESaIwEEC[12]ERKS2_mRKS1_; - # basic_string::_M_copy_assign(const basic_string&, {true,false}_type) - _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE14_M_copy_assign*; - #ifndef HAVE_EXCEPTION_PTR_SINCE_GCC46 # std::future symbols are exported in the first version to support # std::exception_ptr diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h index 97fe797d4b4..981ffc5984a 100644 --- a/libstdc++-v3/include/bits/basic_string.h +++ b/libstdc++-v3/include/bits/basic_string.h @@ -393,15 +393,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 void _M_erase(size_type __pos, size_type __n); -#if __cplusplus >= 201103L - void - _M_copy_assign(const basic_string& __str, /* pocca = */ true_type); - - void - _M_copy_assign(const basic_string& __str, /* pocca = */ false_type) - { this->_M_assign(__str); } -#endif - public: // Construct/copy/destroy: // NB: We overload ctors in some cases instead of using default @@ -636,12 +627,35 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 operator=(const basic_string& __str) { #if __cplusplus >= 201103L - _M_copy_assign(__str, - typename _Alloc_traits::propagate_on_container_copy_assignment()); -#else - this->_M_assign(__str); + if (_Alloc_traits::_S_propagate_on_copy_assign()) + { + if (!_Alloc_traits::_S_always_equal() && !_M_is_local() + && _M_get_allocator() != __str._M_get_allocator()) + { + // Propagating allocator cannot free existing storage so must + // deallocate it before replacing current allocator. + if (__str.size() <= _S_local_capacity) + { + _M_destroy(_M_allocated_capacity); + _M_data(_M_local_data()); + _M_set_length(0); + } + else + { + const auto __len = __str.size(); + auto __alloc = __str._M_get_allocator(); + // If this allocation throws there are no effects: + auto __ptr = _Alloc_traits::allocate(__alloc, __len + 1); + _M_destroy(_M_allocated_capacity); + _M_data(__ptr); + _M_capacity(__len); + _M_set_length(__len); + } + } + std::__alloc_on_copy(_M_get_allocator(), __str._M_get_allocator()); + } #endif - return *this; + return this->assign(__str); } /** diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc index adc8b853137..41b7fa196b0 100644 --- a/libstdc++-v3/include/bits/basic_string.tcc +++ b/libstdc++-v3/include/bits/basic_string.tcc @@ -275,70 +275,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } } -#if __cplusplus >= 201103L - template - void - basic_string<_CharT, _Traits, _Alloc>:: - _M_copy_assign(const basic_string& __str, true_type) - { - struct _Guard // RAII type for strong exception-safety guarantee. - { - // Takes ownership of string's original state. - _Guard(basic_string* __self) - : _M_self(__self), _M_alloc(std::move(__self->_M_get_allocator())), - _M_ptr(__self->_M_data()), - _M_capacity(__self->_M_allocated_capacity), _M_len(__self->length()) - { - __self->_M_data(__self->_M_local_data()); - __self->_M_length(0); - } - - // Restores string's original state if _M_release() was not called. - ~_Guard() - { - if (_M_ptr) - { - _M_self->_M_get_allocator() = std::move(_M_alloc); - _M_self->_M_data(_M_ptr); - _M_self->_M_capacity(_M_capacity); - _M_self->_M_length(_M_len); - } - } - - _Guard(const _Guard&) = delete; - _Guard& operator=(const _Guard&) = delete; - - void _M_release() - { - // Original state can be freed now. - _Alloc_traits::deallocate(_M_alloc, _M_ptr, _M_capacity + 1); - _M_ptr = nullptr; - } - - basic_string* _M_self; - allocator_type _M_alloc; - pointer _M_ptr; - size_type _M_capacity; - size_type _M_len; - }; - - if (!_Alloc_traits::_S_always_equal() && !_M_is_local() - && _M_get_allocator() != __str._M_get_allocator()) - { - // The propagating allocator cannot free existing storage. - _Guard __guard(this); - _M_get_allocator() = __str._M_get_allocator(); - this->_M_assign(__str); - __guard._M_release(); - } - else - { - _M_get_allocator() = __str._M_get_allocator(); - this->_M_assign(__str); - } - } -#endif - template void basic_string<_CharT, _Traits, _Alloc>:: diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/copy_assign.cc b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/copy_assign.cc index 0fc80d7fb4d..bea221dd8e2 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/copy_assign.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/copy_assign.cc @@ -121,6 +121,16 @@ void test03() VERIFY( caught ); VERIFY( v1 == s1 ); VERIFY( v1.get_allocator() == a1 ); + + throw_alloc::set_limit(1); // Allow one more allocation (and no more). + test_type v3(s1, a1); + // No allocation when allocators are equal and capacity is sufficient: + VERIFY( v1.capacity() >= v3.size() ); + v1 = v3; + // No allocation when the contents fit in the small-string buffer: + v2 = "sso"; + v1 = v2; + VERIFY( v1.get_allocator() == a2 ); } int main() diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc index c35e001934d..e83c4c5e193 100644 --- a/libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc +++ b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/wchar_t/copy_assign.cc @@ -121,6 +121,16 @@ void test03() VERIFY( caught ); VERIFY( v1 == s1 ); VERIFY( v1.get_allocator() == a1 ); + + throw_alloc::set_limit(1); // Allow one more allocation (and no more). + test_type v3(s1, a1); + // No allocation when allocators are equal and capacity is sufficient: + VERIFY( v1.capacity() >= v3.size() ); + v1 = v3; + // No allocation when the contents fit in the small-string buffer: + v2 = L"sso"; + v1 = v2; + VERIFY( v1.get_allocator() == a2 ); } int main() -- 2.30.2