From 45e738dc534f46f8bcbf30d53c189b5ea989d7f8 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Tue, 11 Jul 2023 18:16:57 +0800 Subject: [PATCH 01/26] Make `string(const T*)` and `string(const T*, const Alloc&)` adjacent --- stl/inc/xstring | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 6dee3cfb28f..80e08f736cf 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2539,10 +2539,6 @@ public: _Construct<_Construct_strategy::_From_ptr>(_Ptr, _Convert_size(_Traits::length(_Ptr))); } -#if _HAS_CXX23 - basic_string(nullptr_t) = delete; -#endif // _HAS_CXX23 - #if _HAS_CXX17 template ::value, int> = 0> #endif // _HAS_CXX17 @@ -2551,6 +2547,10 @@ public: _Construct<_Construct_strategy::_From_ptr>(_Ptr, _Convert_size(_Traits::length(_Ptr))); } +#if _HAS_CXX23 + basic_string(nullptr_t) = delete; +#endif // _HAS_CXX23 + _CONSTEXPR20 basic_string(_CRT_GUARDOVERFLOW const size_type _Count, const _Elem _Ch) : _Mypair(_Zero_then_variadic_args_t{}) { // construct from _Count * _Ch _Construct<_Construct_strategy::_From_char>(_Ch, _Count); From 9f726f9b6ccb1abbe26bebe563028300a09dc2fa Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Thu, 27 Jul 2023 14:45:36 -0700 Subject: [PATCH 02/26] Add `_NODISCARD` for `_String_val` methods --- stl/inc/xstring | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 80e08f736cf..9c64cc4ad01 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2221,7 +2221,7 @@ public: : sizeof(value_type) <= 8 ? 1 : 0; - _CONSTEXPR20 value_type* _Myptr() noexcept { + _NODISCARD _CONSTEXPR20 value_type* _Myptr() noexcept { value_type* _Result = _Bx._Buf; if (_Large_string_engaged()) { _Result = _Unfancy(_Bx._Ptr); @@ -2230,7 +2230,7 @@ public: return _Result; } - _CONSTEXPR20 const value_type* _Myptr() const noexcept { + _NODISCARD _CONSTEXPR20 const value_type* _Myptr() const noexcept { const value_type* _Result = _Bx._Buf; if (_Large_string_engaged()) { _Result = _Unfancy(_Bx._Ptr); @@ -2239,7 +2239,7 @@ public: return _Result; } - _CONSTEXPR20 bool _Large_string_engaged() const noexcept { + _NODISCARD _CONSTEXPR20 bool _Large_string_engaged() const noexcept { return _BUF_SIZE <= _Myres; } @@ -2272,7 +2272,7 @@ public: _Xout_of_range("invalid string position"); } - _CONSTEXPR20 size_type _Clamp_suffix_size(const size_type _Off, const size_type _Size) const noexcept { + _NODISCARD _CONSTEXPR20 size_type _Clamp_suffix_size(const size_type _Off, const size_type _Size) const noexcept { // trims _Size to the longest it can be assuming a string at/after _Off return (_STD min)(_Size, _Mysize - _Off); } From 02b90edc135db72201fbf5598c8bc78eceb34e59 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Thu, 27 Jul 2023 14:45:58 -0700 Subject: [PATCH 03/26] make `_Activate_SSO_buffer` `_CONSTEXPR20` --- stl/inc/xstring | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 9c64cc4ad01..8daf31ca62b 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2243,7 +2243,7 @@ public: return _BUF_SIZE <= _Myres; } - constexpr void _Activate_SSO_buffer() noexcept { + _CONSTEXPR20 void _Activate_SSO_buffer() noexcept { // start the lifetime of the array elements #if _HAS_CXX20 if (_STD is_constant_evaluated()) { From ca0953aebef0a840a0c80f288d615ee21e560314 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Tue, 11 Jul 2023 18:26:59 +0800 Subject: [PATCH 04/26] Make types of `_BUF_SIZE` and `_ALLOC_MASK` clear --- stl/inc/xstring | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 8daf31ca62b..26efaa20afc 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2360,8 +2360,8 @@ public: using const_reverse_iterator = _STD reverse_iterator; private: - static constexpr auto _BUF_SIZE = _Scary_val::_BUF_SIZE; - static constexpr auto _ALLOC_MASK = _Scary_val::_ALLOC_MASK; + static constexpr size_type _BUF_SIZE = _Scary_val::_BUF_SIZE; + static constexpr size_type _ALLOC_MASK = _Scary_val::_ALLOC_MASK; // When doing _String_val operations by memcpy, we are touching: // _String_val::_Bx::_Buf (type is array of _Elem) From ef708ed26c8bfaebd609f207dcdb33d6e07ce0c7 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Tue, 11 Jul 2023 18:39:12 +0800 Subject: [PATCH 05/26] Introduce `_SMALL_STRING_CAPACITY`; replace `_BUF_SIZE - 1` with it --- stl/inc/xstring | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 26efaa20afc..14392744680 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2220,6 +2220,8 @@ public: : sizeof(value_type) <= 4 ? 3 : sizeof(value_type) <= 8 ? 1 : 0; + // capacity in SSO mode + static constexpr size_type _SMALL_STRING_CAPACITY = _BUF_SIZE - 1; _NODISCARD _CONSTEXPR20 value_type* _Myptr() noexcept { value_type* _Result = _Bx._Buf; @@ -2360,8 +2362,9 @@ public: using const_reverse_iterator = _STD reverse_iterator; private: - static constexpr size_type _BUF_SIZE = _Scary_val::_BUF_SIZE; - static constexpr size_type _ALLOC_MASK = _Scary_val::_ALLOC_MASK; + static constexpr size_type _BUF_SIZE = _Scary_val::_BUF_SIZE; + static constexpr size_type _ALLOC_MASK = _Scary_val::_ALLOC_MASK; + static constexpr size_type _SMALL_STRING_CAPACITY = _Scary_val::_SMALL_STRING_CAPACITY; // When doing _String_val operations by memcpy, we are touching: // _String_val::_Bx::_Buf (type is array of _Elem) @@ -2422,7 +2425,7 @@ private: #endif // _HAS_CXX20 // Don't annotate small strings; only annotate on the heap. if (_Capacity == _BUF_SIZE - 1 || !_Asan_string_should_annotate) { - return; + return; // "_BUF_SIZE - 1" is unchanged here; will be refactored in later pushes. } // Note that `_Capacity`, `_Old_size`, and `_New_size` do not include the null terminator @@ -2626,7 +2629,7 @@ private: if (_Count < _BUF_SIZE) { _My_data._Mysize = _Count; - _My_data._Myres = _BUF_SIZE - 1; + _My_data._Myres = _SMALL_STRING_CAPACITY; if constexpr (_Strat == _Construct_strategy::_From_char) { _Traits::assign(_My_data._Bx._Buf, _Count, _Arg); @@ -2646,7 +2649,7 @@ private: return; } - _My_data._Myres = _BUF_SIZE - 1; + _My_data._Myres = _SMALL_STRING_CAPACITY; size_type _New_capacity = _Calculate_growth(_Count); ++_New_capacity; const pointer _New_ptr = _Allocate_at_least_helper(_Al, _New_capacity); // throws @@ -2684,7 +2687,7 @@ private: _Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _My_data); _My_data._Mysize = 0; - _My_data._Myres = _BUF_SIZE - 1; + _My_data._Myres = _SMALL_STRING_CAPACITY; if constexpr (is_same_v<_Size, size_type>) { if (_Count > max_size()) { @@ -2790,14 +2793,14 @@ public: _STL_INTERNAL_CHECK(_Right_size <= max_size()); _STL_INTERNAL_CHECK(_Right_size <= max_size() - _Left_size); const auto _New_size = static_cast(_Left_size + _Right_size); - size_type _New_capacity = _BUF_SIZE - 1; + size_type _New_capacity = _SMALL_STRING_CAPACITY; auto& _My_data = _Mypair._Myval2; _Elem* _Ptr = _My_data._Bx._Buf; auto&& _Alproxy = _GET_PROXY_ALLOCATOR(_Alty, _Getal()); _Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _My_data); // throws if (_New_capacity < _New_size) { - _New_capacity = _Calculate_growth(_New_size, _BUF_SIZE - 1, max_size()); + _New_capacity = _Calculate_growth(_New_size, _SMALL_STRING_CAPACITY, max_size()); ++_New_capacity; const pointer _Fancyptr = _Allocate_at_least_helper(_Getal(), _New_capacity); // throws --_New_capacity; @@ -2870,7 +2873,7 @@ public: _Xlen_string(); } - auto _New_capacity = _Calculate_growth(_New_size, _BUF_SIZE - 1, _Max); + auto _New_capacity = _Calculate_growth(_New_size, _SMALL_STRING_CAPACITY, _Max); auto&& _Alproxy = _GET_PROXY_ALLOCATOR(_Alty, _Getal()); _Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _My_data); // throws ++_New_capacity; @@ -2931,7 +2934,7 @@ public: } else { _Traits::copy(_My_data._Bx._Buf, _Right, _Res); _My_data._Mysize = _Size; - _My_data._Myres = _BUF_SIZE - 1; + _My_data._Myres = _SMALL_STRING_CAPACITY; } return _Is_large; @@ -4824,7 +4827,7 @@ private: _My_data._Activate_SSO_buffer(); _Traits::copy(_My_data._Bx._Buf, _Unfancy(_Ptr), _My_data._Mysize + 1); _Al.deallocate(_Ptr, _My_data._Myres + 1); - _My_data._Myres = _BUF_SIZE - 1; + _My_data._Myres = _SMALL_STRING_CAPACITY; } _CONSTEXPR20 void _Eos(const size_type _New_size) noexcept { // set new length and null terminator @@ -4836,7 +4839,7 @@ private: // initialize basic_string data members auto& _My_data = _Mypair._Myval2; _My_data._Mysize = 0; - _My_data._Myres = _BUF_SIZE - 1; + _My_data._Myres = _SMALL_STRING_CAPACITY; _My_data._Activate_SSO_buffer(); // the _Traits::assign is last so the codegen doesn't think the char write can alias this @@ -4856,7 +4859,7 @@ private: } _My_data._Mysize = 0; - _My_data._Myres = _BUF_SIZE - 1; + _My_data._Myres = _SMALL_STRING_CAPACITY; // the _Traits::assign is last so the codegen doesn't think the char write can alias this _Traits::assign(_My_data._Bx._Buf[0], _Elem()); } From d3f736b35a99c45f64b44d23c88b7310b18aad10 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Tue, 11 Jul 2023 18:48:03 +0800 Subject: [PATCH 06/26] Refine `_Large_string_engaged`; find a missed usecase for it --- stl/inc/xstring | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 14392744680..57f1d13a1bb 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2242,7 +2242,9 @@ public: } _NODISCARD _CONSTEXPR20 bool _Large_string_engaged() const noexcept { - return _BUF_SIZE <= _Myres; + // Determines whether large mode is used for the string's capacity. + // Doesn't imply anything about the largeness of the string's size. + return _Myres > _SMALL_STRING_CAPACITY; } _CONSTEXPR20 void _Activate_SSO_buffer() noexcept { @@ -2722,7 +2724,7 @@ private: _Start_element_lifetimes(_Unfancy(_New_ptr), _New_capacity + 1); _Traits::copy(_Unfancy(_New_ptr), _Old_ptr, _My_data._Mysize); - if (_My_data._Myres >= _BUF_SIZE) { // Need to deallocate old storage + if (_My_data._Large_string_engaged()) { // Need to deallocate old storage _Al.deallocate(_My_data._Bx._Ptr, _My_data._Myres + 1); _My_data._Bx._Ptr = _New_ptr; } else { From e1ce499c979432b139f615bdd302f12fe1c2ae97 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Tue, 11 Jul 2023 19:10:10 +0800 Subject: [PATCH 07/26] Introduce `_Entails_large_string`; replace wild `_Large_string_engaged` logic with it --- stl/inc/xstring | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 57f1d13a1bb..834d211de30 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2368,6 +2368,13 @@ private: static constexpr size_type _ALLOC_MASK = _Scary_val::_ALLOC_MASK; static constexpr size_type _SMALL_STRING_CAPACITY = _Scary_val::_SMALL_STRING_CAPACITY; + _NODISCARD static _CONSTEXPR20 bool _Entails_large_string(size_type _Size_or_capacity) noexcept { + // Use cases: + // 1. determine whether large mode is used for a given capacity. + // 2. determine whether large mode should be used for a size/capacity request. + return _Size_or_capacity > _SMALL_STRING_CAPACITY; + } + // When doing _String_val operations by memcpy, we are touching: // _String_val::_Bx::_Buf (type is array of _Elem) // _String_val::_Bx::_Ptr (type is pointer) @@ -2426,8 +2433,8 @@ private: } #endif // _HAS_CXX20 // Don't annotate small strings; only annotate on the heap. - if (_Capacity == _BUF_SIZE - 1 || !_Asan_string_should_annotate) { - return; // "_BUF_SIZE - 1" is unchanged here; will be refactored in later pushes. + if (!_Entails_large_string(_Capacity) || !_Asan_string_should_annotate) { + return; } // Note that `_Capacity`, `_Old_size`, and `_New_size` do not include the null terminator @@ -4766,7 +4773,7 @@ private: _Mypair._Myval2._Mysize = _New_size; _Mypair._Myval2._Myres = _New_capacity; _Fn(_Unfancy(_New_ptr), _New_size, _Args...); - if (_BUF_SIZE <= _Old_capacity) { + if (_Entails_large_string(_Old_capacity)) { _Al.deallocate(_Mypair._Myval2._Bx._Ptr, _Old_capacity + 1); _Mypair._Myval2._Bx._Ptr = _New_ptr; } else { @@ -4801,7 +4808,7 @@ private: _My_data._Mysize = _New_size; _My_data._Myres = _New_capacity; _Elem* const _Raw_new = _Unfancy(_New_ptr); - if (_BUF_SIZE <= _Old_capacity) { + if (_Entails_large_string(_Old_capacity)) { const pointer _Old_ptr = _My_data._Bx._Ptr; _Fn(_Raw_new, _Unfancy(_Old_ptr), _Old_size, _Args...); _Al.deallocate(_Old_ptr, _Old_capacity + 1); From e050101a97f09c33f268057ebb1a51a2b51d6603 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Tue, 11 Jul 2023 19:52:34 +0800 Subject: [PATCH 08/26] Replace capacity-decision logic with `_Entails_large_string` --- stl/inc/xstring | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 834d211de30..6b870c24cac 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2636,7 +2636,7 @@ private: auto&& _Alproxy = _GET_PROXY_ALLOCATOR(_Alty, _Al); _Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _My_data); - if (_Count < _BUF_SIZE) { + if (!_Entails_large_string(_Count)) { _My_data._Mysize = _Count; _My_data._Myres = _SMALL_STRING_CAPACITY; @@ -2703,7 +2703,7 @@ private: _Xlen_string(); // result too long } - if (_Count >= _BUF_SIZE) { + if (_Entails_large_string(_Count)) { size_type _New_capacity = _Calculate_growth(_Count); ++_New_capacity; const pointer _New_ptr = _Allocate_at_least_helper(_Al, _New_capacity); // throws @@ -3076,7 +3076,7 @@ private: const auto _Result_size = _Right_data._Clamp_suffix_size(_Roff, _Size_max); const auto _Right_ptr = _Right_data._Myptr(); auto& _Al = _Getal(); - if (_Allocators_equal(_Al, _Right._Getal()) && _Result_size >= _BUF_SIZE) { + if (_Allocators_equal(_Al, _Right._Getal()) && _Entails_large_string(_Result_size)) { if (_Roff != 0) { _Traits::move(_Right_ptr, _Right_ptr + _Roff, _Result_size); } @@ -4013,7 +4013,7 @@ public: return; } - if (_My_data._Mysize < _BUF_SIZE) { + if (!_Entails_large_string(_My_data._Mysize)) { _Become_small(); return; } @@ -4239,13 +4239,13 @@ public: return; } - if (_BUF_SIZE > _Newcap && _Mypair._Myval2._Large_string_engaged()) { + if (!_Entails_large_string(_Newcap) && _Mypair._Myval2._Large_string_engaged()) { // deallocate everything; switch back to "small" mode _Become_small(); return; } - // ignore requests to reserve to [_BUF_SIZE, _Myres) + // ignore requests to reserve to [_SMALL_STRING_CAPACITY + 1, _Myres) } #endif // _HAS_CXX20 @@ -4826,7 +4826,7 @@ private: // release any held storage and return to small string mode auto& _My_data = _Mypair._Myval2; _STL_INTERNAL_CHECK(_My_data._Large_string_engaged()); - _STL_INTERNAL_CHECK(_My_data._Mysize < _BUF_SIZE); + _STL_INTERNAL_CHECK(!_Entails_large_string(_My_data._Mysize)); _My_data._Orphan_all(); _ASAN_STRING_REMOVE(*this); From 2b966c6b8d934d9fd8f6055c78dcb6e628a3ef1b Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Tue, 11 Jul 2023 20:06:35 +0800 Subject: [PATCH 09/26] Refine documentation of interaction part with `basic_stringbuf` --- stl/inc/xstring | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 6b870c24cac..133fa4a5d7e 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2375,6 +2375,10 @@ private: return _Size_or_capacity > _SMALL_STRING_CAPACITY; } + // least real allocation size, including space reserved for terminating null + // used exclusively by _Move_assign_from_buffer and _Release_to_buffer + static constexpr size_type _LEAST_ALLOCATION_SIZE = _SMALL_STRING_CAPACITY + 1 + 1; + // When doing _String_val operations by memcpy, we are touching: // _String_val::_Bx::_Buf (type is array of _Elem) // _String_val::_Bx::_Ptr (type is pointer) @@ -2926,22 +2930,23 @@ public: _Tidy_init(); } - _NODISCARD bool _Move_assign_from_buffer(_Elem* const _Right, const size_type _Size, const size_type _Res) { + _NODISCARD bool _Move_assign_from_buffer( + _Elem* const _Right, const size_type _Size, const size_type _Buf_res /* real allocation size */) { // Move assign from a buffer, used exclusively by basic_stringbuf; returns _Large_string_engaged() auto& _My_data = _Mypair._Myval2; _STL_INTERNAL_CHECK(!_My_data._Large_string_engaged() && _My_data._Mysize == 0); - _STL_INTERNAL_CHECK(_Size < _Res); // So there is room for null terminator + _STL_INTERNAL_CHECK(_Size < _Buf_res); // So there is room for null terminator _Traits::assign(_Right[_Size], _Elem()); - const bool _Is_large = _Res > _BUF_SIZE; // Note: _BUF_SIZE because _Res now includes the null terminator + const bool _Is_large = _Buf_res >= _LEAST_ALLOCATION_SIZE; if (_Is_large) { _ASAN_STRING_REMOVE(*this); _Construct_in_place(_My_data._Bx._Ptr, _Refancy(_Right)); _My_data._Mysize = _Size; - _My_data._Myres = _Res - 1; + _My_data._Myres = _Buf_res - 1; _ASAN_STRING_CREATE(*this); } else { - _Traits::copy(_My_data._Bx._Buf, _Right, _Res); + _Traits::copy(_My_data._Bx._Buf, _Right, _Buf_res); _My_data._Mysize = _Size; _My_data._Myres = _SMALL_STRING_CAPACITY; } @@ -2953,7 +2958,7 @@ public: struct _Released_buffer { pointer _Ptr; size_type _Size; - size_type _Res; + size_type _Res; // real allocation size }; _NODISCARD _Released_buffer _Release_to_buffer(_Alloc& _Al) { @@ -2966,8 +2971,8 @@ public: _Result._Ptr = _My_data._Bx._Ptr; _Result._Res = _My_data._Myres + 1; } else { - // use _BUF_SIZE + 1 to avoid SSO, if the buffer is assigned back - size_type _Allocated = _BUF_SIZE + 1; + // use _LEAST_ALLOCATION_SIZE to avoid SSO, if the buffer is assigned back + size_type _Allocated = _LEAST_ALLOCATION_SIZE; _Result._Ptr = _Allocate_at_least_helper(_Al, _Allocated); _Traits::copy(_Unfancy(_Result._Ptr), _My_data._Bx._Buf, _BUF_SIZE); _Result._Res = _Allocated; From d0c9dfc7e05673fe1a516af9bdb5f38f19af0ce6 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Thu, 27 Jul 2023 15:03:45 -0700 Subject: [PATCH 10/26] Try to improve documentation for `_Mysize` and `_Myres` --- stl/inc/xstring | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 133fa4a5d7e..f20eaced8a0 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2293,8 +2293,10 @@ public: }; _Bxty _Bx; - size_type _Mysize = 0; // current length of string - size_type _Myres = 0; // current storage reserved for string + // invariant: _Myres >= _Mysize, and _Myres >= _SMALL_STRING_CAPACITY (after string's construction) + // both _Mysize and _Myres doesn't take account of the extra null terminator + size_type _Mysize = 0; // current length of string (size) + size_type _Myres = 0; // current storage reserved for string (capacity) }; // get _Ty's size after being EBCO'd From cb010d7893870a21abc6bb1dd0f75e700766b4b2 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Thu, 27 Jul 2023 17:42:07 -0700 Subject: [PATCH 11/26] Replace two `auto`s with explicit types. --- stl/inc/xstring | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index f20eaced8a0..41e4341db50 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2729,7 +2729,7 @@ private: _Xlen_string(); // result too long } - const auto _Old_ptr = _My_data._Myptr(); + _Elem* const _Old_ptr = _My_data._Myptr(); size_type _New_capacity = _Calculate_growth(_My_data._Mysize); ++_New_capacity; const pointer _New_ptr = _Allocate_at_least_helper(_Al, _New_capacity); // throws @@ -2888,8 +2888,8 @@ public: _Xlen_string(); } - auto _New_capacity = _Calculate_growth(_New_size, _SMALL_STRING_CAPACITY, _Max); - auto&& _Alproxy = _GET_PROXY_ALLOCATOR(_Alty, _Getal()); + size_type _New_capacity = _Calculate_growth(_New_size, _SMALL_STRING_CAPACITY, _Max); + auto&& _Alproxy = _GET_PROXY_ALLOCATOR(_Alty, _Getal()); _Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _My_data); // throws ++_New_capacity; const pointer _Fancyptr = _Allocate_at_least_helper(_Getal(), _New_capacity); // throws From 28c834ecf2e614dda14dfc0afda0095528702563 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Thu, 27 Jul 2023 15:37:19 -0700 Subject: [PATCH 12/26] Introduce `_Allocate_for_capacity` and `_Deallocate_for_capacity` and apply them wherever suitable; merge `_Start_element_lifetimes` into `_Allocate_for_capacity` --- stl/inc/xstring | 87 +++++++++++++++++++------------------------------ 1 file changed, 33 insertions(+), 54 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 41e4341db50..ca3435d6298 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2606,19 +2606,28 @@ public: } private: - static constexpr void _Start_element_lifetimes(_Elem* const _Ptr, const size_type _Size) { + _NODISCARD static _CONSTEXPR20 pointer _Allocate_for_capacity(_Alty& _Al, size_type& _Capacity) { + _STL_INTERNAL_CHECK(_Entails_large_string(_Capacity)); + ++_Capacity; // Take null terminator into consideration + const pointer _Fancy_ptr = _Allocate_at_least_helper(_Al, _Capacity); +#if _HAS_CXX20 // Start element lifetimes to avoid UB. This is a more general mechanism than _String_val::_Activate_SSO_buffer, // but likely more impactful to throughput. -#if _HAS_CXX20 if (_STD is_constant_evaluated()) { - for (size_type _Idx = 0; _Idx < _Size; ++_Idx) { + _Elem* const _Ptr = _Unfancy(_Fancy_ptr); + for (size_type _Idx = 0; _Idx < _Capacity; ++_Idx) { _STD construct_at(_Ptr + _Idx); } } -#else // ^^^ C++20-or-later / pre-C++20 vvv - (void) _Ptr; - (void) _Size; #endif // _HAS_CXX20 + --_Capacity; + return _Fancy_ptr; + } + + static _CONSTEXPR20 void _Deallocate_for_capacity( + _Alty& _Al, const pointer& _Old_ptr, const size_type _Capacity) noexcept { + _STL_INTERNAL_CHECK(_Entails_large_string(_Capacity)); + _Al.deallocate(_Old_ptr, _Capacity + 1); // +1 for null terminator } enum class _Construct_strategy : uint8_t { _From_char, _From_ptr, _From_string }; @@ -2666,13 +2675,9 @@ private: _My_data._Myres = _SMALL_STRING_CAPACITY; size_type _New_capacity = _Calculate_growth(_Count); - ++_New_capacity; - const pointer _New_ptr = _Allocate_at_least_helper(_Al, _New_capacity); // throws - --_New_capacity; + const pointer _New_ptr = _Allocate_for_capacity(_Al, _New_capacity); // throws _Construct_in_place(_My_data._Bx._Ptr, _New_ptr); - _Start_element_lifetimes(_Unfancy(_New_ptr), _New_capacity + 1); - _My_data._Mysize = _Count; _My_data._Myres = _New_capacity; if constexpr (_Strat == _Construct_strategy::_From_char) { @@ -2711,13 +2716,9 @@ private: if (_Entails_large_string(_Count)) { size_type _New_capacity = _Calculate_growth(_Count); - ++_New_capacity; - const pointer _New_ptr = _Allocate_at_least_helper(_Al, _New_capacity); // throws - --_New_capacity; + const pointer _New_ptr = _Allocate_for_capacity(_Al, _New_capacity); // throws _Construct_in_place(_My_data._Bx._Ptr, _New_ptr); _My_data._Myres = _New_capacity; - - _Start_element_lifetimes(_Unfancy(_New_ptr), _New_capacity + 1); } } @@ -2731,14 +2732,11 @@ private: _Elem* const _Old_ptr = _My_data._Myptr(); size_type _New_capacity = _Calculate_growth(_My_data._Mysize); - ++_New_capacity; - const pointer _New_ptr = _Allocate_at_least_helper(_Al, _New_capacity); // throws - --_New_capacity; + const pointer _New_ptr = _Allocate_for_capacity(_Al, _New_capacity); // throws - _Start_element_lifetimes(_Unfancy(_New_ptr), _New_capacity + 1); _Traits::copy(_Unfancy(_New_ptr), _Old_ptr, _My_data._Mysize); if (_My_data._Large_string_engaged()) { // Need to deallocate old storage - _Al.deallocate(_My_data._Bx._Ptr, _My_data._Myres + 1); + _Deallocate_for_capacity(_Al, _My_data._Bx._Ptr, _My_data._Myres); _My_data._Bx._Ptr = _New_ptr; } else { _Construct_in_place(_My_data._Bx._Ptr, _New_ptr); @@ -2815,14 +2813,10 @@ public: _Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _My_data); // throws if (_New_capacity < _New_size) { - _New_capacity = _Calculate_growth(_New_size, _SMALL_STRING_CAPACITY, max_size()); - ++_New_capacity; - const pointer _Fancyptr = _Allocate_at_least_helper(_Getal(), _New_capacity); // throws - --_New_capacity; - _Ptr = _Unfancy(_Fancyptr); + _New_capacity = _Calculate_growth(_New_size, _SMALL_STRING_CAPACITY, max_size()); + const pointer _Fancyptr = _Allocate_for_capacity(_Getal(), _New_capacity); // throws + _Ptr = _Unfancy(_Fancyptr); _Construct_in_place(_My_data._Bx._Ptr, _Fancyptr); - - _Start_element_lifetimes(_Ptr, _New_capacity + 1); } _My_data._Mysize = _New_size; @@ -2891,11 +2885,8 @@ public: size_type _New_capacity = _Calculate_growth(_New_size, _SMALL_STRING_CAPACITY, _Max); auto&& _Alproxy = _GET_PROXY_ALLOCATOR(_Alty, _Getal()); _Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _My_data); // throws - ++_New_capacity; - const pointer _Fancyptr = _Allocate_at_least_helper(_Getal(), _New_capacity); // throws - --_New_capacity; + const pointer _Fancyptr = _Allocate_for_capacity(_Getal(), _New_capacity); // throws // nothrow hereafter - _Start_element_lifetimes(_Unfancy(_Fancyptr), _New_capacity + 1); _Construct_in_place(_My_data._Bx._Ptr, _Fancyptr); _My_data._Mysize = _New_size; _My_data._Myres = _New_capacity; @@ -3198,11 +3189,7 @@ public: const auto _New_size = _Right._Mypair._Myval2._Mysize; auto _New_capacity = _Calculate_growth(_New_size, 0, _Right.max_size()); auto _Right_al_non_const = _Right_al; - ++_New_capacity; - const auto _New_ptr = _Allocate_at_least_helper(_Right_al_non_const, _New_capacity); // throws - --_New_capacity; - - _Start_element_lifetimes(_Unfancy(_New_ptr), _New_capacity + 1); + const auto _New_ptr = _Allocate_for_capacity(_Right_al_non_const, _New_capacity); // throws _Traits::copy(_Unfancy(_New_ptr), _Unfancy(_Right._Mypair._Myval2._Bx._Ptr), _New_size + 1); _Tidy_deallocate(); @@ -4025,17 +4012,15 @@ public: return; } - const size_type _Target_capacity = (_STD min)(_My_data._Mysize | _ALLOC_MASK, max_size()); + size_type _Target_capacity = (_STD min)(_My_data._Mysize | _ALLOC_MASK, max_size()); if (_Target_capacity < _My_data._Myres) { // worth shrinking, do it auto& _Al = _Getal(); - const pointer _New_ptr = _Al.allocate(_Target_capacity + 1); // throws + const pointer _New_ptr = _Allocate_for_capacity(_Al, _Target_capacity); // throws _ASAN_STRING_REMOVE(*this); - _Start_element_lifetimes(_Unfancy(_New_ptr), _Target_capacity + 1); - _My_data._Orphan_all(); _Traits::copy(_Unfancy(_New_ptr), _Unfancy(_My_data._Bx._Ptr), _My_data._Mysize + 1); - _Al.deallocate(_My_data._Bx._Ptr, _My_data._Myres + 1); + _Deallocate_for_capacity(_Al, _My_data._Bx._Ptr, _My_data._Myres); _My_data._Bx._Ptr = _New_ptr; _My_data._Myres = _Target_capacity; _ASAN_STRING_CREATE(*this); @@ -4770,18 +4755,15 @@ private: const size_type _Old_capacity = _Mypair._Myval2._Myres; size_type _New_capacity = _Calculate_growth(_New_size); auto& _Al = _Getal(); - ++_New_capacity; - const pointer _New_ptr = _Allocate_at_least_helper(_Al, _New_capacity); // throws - --_New_capacity; + const pointer _New_ptr = _Allocate_for_capacity(_Al, _New_capacity); // throws - _Start_element_lifetimes(_Unfancy(_New_ptr), _New_capacity + 1); _Mypair._Myval2._Orphan_all(); _ASAN_STRING_REMOVE(*this); _Mypair._Myval2._Mysize = _New_size; _Mypair._Myval2._Myres = _New_capacity; _Fn(_Unfancy(_New_ptr), _New_size, _Args...); if (_Entails_large_string(_Old_capacity)) { - _Al.deallocate(_Mypair._Myval2._Bx._Ptr, _Old_capacity + 1); + _Deallocate_for_capacity(_Al, _Mypair._Myval2._Bx._Ptr, _Old_capacity); _Mypair._Myval2._Bx._Ptr = _New_ptr; } else { _Construct_in_place(_Mypair._Myval2._Bx._Ptr, _New_ptr); @@ -4805,11 +4787,8 @@ private: const size_type _Old_capacity = _My_data._Myres; size_type _New_capacity = _Calculate_growth(_New_size); auto& _Al = _Getal(); - ++_New_capacity; - const pointer _New_ptr = _Allocate_at_least_helper(_Al, _New_capacity); // throws - --_New_capacity; + const pointer _New_ptr = _Allocate_for_capacity(_Al, _New_capacity); // throws - _Start_element_lifetimes(_Unfancy(_New_ptr), _New_capacity + 1); _My_data._Orphan_all(); _ASAN_STRING_REMOVE(*this); _My_data._Mysize = _New_size; @@ -4818,7 +4797,7 @@ private: if (_Entails_large_string(_Old_capacity)) { const pointer _Old_ptr = _My_data._Bx._Ptr; _Fn(_Raw_new, _Unfancy(_Old_ptr), _Old_size, _Args...); - _Al.deallocate(_Old_ptr, _Old_capacity + 1); + _Deallocate_for_capacity(_Al, _Old_ptr, _Old_capacity); _My_data._Bx._Ptr = _New_ptr; } else { _Fn(_Raw_new, _My_data._Bx._Buf, _Old_size, _Args...); @@ -4842,7 +4821,7 @@ private: _Destroy_in_place(_My_data._Bx._Ptr); _My_data._Activate_SSO_buffer(); _Traits::copy(_My_data._Bx._Buf, _Unfancy(_Ptr), _My_data._Mysize + 1); - _Al.deallocate(_Ptr, _My_data._Myres + 1); + _Deallocate_for_capacity(_Al, _Ptr, _My_data._Myres); _My_data._Myres = _SMALL_STRING_CAPACITY; } @@ -4871,7 +4850,7 @@ private: auto& _Al = _Getal(); _Destroy_in_place(_My_data._Bx._Ptr); _My_data._Activate_SSO_buffer(); - _Al.deallocate(_Ptr, _My_data._Myres + 1); + _Deallocate_for_capacity(_Al, _Ptr, _My_data._Myres); } _My_data._Mysize = 0; From 5ba8339d37a87b6815152c8a872b51147549c1c5 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Thu, 27 Jul 2023 15:54:51 -0700 Subject: [PATCH 13/26] Fix a misuse of `_Large_string_engaged` (which will otherwise break the invariant that string won't do allocations for small capacity) --- stl/inc/xstring | 42 +++++++++++------------------------------- 1 file changed, 11 insertions(+), 31 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index ca3435d6298..cf98c87f755 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -3147,30 +3147,6 @@ public: static constexpr auto npos{static_cast(-1)}; -private: - _CONSTEXPR20 void _Copy_assign_val_from_small(const basic_string& _Right) { - // TRANSITION, VSO-761321; inline into only caller when that's fixed - _Tidy_deallocate(); -#if !defined(_INSERT_STRING_ANNOTATION) - if constexpr (_Can_memcpy_val) { -#if _HAS_CXX20 - if (!_STD is_constant_evaluated()) -#endif // _HAS_CXX20 - { - _Memcpy_val_from(_Right); - return; - } - } -#endif // !defined(_INSERT_STRING_ANNOTATION) - - auto& _My_data = _Mypair._Myval2; - auto& _Right_data = _Right._Mypair._Myval2; - - _Traits::copy(_My_data._Bx._Buf, _Right_data._Bx._Buf, _Right_data._Mysize + 1); - _My_data._Mysize = _Right_data._Mysize; - _My_data._Myres = _Right_data._Myres; - } - public: _CONSTEXPR20 basic_string& operator=(const basic_string& _Right) { if (this == _STD addressof(_Right)) { @@ -3185,19 +3161,23 @@ public: auto&& _Right_alproxy = _GET_PROXY_ALLOCATOR(_Alty, _Right_al); _Container_proxy_ptr<_Alty> _New_proxy(_Right_alproxy, _Leave_proxy_unbound{}); // throws - if (_Right._Mypair._Myval2._Large_string_engaged()) { - const auto _New_size = _Right._Mypair._Myval2._Mysize; - auto _New_capacity = _Calculate_growth(_New_size, 0, _Right.max_size()); + const size_type _Right_size = _Right._Mypair._Myval2._Mysize; + const _Elem* const _Right_ptr = _Right._Mypair._Myval2._Myptr(); + if (_Entails_large_string(_Right_size)) { + size_type _New_capacity = _Calculate_growth(_Right_size, _SMALL_STRING_CAPACITY, _Right.max_size()); auto _Right_al_non_const = _Right_al; - const auto _New_ptr = _Allocate_for_capacity(_Right_al_non_const, _New_capacity); // throws + const pointer _New_ptr = _Allocate_for_capacity(_Right_al_non_const, _New_capacity); // throws - _Traits::copy(_Unfancy(_New_ptr), _Unfancy(_Right._Mypair._Myval2._Bx._Ptr), _New_size + 1); + _Traits::copy(_Unfancy(_New_ptr), _Right_ptr, _Right_size + 1); _Tidy_deallocate(); _Mypair._Myval2._Bx._Ptr = _New_ptr; - _Mypair._Myval2._Mysize = _New_size; + _Mypair._Myval2._Mysize = _Right_size; _Mypair._Myval2._Myres = _New_capacity; } else { - _Copy_assign_val_from_small(_Right); + _Tidy_deallocate(); + _Traits::copy(_Mypair._Myval2._Bx._Buf, _Right_ptr, _Right_size + 1); + _Mypair._Myval2._Mysize = _Right_size; + _Mypair._Myval2._Myres = _SMALL_STRING_CAPACITY; } _Pocca(_Al, _Right_al); From 9b729dee02f21fe47777b9897d7668aa55bf6bc1 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 27 Jul 2023 16:31:31 -0700 Subject: [PATCH 14/26] Serially run the tests in Dev09_181509_tr1_inf_loop_uniform_int_ull. --- .../Dev09_181509_tr1_inf_loop_uniform_int_ull/env.lst | 2 +- .../Dev09_181509_tr1_inf_loop_uniform_int_ull/test.cpp | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/std/tests/Dev09_181509_tr1_inf_loop_uniform_int_ull/env.lst b/tests/std/tests/Dev09_181509_tr1_inf_loop_uniform_int_ull/env.lst index 2de7aab2959..19f025bd0e6 100644 --- a/tests/std/tests/Dev09_181509_tr1_inf_loop_uniform_int_ull/env.lst +++ b/tests/std/tests/Dev09_181509_tr1_inf_loop_uniform_int_ull/env.lst @@ -1,4 +1,4 @@ # Copyright (c) Microsoft Corporation. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -RUNALL_INCLUDE ..\usual_17_matrix.lst +RUNALL_INCLUDE ..\usual_matrix.lst diff --git a/tests/std/tests/Dev09_181509_tr1_inf_loop_uniform_int_ull/test.cpp b/tests/std/tests/Dev09_181509_tr1_inf_loop_uniform_int_ull/test.cpp index ce2e1988e75..134b48978ca 100644 --- a/tests/std/tests/Dev09_181509_tr1_inf_loop_uniform_int_ull/test.cpp +++ b/tests/std/tests/Dev09_181509_tr1_inf_loop_uniform_int_ull/test.cpp @@ -1,16 +1,13 @@ // Copyright (c) Microsoft Corporation. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -#include #include #include -#include #include #include #include using namespace std; -using namespace std::execution; constexpr auto int16_min = numeric_limits::min(); constexpr auto int16_max = numeric_limits::max(); @@ -121,13 +118,13 @@ class Wacky { }; int main() { -#ifndef _M_CEE // TRANSITION, VSO-1659695 vector tests; add_tests(tests); add_tests(tests); add_tests(tests); - for_each(par, tests.begin(), tests.end(), [](fp_t fn) { fn(); }); -#endif // _M_CEE + for (const auto& fn : tests) { + fn(); + } } From 2b017dbff4bb1b26f2056849ba0dae364ab5f497 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 27 Jul 2023 17:12:07 -0700 Subject: [PATCH 15/26] Fix comment grammar. --- stl/inc/xstring | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index cf98c87f755..514b4cda79b 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2294,7 +2294,7 @@ public: _Bxty _Bx; // invariant: _Myres >= _Mysize, and _Myres >= _SMALL_STRING_CAPACITY (after string's construction) - // both _Mysize and _Myres doesn't take account of the extra null terminator + // neither _Mysize nor _Myres takes account of the extra null terminator size_type _Mysize = 0; // current length of string (size) size_type _Myres = 0; // current storage reserved for string (capacity) }; From e600d39d3807af9272cbbbc2705b35bf79554dc3 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 27 Jul 2023 17:24:17 -0700 Subject: [PATCH 16/26] In `_Deallocate_for_capacity`, pass `_Old_ptr` by value. --- stl/inc/xstring | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 514b4cda79b..19a8dd9060c 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2625,7 +2625,7 @@ private: } static _CONSTEXPR20 void _Deallocate_for_capacity( - _Alty& _Al, const pointer& _Old_ptr, const size_type _Capacity) noexcept { + _Alty& _Al, const pointer _Old_ptr, const size_type _Capacity) noexcept { _STL_INTERNAL_CHECK(_Entails_large_string(_Capacity)); _Al.deallocate(_Old_ptr, _Capacity + 1); // +1 for null terminator } From 6f8a3cbf4dfe8172d70f47a3aaf78cf801019de5 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 27 Jul 2023 17:49:28 -0700 Subject: [PATCH 17/26] Drop redundant `public:` after the removal of `_Copy_assign_val_from_small`. --- stl/inc/xstring | 1 - 1 file changed, 1 deletion(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 19a8dd9060c..f161000e470 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -3147,7 +3147,6 @@ public: static constexpr auto npos{static_cast(-1)}; -public: _CONSTEXPR20 basic_string& operator=(const basic_string& _Right) { if (this == _STD addressof(_Right)) { return *this; From 0baefec15143ed9320ec7957c547b48c86f6bbc3 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 27 Jul 2023 18:35:14 -0700 Subject: [PATCH 18/26] Add `test_string_copy_assign_pocca_sso()`. --- .../test.cpp | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/std/tests/VSO_0000000_allocator_propagation/test.cpp b/tests/std/tests/VSO_0000000_allocator_propagation/test.cpp index fb2a8b15466..d697fc595a2 100644 --- a/tests/std/tests/VSO_0000000_allocator_propagation/test.cpp +++ b/tests/std/tests/VSO_0000000_allocator_propagation/test.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -635,6 +636,31 @@ void test_string_copy_assign(const size_t id1, const size_t id2, const size_t id assert(dst.get_allocator().id() == id3); } +void test_string_copy_assign_pocca_sso() { + // GH-3862 fixed a bug where the POCCA codepath in basic_string's copy assignment operator mishandled + // the scenario where the string on the right hand side has a large capacity but a small size - so while + // the RHS has dynamically allocated memory, the LHS should activate the Small String Optimization. + + using Al = CopyAlloc; + using Str = basic_string, Al>; + + Str left{Al{11}}; + Str right{Al{22}}; + + left.assign(5, 'a'); + + right.assign(1729, 'x'); + right.assign(7, 'y'); + + assert(left == "aaaaa"); + assert(right == "yyyyyyy"); + + left = right; + + assert(left == "yyyyyyy"); + assert(right == "yyyyyyy"); +} + void test_string_move_ctor() { basic_string, StationaryAlloc> src( {5, 10, 20, 30}, StationaryAlloc(11)); @@ -770,6 +796,8 @@ void test_string() { test_string_copy_assign>(11, 22, 11); // POCCA, non-equal allocators test_string_copy_assign>(11, 22, 11); // POCCA, always-equal allocators + test_string_copy_assign_pocca_sso(); + test_string_move_ctor(); test_string_move_alloc_ctor(11, 11); // equal allocators From 9b0e5906f68481e733ffcdee78db9ab11dc93d10 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 27 Jul 2023 18:45:21 -0700 Subject: [PATCH 19/26] Restore `shrink_to_fit` to allocate exactly. --- stl/inc/xstring | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index f161000e470..de80f3d711a 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2606,10 +2606,21 @@ public: } private: + enum class _Reallocation_policy { _At_least, _Exactly }; + + template <_Reallocation_policy _Policy = _Reallocation_policy::_At_least> _NODISCARD static _CONSTEXPR20 pointer _Allocate_for_capacity(_Alty& _Al, size_type& _Capacity) { _STL_INTERNAL_CHECK(_Entails_large_string(_Capacity)); ++_Capacity; // Take null terminator into consideration - const pointer _Fancy_ptr = _Allocate_at_least_helper(_Al, _Capacity); + + pointer _Fancy_ptr; + if constexpr (_Policy == _Reallocation_policy::_At_least) { + _Fancy_ptr = _Allocate_at_least_helper(_Al, _Capacity); + } else { + _STL_INTERNAL_STATIC_ASSERT(_Policy == _Reallocation_policy::_Exactly); + _Fancy_ptr = _Al.allocate(_Capacity); + } + #if _HAS_CXX20 // Start element lifetimes to avoid UB. This is a more general mechanism than _String_val::_Activate_SSO_buffer, // but likely more impactful to throughput. @@ -3993,8 +4004,9 @@ public: size_type _Target_capacity = (_STD min)(_My_data._Mysize | _ALLOC_MASK, max_size()); if (_Target_capacity < _My_data._Myres) { // worth shrinking, do it - auto& _Al = _Getal(); - const pointer _New_ptr = _Allocate_for_capacity(_Al, _Target_capacity); // throws + auto& _Al = _Getal(); + const pointer _New_ptr = + _Allocate_for_capacity<_Reallocation_policy::_Exactly>(_Al, _Target_capacity); // throws _ASAN_STRING_REMOVE(*this); _My_data._Orphan_all(); From 3a27c907ea4d8258527f3a138a1a282ec16ed385 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 27 Jul 2023 20:38:17 -0700 Subject: [PATCH 20/26] `_Reallocation_policy` => `_Allocation_policy` --- stl/inc/xstring | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index de80f3d711a..28ca013ae43 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2606,18 +2606,18 @@ public: } private: - enum class _Reallocation_policy { _At_least, _Exactly }; + enum class _Allocation_policy { _At_least, _Exactly }; - template <_Reallocation_policy _Policy = _Reallocation_policy::_At_least> + template <_Allocation_policy _Policy = _Allocation_policy::_At_least> _NODISCARD static _CONSTEXPR20 pointer _Allocate_for_capacity(_Alty& _Al, size_type& _Capacity) { _STL_INTERNAL_CHECK(_Entails_large_string(_Capacity)); ++_Capacity; // Take null terminator into consideration pointer _Fancy_ptr; - if constexpr (_Policy == _Reallocation_policy::_At_least) { + if constexpr (_Policy == _Allocation_policy::_At_least) { _Fancy_ptr = _Allocate_at_least_helper(_Al, _Capacity); } else { - _STL_INTERNAL_STATIC_ASSERT(_Policy == _Reallocation_policy::_Exactly); + _STL_INTERNAL_STATIC_ASSERT(_Policy == _Allocation_policy::_Exactly); _Fancy_ptr = _Al.allocate(_Capacity); } @@ -4006,7 +4006,7 @@ public: if (_Target_capacity < _My_data._Myres) { // worth shrinking, do it auto& _Al = _Getal(); const pointer _New_ptr = - _Allocate_for_capacity<_Reallocation_policy::_Exactly>(_Al, _Target_capacity); // throws + _Allocate_for_capacity<_Allocation_policy::_Exactly>(_Al, _Target_capacity); // throws _ASAN_STRING_REMOVE(*this); _My_data._Orphan_all(); From 6ea7f29f5eeb05240a57169e38872e8fd0fe4579 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Wed, 9 Aug 2023 13:23:53 -0700 Subject: [PATCH 21/26] Rename to avoid macro identifier space Rename `_ALLOC_MASK`, `_SMALL_STRING_CAPACITY` and `_LEAST_ALLOCATION_SIZE` to avoid the macro identifier namespace. Comment that `_BUF_SIZE` is used by the debugger visualizer (and hence shouldn't be renamed). --- stl/inc/xstring | 56 ++++++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 28ca013ae43..b90b7fff6cd 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2212,16 +2212,16 @@ public: _CONSTEXPR20 _String_val() noexcept : _Bx() {} - // length of internal buffer, [1, 16]: + // length of internal buffer, [1, 16] (NB: used by the debugger visualizer) static constexpr size_type _BUF_SIZE = 16 / sizeof(value_type) < 1 ? 1 : 16 / sizeof(value_type); - // roundup mask for allocated buffers, [0, 15]: - static constexpr size_type _ALLOC_MASK = sizeof(value_type) <= 1 ? 15 + // roundup mask for allocated buffers, [0, 15] + static constexpr size_type _Alloc_mask = sizeof(value_type) <= 1 ? 15 : sizeof(value_type) <= 2 ? 7 : sizeof(value_type) <= 4 ? 3 : sizeof(value_type) <= 8 ? 1 : 0; // capacity in SSO mode - static constexpr size_type _SMALL_STRING_CAPACITY = _BUF_SIZE - 1; + static constexpr size_type _Small_string_capacity = _BUF_SIZE - 1; _NODISCARD _CONSTEXPR20 value_type* _Myptr() noexcept { value_type* _Result = _Bx._Buf; @@ -2244,7 +2244,7 @@ public: _NODISCARD _CONSTEXPR20 bool _Large_string_engaged() const noexcept { // Determines whether large mode is used for the string's capacity. // Doesn't imply anything about the largeness of the string's size. - return _Myres > _SMALL_STRING_CAPACITY; + return _Myres > _Small_string_capacity; } _CONSTEXPR20 void _Activate_SSO_buffer() noexcept { @@ -2293,7 +2293,7 @@ public: }; _Bxty _Bx; - // invariant: _Myres >= _Mysize, and _Myres >= _SMALL_STRING_CAPACITY (after string's construction) + // invariant: _Myres >= _Mysize, and _Myres >= _Small_string_capacity (after string's construction) // neither _Mysize nor _Myres takes account of the extra null terminator size_type _Mysize = 0; // current length of string (size) size_type _Myres = 0; // current storage reserved for string (capacity) @@ -2367,19 +2367,19 @@ public: private: static constexpr size_type _BUF_SIZE = _Scary_val::_BUF_SIZE; - static constexpr size_type _ALLOC_MASK = _Scary_val::_ALLOC_MASK; - static constexpr size_type _SMALL_STRING_CAPACITY = _Scary_val::_SMALL_STRING_CAPACITY; + static constexpr size_type _Alloc_mask = _Scary_val::_Alloc_mask; + static constexpr size_type _Small_string_capacity = _Scary_val::_Small_string_capacity; _NODISCARD static _CONSTEXPR20 bool _Entails_large_string(size_type _Size_or_capacity) noexcept { // Use cases: // 1. determine whether large mode is used for a given capacity. // 2. determine whether large mode should be used for a size/capacity request. - return _Size_or_capacity > _SMALL_STRING_CAPACITY; + return _Size_or_capacity > _Small_string_capacity; } // least real allocation size, including space reserved for terminating null // used exclusively by _Move_assign_from_buffer and _Release_to_buffer - static constexpr size_type _LEAST_ALLOCATION_SIZE = _SMALL_STRING_CAPACITY + 1 + 1; + static constexpr size_type _Least_allocation_size = _Small_string_capacity + 1 + 1; // When doing _String_val operations by memcpy, we are touching: // _String_val::_Bx::_Buf (type is array of _Elem) @@ -2664,7 +2664,7 @@ private: if (!_Entails_large_string(_Count)) { _My_data._Mysize = _Count; - _My_data._Myres = _SMALL_STRING_CAPACITY; + _My_data._Myres = _Small_string_capacity; if constexpr (_Strat == _Construct_strategy::_From_char) { _Traits::assign(_My_data._Bx._Buf, _Count, _Arg); @@ -2684,7 +2684,7 @@ private: return; } - _My_data._Myres = _SMALL_STRING_CAPACITY; + _My_data._Myres = _Small_string_capacity; size_type _New_capacity = _Calculate_growth(_Count); const pointer _New_ptr = _Allocate_for_capacity(_Al, _New_capacity); // throws _Construct_in_place(_My_data._Bx._Ptr, _New_ptr); @@ -2718,7 +2718,7 @@ private: _Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _My_data); _My_data._Mysize = 0; - _My_data._Myres = _SMALL_STRING_CAPACITY; + _My_data._Myres = _Small_string_capacity; if constexpr (is_same_v<_Size, size_type>) { if (_Count > max_size()) { @@ -2817,14 +2817,14 @@ public: _STL_INTERNAL_CHECK(_Right_size <= max_size()); _STL_INTERNAL_CHECK(_Right_size <= max_size() - _Left_size); const auto _New_size = static_cast(_Left_size + _Right_size); - size_type _New_capacity = _SMALL_STRING_CAPACITY; + size_type _New_capacity = _Small_string_capacity; auto& _My_data = _Mypair._Myval2; _Elem* _Ptr = _My_data._Bx._Buf; auto&& _Alproxy = _GET_PROXY_ALLOCATOR(_Alty, _Getal()); _Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _My_data); // throws if (_New_capacity < _New_size) { - _New_capacity = _Calculate_growth(_New_size, _SMALL_STRING_CAPACITY, max_size()); + _New_capacity = _Calculate_growth(_New_size, _Small_string_capacity, max_size()); const pointer _Fancyptr = _Allocate_for_capacity(_Getal(), _New_capacity); // throws _Ptr = _Unfancy(_Fancyptr); _Construct_in_place(_My_data._Bx._Ptr, _Fancyptr); @@ -2893,7 +2893,7 @@ public: _Xlen_string(); } - size_type _New_capacity = _Calculate_growth(_New_size, _SMALL_STRING_CAPACITY, _Max); + size_type _New_capacity = _Calculate_growth(_New_size, _Small_string_capacity, _Max); auto&& _Alproxy = _GET_PROXY_ALLOCATOR(_Alty, _Getal()); _Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _My_data); // throws const pointer _Fancyptr = _Allocate_for_capacity(_Getal(), _New_capacity); // throws @@ -2942,7 +2942,7 @@ public: _STL_INTERNAL_CHECK(_Size < _Buf_res); // So there is room for null terminator _Traits::assign(_Right[_Size], _Elem()); - const bool _Is_large = _Buf_res >= _LEAST_ALLOCATION_SIZE; + const bool _Is_large = _Buf_res >= _Least_allocation_size; if (_Is_large) { _ASAN_STRING_REMOVE(*this); _Construct_in_place(_My_data._Bx._Ptr, _Refancy(_Right)); @@ -2952,7 +2952,7 @@ public: } else { _Traits::copy(_My_data._Bx._Buf, _Right, _Buf_res); _My_data._Mysize = _Size; - _My_data._Myres = _SMALL_STRING_CAPACITY; + _My_data._Myres = _Small_string_capacity; } return _Is_large; @@ -2975,8 +2975,8 @@ public: _Result._Ptr = _My_data._Bx._Ptr; _Result._Res = _My_data._Myres + 1; } else { - // use _LEAST_ALLOCATION_SIZE to avoid SSO, if the buffer is assigned back - size_type _Allocated = _LEAST_ALLOCATION_SIZE; + // use _Least_allocation_size to avoid SSO, if the buffer is assigned back + size_type _Allocated = _Least_allocation_size; _Result._Ptr = _Allocate_at_least_helper(_Al, _Allocated); _Traits::copy(_Unfancy(_Result._Ptr), _My_data._Bx._Buf, _BUF_SIZE); _Result._Res = _Allocated; @@ -3174,7 +3174,7 @@ public: const size_type _Right_size = _Right._Mypair._Myval2._Mysize; const _Elem* const _Right_ptr = _Right._Mypair._Myval2._Myptr(); if (_Entails_large_string(_Right_size)) { - size_type _New_capacity = _Calculate_growth(_Right_size, _SMALL_STRING_CAPACITY, _Right.max_size()); + size_type _New_capacity = _Calculate_growth(_Right_size, _Small_string_capacity, _Right.max_size()); auto _Right_al_non_const = _Right_al; const pointer _New_ptr = _Allocate_for_capacity(_Right_al_non_const, _New_capacity); // throws @@ -3187,7 +3187,7 @@ public: _Tidy_deallocate(); _Traits::copy(_Mypair._Myval2._Bx._Buf, _Right_ptr, _Right_size + 1); _Mypair._Myval2._Mysize = _Right_size; - _Mypair._Myval2._Myres = _SMALL_STRING_CAPACITY; + _Mypair._Myval2._Myres = _Small_string_capacity; } _Pocca(_Al, _Right_al); @@ -4002,7 +4002,7 @@ public: return; } - size_type _Target_capacity = (_STD min)(_My_data._Mysize | _ALLOC_MASK, max_size()); + size_type _Target_capacity = (_STD min)(_My_data._Mysize | _Alloc_mask, max_size()); if (_Target_capacity < _My_data._Myres) { // worth shrinking, do it auto& _Al = _Getal(); const pointer _New_ptr = @@ -4228,7 +4228,7 @@ public: return; } - // ignore requests to reserve to [_SMALL_STRING_CAPACITY + 1, _Myres) + // ignore requests to reserve to [_Small_string_capacity + 1, _Myres) } #endif // _HAS_CXX20 @@ -4719,7 +4719,7 @@ public: private: _NODISCARD static _CONSTEXPR20 size_type _Calculate_growth( const size_type _Requested, const size_type _Old, const size_type _Max) noexcept { - const size_type _Masked = _Requested | _ALLOC_MASK; + const size_type _Masked = _Requested | _Alloc_mask; if (_Masked > _Max) { // the mask overflows, settle for max_size() return _Max; } @@ -4813,7 +4813,7 @@ private: _My_data._Activate_SSO_buffer(); _Traits::copy(_My_data._Bx._Buf, _Unfancy(_Ptr), _My_data._Mysize + 1); _Deallocate_for_capacity(_Al, _Ptr, _My_data._Myres); - _My_data._Myres = _SMALL_STRING_CAPACITY; + _My_data._Myres = _Small_string_capacity; } _CONSTEXPR20 void _Eos(const size_type _New_size) noexcept { // set new length and null terminator @@ -4825,7 +4825,7 @@ private: // initialize basic_string data members auto& _My_data = _Mypair._Myval2; _My_data._Mysize = 0; - _My_data._Myres = _SMALL_STRING_CAPACITY; + _My_data._Myres = _Small_string_capacity; _My_data._Activate_SSO_buffer(); // the _Traits::assign is last so the codegen doesn't think the char write can alias this @@ -4845,7 +4845,7 @@ private: } _My_data._Mysize = 0; - _My_data._Myres = _SMALL_STRING_CAPACITY; + _My_data._Myres = _Small_string_capacity; // the _Traits::assign is last so the codegen doesn't think the char write can alias this _Traits::assign(_My_data._Bx._Buf[0], _Elem()); } From a2cb72baa063e26941d99eca98d86ac42fd67a5e Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Wed, 9 Aug 2023 12:48:01 -0700 Subject: [PATCH 22/26] Rename `_Large_string_engaged` to `_Large_mode_engaged` ... to avoid confusion about size vs. capacity since `_Large_string_engaged()` can be `true` for a very small string with a large capacity. --- stl/inc/xstring | 44 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index b90b7fff6cd..d29fc0c5bad 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2225,7 +2225,7 @@ public: _NODISCARD _CONSTEXPR20 value_type* _Myptr() noexcept { value_type* _Result = _Bx._Buf; - if (_Large_string_engaged()) { + if (_Large_mode_engaged()) { _Result = _Unfancy(_Bx._Ptr); } @@ -2234,16 +2234,14 @@ public: _NODISCARD _CONSTEXPR20 const value_type* _Myptr() const noexcept { const value_type* _Result = _Bx._Buf; - if (_Large_string_engaged()) { + if (_Large_mode_engaged()) { _Result = _Unfancy(_Bx._Ptr); } return _Result; } - _NODISCARD _CONSTEXPR20 bool _Large_string_engaged() const noexcept { - // Determines whether large mode is used for the string's capacity. - // Doesn't imply anything about the largeness of the string's size. + _NODISCARD _CONSTEXPR20 bool _Large_mode_engaged() const noexcept { return _Myres > _Small_string_capacity; } @@ -2646,7 +2644,7 @@ private: template <_Construct_strategy _Strat, class _Char_or_ptr> _CONSTEXPR20 void _Construct(const _Char_or_ptr _Arg, _CRT_GUARDOVERFLOW const size_type _Count) { auto& _My_data = _Mypair._Myval2; - _STL_INTERNAL_CHECK(!_My_data._Large_string_engaged()); + _STL_INTERNAL_CHECK(!_My_data._Large_mode_engaged()); if constexpr (_Strat == _Construct_strategy::_From_char) { _STL_INTERNAL_STATIC_ASSERT(is_same_v<_Char_or_ptr, _Elem>); @@ -2746,7 +2744,7 @@ private: const pointer _New_ptr = _Allocate_for_capacity(_Al, _New_capacity); // throws _Traits::copy(_Unfancy(_New_ptr), _Old_ptr, _My_data._Mysize); - if (_My_data._Large_string_engaged()) { // Need to deallocate old storage + if (_My_data._Large_mode_engaged()) { // Need to deallocate old storage _Deallocate_for_capacity(_Al, _My_data._Bx._Ptr, _My_data._Myres); _My_data._Bx._Ptr = _New_ptr; } else { @@ -2875,8 +2873,8 @@ public: // therefore: (by the distributive property) // (!_Fits_in_left && _Fits_in_right) // implying _Right has more capacity // || (_Right_capacity > _Left_capacity && _Fits_in_right) // tests that _Right has more capacity - // therefore: _Right must have more than the minimum capacity, so it must be _Large_string_engaged() - _STL_INTERNAL_CHECK(_Right_data._Large_string_engaged()); + // therefore: _Right must have more than the minimum capacity, so it must be _Large_mode_engaged() + _STL_INTERNAL_CHECK(_Right_data._Large_mode_engaged()); _My_data._Alloc_proxy(_GET_PROXY_ALLOCATOR(_Alty, _Getal())); // throws, hereafter nothrow in this block _Take_contents(_Right); const auto _Ptr = _Unfancy(_My_data._Bx._Ptr); @@ -2936,9 +2934,9 @@ public: _NODISCARD bool _Move_assign_from_buffer( _Elem* const _Right, const size_type _Size, const size_type _Buf_res /* real allocation size */) { - // Move assign from a buffer, used exclusively by basic_stringbuf; returns _Large_string_engaged() + // Move assign from a buffer, used exclusively by basic_stringbuf; returns _Large_mode_engaged() auto& _My_data = _Mypair._Myval2; - _STL_INTERNAL_CHECK(!_My_data._Large_string_engaged() && _My_data._Mysize == 0); + _STL_INTERNAL_CHECK(!_My_data._Large_mode_engaged() && _My_data._Mysize == 0); _STL_INTERNAL_CHECK(_Size < _Buf_res); // So there is room for null terminator _Traits::assign(_Right[_Size], _Elem()); @@ -2971,7 +2969,7 @@ public: auto& _My_data = _Mypair._Myval2; _Result._Size = _My_data._Mysize; _ASAN_STRING_REMOVE(*this); - if (_My_data._Large_string_engaged()) { + if (_My_data._Large_mode_engaged()) { _Result._Ptr = _My_data._Bx._Ptr; _Result._Res = _My_data._Myres + 1; } else { @@ -3046,7 +3044,7 @@ private: #endif // _HAS_CXX20 { #if _ITERATOR_DEBUG_LEVEL != 0 - if (_Right_data._Large_string_engaged()) { + if (_Right_data._Large_mode_engaged()) { // take ownership of _Right's iterators along with its buffer _Swap_proxy_and_iterators(_Right); } else { @@ -3061,7 +3059,7 @@ private: } #endif // !defined(_INSERT_STRING_ANNOTATION) - if (_Right_data._Large_string_engaged()) { // steal buffer + if (_Right_data._Large_mode_engaged()) { // steal buffer _Construct_in_place(_My_data._Bx._Ptr, _Right_data._Bx._Ptr); _Right_data._Bx._Ptr = nullptr; _Swap_proxy_and_iterators(_Right); @@ -3993,7 +3991,7 @@ public: _CONSTEXPR20 void shrink_to_fit() { // reduce capacity auto& _My_data = _Mypair._Myval2; - if (!_My_data._Large_string_engaged()) { // can't shrink from small mode + if (!_My_data._Large_mode_engaged()) { // can't shrink from small mode return; } @@ -4197,7 +4195,7 @@ public: } _CXX20_DEPRECATE_STRING_RESERVE_WITHOUT_ARGUMENT void reserve() { - if (_Mypair._Myval2._Mysize == 0 && _Mypair._Myval2._Large_string_engaged()) { + if (_Mypair._Myval2._Mysize == 0 && _Mypair._Myval2._Large_mode_engaged()) { _Become_small(); } } @@ -4222,7 +4220,7 @@ public: return; } - if (!_Entails_large_string(_Newcap) && _Mypair._Myval2._Large_string_engaged()) { + if (!_Entails_large_string(_Newcap) && _Mypair._Myval2._Large_mode_engaged()) { // deallocate everything; switch back to "small" mode _Become_small(); return; @@ -4272,8 +4270,8 @@ public: auto& _My_data = _Mypair._Myval2; auto& _Right_data = _Right._Mypair._Myval2; - const bool _My_large = _My_data._Large_string_engaged(); - const bool _Right_large = _Right_data._Large_string_engaged(); + const bool _My_large = _My_data._Large_mode_engaged(); + const bool _Right_large = _Right_data._Large_mode_engaged(); #if !defined(_INSERT_STRING_ANNOTATION) if constexpr (_Can_memcpy_val) { @@ -4320,11 +4318,11 @@ public: auto& _My_data = _Mypair._Myval2; auto& _Right_data = _Right._Mypair._Myval2; - if (!_My_data._Large_string_engaged()) { + if (!_My_data._Large_mode_engaged()) { _My_data._Orphan_all(); } - if (!_Right_data._Large_string_engaged()) { + if (!_Right_data._Large_mode_engaged()) { _Right_data._Orphan_all(); } @@ -4802,7 +4800,7 @@ private: _CONSTEXPR20 void _Become_small() { // release any held storage and return to small string mode auto& _My_data = _Mypair._Myval2; - _STL_INTERNAL_CHECK(_My_data._Large_string_engaged()); + _STL_INTERNAL_CHECK(_My_data._Large_mode_engaged()); _STL_INTERNAL_CHECK(!_Entails_large_string(_My_data._Mysize)); _My_data._Orphan_all(); @@ -4835,7 +4833,7 @@ private: _CONSTEXPR20 void _Tidy_deallocate() noexcept { // initialize buffer, deallocating any storage auto& _My_data = _Mypair._Myval2; _My_data._Orphan_all(); - if (_My_data._Large_string_engaged()) { + if (_My_data._Large_mode_engaged()) { _ASAN_STRING_REMOVE(*this); const pointer _Ptr = _My_data._Bx._Ptr; auto& _Al = _Getal(); From 9921debb85538a09d689c0440fa42c0f983a7b8e Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Wed, 9 Aug 2023 13:27:41 -0700 Subject: [PATCH 23/26] Expand `_Entails_large_string` into callsites With the addition of `_SMALL_STRING_CAPACITY`, I think the expressions themselves (e.g., `_Count > _SMALL_STRING_CAPACITY`) are more readable than calls to the function. --- stl/inc/xstring | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index d29fc0c5bad..161f02e0ef9 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2368,13 +2368,6 @@ private: static constexpr size_type _Alloc_mask = _Scary_val::_Alloc_mask; static constexpr size_type _Small_string_capacity = _Scary_val::_Small_string_capacity; - _NODISCARD static _CONSTEXPR20 bool _Entails_large_string(size_type _Size_or_capacity) noexcept { - // Use cases: - // 1. determine whether large mode is used for a given capacity. - // 2. determine whether large mode should be used for a size/capacity request. - return _Size_or_capacity > _Small_string_capacity; - } - // least real allocation size, including space reserved for terminating null // used exclusively by _Move_assign_from_buffer and _Release_to_buffer static constexpr size_type _Least_allocation_size = _Small_string_capacity + 1 + 1; @@ -2437,7 +2430,7 @@ private: } #endif // _HAS_CXX20 // Don't annotate small strings; only annotate on the heap. - if (!_Entails_large_string(_Capacity) || !_Asan_string_should_annotate) { + if (_Capacity <= _Small_string_capacity || !_Asan_string_should_annotate) { return; } @@ -2608,7 +2601,7 @@ private: template <_Allocation_policy _Policy = _Allocation_policy::_At_least> _NODISCARD static _CONSTEXPR20 pointer _Allocate_for_capacity(_Alty& _Al, size_type& _Capacity) { - _STL_INTERNAL_CHECK(_Entails_large_string(_Capacity)); + _STL_INTERNAL_CHECK(_Capacity > _Small_string_capacity); ++_Capacity; // Take null terminator into consideration pointer _Fancy_ptr; @@ -2635,7 +2628,7 @@ private: static _CONSTEXPR20 void _Deallocate_for_capacity( _Alty& _Al, const pointer _Old_ptr, const size_type _Capacity) noexcept { - _STL_INTERNAL_CHECK(_Entails_large_string(_Capacity)); + _STL_INTERNAL_CHECK(_Capacity > _Small_string_capacity); _Al.deallocate(_Old_ptr, _Capacity + 1); // +1 for null terminator } @@ -2660,7 +2653,7 @@ private: auto&& _Alproxy = _GET_PROXY_ALLOCATOR(_Alty, _Al); _Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _My_data); - if (!_Entails_large_string(_Count)) { + if (_Count <= _Small_string_capacity) { _My_data._Mysize = _Count; _My_data._Myres = _Small_string_capacity; @@ -2723,7 +2716,7 @@ private: _Xlen_string(); // result too long } - if (_Entails_large_string(_Count)) { + if (_Count > _Small_string_capacity) { size_type _New_capacity = _Calculate_growth(_Count); const pointer _New_ptr = _Allocate_for_capacity(_Al, _New_capacity); // throws _Construct_in_place(_My_data._Bx._Ptr, _New_ptr); @@ -3083,7 +3076,7 @@ private: const auto _Result_size = _Right_data._Clamp_suffix_size(_Roff, _Size_max); const auto _Right_ptr = _Right_data._Myptr(); auto& _Al = _Getal(); - if (_Allocators_equal(_Al, _Right._Getal()) && _Entails_large_string(_Result_size)) { + if (_Allocators_equal(_Al, _Right._Getal()) && _Result_size > _Small_string_capacity) { if (_Roff != 0) { _Traits::move(_Right_ptr, _Right_ptr + _Roff, _Result_size); } @@ -3171,7 +3164,7 @@ public: const size_type _Right_size = _Right._Mypair._Myval2._Mysize; const _Elem* const _Right_ptr = _Right._Mypair._Myval2._Myptr(); - if (_Entails_large_string(_Right_size)) { + if (_Right_size > _Small_string_capacity) { size_type _New_capacity = _Calculate_growth(_Right_size, _Small_string_capacity, _Right.max_size()); auto _Right_al_non_const = _Right_al; const pointer _New_ptr = _Allocate_for_capacity(_Right_al_non_const, _New_capacity); // throws @@ -3995,7 +3988,7 @@ public: return; } - if (!_Entails_large_string(_My_data._Mysize)) { + if (_My_data._Mysize <= _Small_string_capacity) { _Become_small(); return; } @@ -4220,7 +4213,7 @@ public: return; } - if (!_Entails_large_string(_Newcap) && _Mypair._Myval2._Large_mode_engaged()) { + if (_Newcap <= _Small_string_capacity && _Mypair._Myval2._Large_mode_engaged()) { // deallocate everything; switch back to "small" mode _Become_small(); return; @@ -4751,7 +4744,7 @@ private: _Mypair._Myval2._Mysize = _New_size; _Mypair._Myval2._Myres = _New_capacity; _Fn(_Unfancy(_New_ptr), _New_size, _Args...); - if (_Entails_large_string(_Old_capacity)) { + if (_Old_capacity > _Small_string_capacity) { _Deallocate_for_capacity(_Al, _Mypair._Myval2._Bx._Ptr, _Old_capacity); _Mypair._Myval2._Bx._Ptr = _New_ptr; } else { @@ -4783,7 +4776,7 @@ private: _My_data._Mysize = _New_size; _My_data._Myres = _New_capacity; _Elem* const _Raw_new = _Unfancy(_New_ptr); - if (_Entails_large_string(_Old_capacity)) { + if (_Old_capacity > _Small_string_capacity) { const pointer _Old_ptr = _My_data._Bx._Ptr; _Fn(_Raw_new, _Unfancy(_Old_ptr), _Old_size, _Args...); _Deallocate_for_capacity(_Al, _Old_ptr, _Old_capacity); @@ -4801,7 +4794,7 @@ private: // release any held storage and return to small string mode auto& _My_data = _Mypair._Myval2; _STL_INTERNAL_CHECK(_My_data._Large_mode_engaged()); - _STL_INTERNAL_CHECK(!_Entails_large_string(_My_data._Mysize)); + _STL_INTERNAL_CHECK(_My_data._Mysize <= _Small_string_capacity); _My_data._Orphan_all(); _ASAN_STRING_REMOVE(*this); From 84b8ced778b462551b6bac33111f86b79da78870 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Wed, 9 Aug 2023 12:52:29 -0700 Subject: [PATCH 24/26] "small mode" instead of "SSO mode" It's more clearly the opposite of "large mode". --- stl/inc/xstring | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 161f02e0ef9..abd4fd0766f 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2220,7 +2220,7 @@ public: : sizeof(value_type) <= 4 ? 3 : sizeof(value_type) <= 8 ? 1 : 0; - // capacity in SSO mode + // capacity in small mode static constexpr size_type _Small_string_capacity = _BUF_SIZE - 1; _NODISCARD _CONSTEXPR20 value_type* _Myptr() noexcept { @@ -2701,7 +2701,7 @@ private: // Pre: _First models input_iterator or meets the Cpp17InputIterator requirements // Pre: [_First, _Last) is a valid range // Pre: if is_same_v<_Size, size_type>, _Count is the length of [_First, _Last). - // Pre: *this is in SSO mode; the lifetime of the SSO elements has already begun + // Pre: *this is in small mode; the lifetime of the SSO elements has already begun auto& _My_data = _Mypair._Myval2; auto& _Al = _Getal(); @@ -2966,7 +2966,7 @@ public: _Result._Ptr = _My_data._Bx._Ptr; _Result._Res = _My_data._Myres + 1; } else { - // use _Least_allocation_size to avoid SSO, if the buffer is assigned back + // use _Least_allocation_size to avoid small mode, if the buffer is assigned back size_type _Allocated = _Least_allocation_size; _Result._Ptr = _Allocate_at_least_helper(_Al, _Allocated); _Traits::copy(_Unfancy(_Result._Ptr), _My_data._Bx._Buf, _BUF_SIZE); From 9ed46dc642f6eb2eedd4ff3ccdd48891369812dd Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Wed, 9 Aug 2023 12:54:28 -0700 Subject: [PATCH 25/26] Rename `_Buf_res` and `_Released_buffer::_Res` ... to the more descriptive `_Actual_allocation_size`. --- stl/inc/sstream | 10 +++++----- stl/inc/xstring | 18 +++++++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/stl/inc/sstream b/stl/inc/sstream index 738113dc1a9..9a4aafa910a 100644 --- a/stl/inc/sstream +++ b/stl/inc/sstream @@ -473,11 +473,11 @@ protected: } // finite buffer that can be read or written, set it up - auto [_Ptr, _Size, _Res] = _Str._Release_to_buffer(_Al); - _Elem* const _Pnew = _Unfancy(_Ptr); - _Seekhigh = _Pnew + _Size; - auto _Next = (_State & (_Atend | _Append)) ? _Seekhigh : _Pnew; - auto _End_buffer = _Pnew + _Res; + auto [_Ptr, _Size, _Actual_allocation_size] = _Str._Release_to_buffer(_Al); + _Elem* const _Pnew = _Unfancy(_Ptr); + _Seekhigh = _Pnew + _Size; + auto _Next = (_State & (_Atend | _Append)) ? _Seekhigh : _Pnew; + auto _End_buffer = _Pnew + _Actual_allocation_size; _Mysb::setp(_Pnew, _Next, _End_buffer); if (_State & _Noread) { // maintain "_Allocated == eback() points to buffer base" invariant diff --git a/stl/inc/xstring b/stl/inc/xstring index abd4fd0766f..74c7568203b 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2926,22 +2926,22 @@ public: } _NODISCARD bool _Move_assign_from_buffer( - _Elem* const _Right, const size_type _Size, const size_type _Buf_res /* real allocation size */) { + _Elem* const _Right, const size_type _Size, const size_type _Actual_allocation_size) { // Move assign from a buffer, used exclusively by basic_stringbuf; returns _Large_mode_engaged() auto& _My_data = _Mypair._Myval2; _STL_INTERNAL_CHECK(!_My_data._Large_mode_engaged() && _My_data._Mysize == 0); - _STL_INTERNAL_CHECK(_Size < _Buf_res); // So there is room for null terminator + _STL_INTERNAL_CHECK(_Size < _Actual_allocation_size); // So there is room for null terminator _Traits::assign(_Right[_Size], _Elem()); - const bool _Is_large = _Buf_res >= _Least_allocation_size; + const bool _Is_large = _Actual_allocation_size >= _Least_allocation_size; if (_Is_large) { _ASAN_STRING_REMOVE(*this); _Construct_in_place(_My_data._Bx._Ptr, _Refancy(_Right)); _My_data._Mysize = _Size; - _My_data._Myres = _Buf_res - 1; + _My_data._Myres = _Actual_allocation_size - 1; _ASAN_STRING_CREATE(*this); } else { - _Traits::copy(_My_data._Bx._Buf, _Right, _Buf_res); + _Traits::copy(_My_data._Bx._Buf, _Right, _Actual_allocation_size); _My_data._Mysize = _Size; _My_data._Myres = _Small_string_capacity; } @@ -2953,7 +2953,7 @@ public: struct _Released_buffer { pointer _Ptr; size_type _Size; - size_type _Res; // real allocation size + size_type _Actual_allocation_size; }; _NODISCARD _Released_buffer _Release_to_buffer(_Alloc& _Al) { @@ -2963,14 +2963,14 @@ public: _Result._Size = _My_data._Mysize; _ASAN_STRING_REMOVE(*this); if (_My_data._Large_mode_engaged()) { - _Result._Ptr = _My_data._Bx._Ptr; - _Result._Res = _My_data._Myres + 1; + _Result._Ptr = _My_data._Bx._Ptr; + _Result._Actual_allocation_size = _My_data._Myres + 1; } else { // use _Least_allocation_size to avoid small mode, if the buffer is assigned back size_type _Allocated = _Least_allocation_size; _Result._Ptr = _Allocate_at_least_helper(_Al, _Allocated); _Traits::copy(_Unfancy(_Result._Ptr), _My_data._Bx._Buf, _BUF_SIZE); - _Result._Res = _Allocated; + _Result._Actual_allocation_size = _Allocated; } _My_data._Orphan_all(); _Tidy_init(); From e2ac4ced96c9069c973441529fc903178261f05d Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Wed, 9 Aug 2023 12:55:14 -0700 Subject: [PATCH 26/26] Don't enumerate users of `_Least_allocation_size` Grepping is quick, and this is very likely to bitrot. --- stl/inc/xstring | 1 - 1 file changed, 1 deletion(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 74c7568203b..3ba4d362d76 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2369,7 +2369,6 @@ private: static constexpr size_type _Small_string_capacity = _Scary_val::_Small_string_capacity; // least real allocation size, including space reserved for terminating null - // used exclusively by _Move_assign_from_buffer and _Release_to_buffer static constexpr size_type _Least_allocation_size = _Small_string_capacity + 1 + 1; // When doing _String_val operations by memcpy, we are touching: