From 72229ffd5bf3d5a74e3d68ebed00a0919eb7f071 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Wed, 11 Jan 2023 18:19:27 -0800 Subject: [PATCH] ``: constexpr string element lifetime (#3334) --- stl/inc/xstring | 77 +++++++++++-------------------- tests/libcxx/expected_results.txt | 7 --- tests/libcxx/skipped_tests.txt | 7 --- 3 files changed, 28 insertions(+), 63 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index d159988e4f..2347690322 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2286,7 +2286,7 @@ public: } constexpr void _Activate_SSO_buffer() noexcept { - // begin the lifetime of the array elements (e.g., before copying into them) + // start the lifetime of the array elements #if _HAS_CXX20 if (_STD is_constant_evaluated()) { for (size_type _Idx = 0; _Idx < _BUF_SIZE; ++_Idx) { @@ -2629,6 +2629,21 @@ public: } private: + static constexpr void _Start_element_lifetimes(_Elem* const _Ptr, const size_type _Size) { + // 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) { + _STD construct_at(_Ptr + _Idx); + } + } +#else // ^^^ C++20-or-later / pre-C++20 vvv + (void) _Ptr; + (void) _Size; +#endif // _HAS_CXX20 + } + enum class _Construct_strategy : uint8_t { _From_char, _From_ptr, _From_string }; template <_Construct_strategy _Strat, class _Char_or_ptr> @@ -2658,13 +2673,13 @@ private: _Traits::assign(_My_data._Bx._Buf, _Count, _Arg); _Traits::assign(_My_data._Bx._Buf[_Count], _Elem()); } else if constexpr (_Strat == _Construct_strategy::_From_ptr) { - _Traits::move(_My_data._Bx._Buf, _Arg, _Count); + _Traits::copy(_My_data._Bx._Buf, _Arg, _Count); _Traits::assign(_My_data._Bx._Buf[_Count], _Elem()); } else { // _Strat == _Construct_strategy::_From_string #ifdef _INSERT_STRING_ANNOTATION - _Traits::move(_My_data._Bx._Buf, _Arg, _Count + 1); + _Traits::copy(_My_data._Bx._Buf, _Arg, _Count + 1); #else // ^^^ _INSERT_STRING_ANNOTATION / !_INSERT_STRING_ANNOTATION vvv - _Traits::move(_My_data._Bx._Buf, _Arg, _BUF_SIZE); + _Traits::copy(_My_data._Bx._Buf, _Arg, _BUF_SIZE); #endif // !_INSERT_STRING_ANNOTATION } @@ -2677,11 +2692,7 @@ private: const pointer _New_ptr = _Al.allocate(_New_capacity + 1); // throws _Construct_in_place(_My_data._Bx._Ptr, _New_ptr); -#if _HAS_CXX20 - if (_STD is_constant_evaluated()) { // Begin the lifetimes of the objects before copying to avoid UB - _Traits::assign(_Unfancy(_New_ptr), _New_capacity + 1, _Elem()); - } -#endif // _HAS_CXX20 + _Start_element_lifetimes(_Unfancy(_New_ptr), _New_capacity + 1); _My_data._Mysize = _Count; _My_data._Myres = _New_capacity; @@ -2725,11 +2736,7 @@ private: _Construct_in_place(_My_data._Bx._Ptr, _New_ptr); _My_data._Myres = _New_capacity; -#if _HAS_CXX20 - if (_STD is_constant_evaluated()) { // Begin the lifetimes of the objects before copying to avoid UB - _Traits::assign(_Unfancy(_New_ptr), _New_capacity + 1, _Elem()); - } -#endif // _HAS_CXX20 + _Start_element_lifetimes(_Unfancy(_New_ptr), _New_capacity + 1); } } @@ -2745,11 +2752,7 @@ private: const size_type _New_capacity = _Calculate_growth(_My_data._Mysize); const pointer _New_ptr = _Al.allocate(_New_capacity + 1); // throws -#if _HAS_CXX20 - if (_STD is_constant_evaluated()) { // Begin the lifetimes of the objects before copying to avoid UB - _Traits::assign(_Unfancy(_New_ptr), _New_capacity + 1, _Elem()); - } -#endif // _HAS_CXX20 + _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 _Al.deallocate(_My_data._Bx._Ptr, _My_data._Myres + 1); @@ -2834,11 +2837,7 @@ public: _Ptr = _Unfancy(_Fancyptr); _Construct_in_place(_My_data._Bx._Ptr, _Fancyptr); -#if _HAS_CXX20 - if (_STD is_constant_evaluated()) { // Begin the lifetimes of the objects before copying to avoid UB - _Traits::assign(_Ptr, _New_capacity + 1, _Elem()); - } -#endif // _HAS_CXX20 + _Start_element_lifetimes(_Ptr, _New_capacity + 1); } _My_data._Mysize = _New_size; @@ -2909,11 +2908,7 @@ public: _Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _My_data); // throws const pointer _Fancyptr = _Getal().allocate(_New_capacity + 1); // throws // nothrow hereafter -#if _HAS_CXX20 - if (_STD is_constant_evaluated()) { // Begin the lifetimes of the objects before copying to avoid UB - _Traits::assign(_Unfancy(_Fancyptr), _New_capacity + 1, _Elem()); - } -#endif // _HAS_CXX20 + _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; @@ -3217,11 +3212,7 @@ public: auto _Right_al_non_const = _Right_al; const auto _New_ptr = _Right_al_non_const.allocate(_New_capacity + 1); // throws -#if _HAS_CXX20 - if (_STD is_constant_evaluated()) { // Begin the lifetimes of the objects before copying to avoid UB - _Traits::assign(_Unfancy(_New_ptr), _New_size + 1, _Elem()); - } -#endif // _HAS_CXX20 + _Start_element_lifetimes(_Unfancy(_New_ptr), _New_size + 1); _Traits::copy(_Unfancy(_New_ptr), _Unfancy(_Right._Mypair._Myval2._Bx._Ptr), _New_size + 1); _Tidy_deallocate(); @@ -4049,11 +4040,7 @@ public: const pointer _New_ptr = _Al.allocate(_Target_capacity + 1); // throws _ASAN_STRING_REMOVE(*this); -#if _HAS_CXX20 - if (_STD is_constant_evaluated()) { // Begin the lifetimes of the objects before copying to avoid UB - _Traits::assign(_Unfancy(_New_ptr), _Target_capacity + 1, _Elem()); - } -#endif // _HAS_CXX20 + _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); @@ -4792,11 +4779,7 @@ private: auto& _Al = _Getal(); const pointer _New_ptr = _Al.allocate(_New_capacity + 1); // throws -#if _HAS_CXX20 - if (_STD is_constant_evaluated()) { // Begin the lifetimes of the objects before copying to avoid UB - _Traits::assign(_Unfancy(_New_ptr), _New_capacity + 1, _Elem()); - } -#endif // _HAS_CXX20 + _Start_element_lifetimes(_Unfancy(_New_ptr), _New_capacity + 1); _Mypair._Myval2._Orphan_all(); _ASAN_STRING_REMOVE(*this); _Mypair._Myval2._Mysize = _New_size; @@ -4829,11 +4812,7 @@ private: auto& _Al = _Getal(); const pointer _New_ptr = _Al.allocate(_New_capacity + 1); // throws -#if _HAS_CXX20 - if (_STD is_constant_evaluated()) { // Begin the lifetimes of the objects before copying to avoid UB - _Traits::assign(_Unfancy(_New_ptr), _New_capacity + 1, _Elem()); - } -#endif // _HAS_CXX20 + _Start_element_lifetimes(_Unfancy(_New_ptr), _New_capacity + 1); _My_data._Orphan_all(); _ASAN_STRING_REMOVE(*this); _My_data._Mysize = _New_size; diff --git a/tests/libcxx/expected_results.txt b/tests/libcxx/expected_results.txt index a77a6c9724..d1d5b5ed10 100644 --- a/tests/libcxx/expected_results.txt +++ b/tests/libcxx/expected_results.txt @@ -1009,7 +1009,6 @@ std/ranges/range.adaptors/range.lazy.split/view_interface.pass.cpp FAIL std/ranges/range.adaptors/range.take/adaptor.pass.cpp FAIL std/ranges/range.factories/range.single.view/cpo.pass.cpp FAIL std/strings/basic.string/string.cons/dtor.pass.cpp FAIL -std/strings/basic.string/string.cons/implicit_deduction_guides.pass.cpp FAIL std/strings/basic.string/string.modifiers/string_erase/iter.pass.cpp:0 FAIL std/strings/basic.string/string.modifiers/string_erase/iter_iter.pass.cpp:0 FAIL std/strings/basic.string/string.modifiers/string_insert/iter_iter_iter.pass.cpp:0 FAIL @@ -1020,12 +1019,6 @@ std/strings/basic.string/string.modifiers/string_replace/iter_iter_pointer_size. std/strings/basic.string/string.modifiers/string_replace/iter_iter_size_char.pass.cpp:0 FAIL std/strings/basic.string/string.modifiers/string_replace/iter_iter_string.pass.cpp:0 FAIL std/strings/basic.string/string.modifiers/string_replace/iter_iter_string_view.pass.cpp:0 FAIL -std/strings/string.view/string.view.comparison/equal.pass.cpp FAIL -std/strings/string.view/string.view.comparison/greater.pass.cpp FAIL -std/strings/string.view/string.view.comparison/greater_equal.pass.cpp FAIL -std/strings/string.view/string.view.comparison/less.pass.cpp FAIL -std/strings/string.view/string.view.comparison/less_equal.pass.cpp FAIL -std/strings/string.view/string.view.comparison/not_equal.pass.cpp FAIL std/thread/futures/futures.task/futures.task.members/ctor2.compile.pass.cpp FAIL std/utilities/function.objects/func.wrap/func.wrap.func/addressof.pass.cpp:0 FAIL std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.alg/swap.pass.cpp:0 FAIL diff --git a/tests/libcxx/skipped_tests.txt b/tests/libcxx/skipped_tests.txt index fdcf3360ad..ea976d9948 100644 --- a/tests/libcxx/skipped_tests.txt +++ b/tests/libcxx/skipped_tests.txt @@ -1009,7 +1009,6 @@ ranges\range.adaptors\range.lazy.split\view_interface.pass.cpp ranges\range.adaptors\range.take\adaptor.pass.cpp ranges\range.factories\range.single.view\cpo.pass.cpp strings\basic.string\string.cons\dtor.pass.cpp -strings\basic.string\string.cons\implicit_deduction_guides.pass.cpp strings\basic.string\string.modifiers\string_erase\iter.pass.cpp strings\basic.string\string.modifiers\string_erase\iter_iter.pass.cpp strings\basic.string\string.modifiers\string_insert\iter_iter_iter.pass.cpp @@ -1020,12 +1019,6 @@ strings\basic.string\string.modifiers\string_replace\iter_iter_pointer_size.pass strings\basic.string\string.modifiers\string_replace\iter_iter_size_char.pass.cpp strings\basic.string\string.modifiers\string_replace\iter_iter_string.pass.cpp strings\basic.string\string.modifiers\string_replace\iter_iter_string_view.pass.cpp -strings\string.view\string.view.comparison\equal.pass.cpp -strings\string.view\string.view.comparison\greater.pass.cpp -strings\string.view\string.view.comparison\greater_equal.pass.cpp -strings\string.view\string.view.comparison\less.pass.cpp -strings\string.view\string.view.comparison\less_equal.pass.cpp -strings\string.view\string.view.comparison\not_equal.pass.cpp thread\futures\futures.task\futures.task.members\ctor2.compile.pass.cpp utilities\function.objects\func.wrap\func.wrap.func\addressof.pass.cpp utilities\function.objects\func.wrap\func.wrap.func\func.wrap.func.alg\swap.pass.cpp