From 057df3db3886b37b39d7c05b1068c113196f9863 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 6 Oct 2023 15:16:38 +0800 Subject: [PATCH 1/6] Use `memcpy` in the iterator-pair construction of `basic_string` --- stl/inc/xstring | 67 +++-- .../tests/P1206R7_string_from_range/test.cpp | 252 +++++++++++++++++- 2 files changed, 289 insertions(+), 30 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 8b7fc948540..de05094cec7 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2722,35 +2722,62 @@ private: } _Tidy_deallocate_guard _Guard{this}; - for (; _First != _Last; ++_First) { - if constexpr (!is_same_v<_Size, size_type>) { - if (_My_data._Mysize == _My_data._Myres) { // Need to grow - if (_My_data._Mysize == max_size()) { - _Xlen_string(); // result too long - } - _Elem* const _Old_ptr = _My_data._Myptr(); - size_type _New_capacity = _Calculate_growth(_My_data._Mysize + 1); - const pointer _New_ptr = _Allocate_for_capacity(_Al, _New_capacity); // throws + constexpr bool _Can_construct_by_memcpy = _Is_specialization_v<_Traits, char_traits> && _Is_EcharT<_Elem> + && _Iterator_is_contiguous<_Iter> && !_Iterator_is_volatile<_Iter> + && is_integral_v<_Iter_value_t<_Iter>> + && sizeof(_Iter_value_t<_Iter>) == sizeof(_Elem); + + if constexpr (_Can_construct_by_memcpy) { + _STL_INTERNAL_STATIC_ASSERT(is_same_v<_Size, size_type>); - _Traits::copy(_Unfancy(_New_ptr), _Old_ptr, _My_data._Mysize); - 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 { - _Construct_in_place(_My_data._Bx._Ptr, _New_ptr); + const auto _Data = _My_data._Myptr(); + const auto _Src_data = _STD _To_address(_First); + +#if _HAS_CXX20 + if (_STD is_constant_evaluated()) { + for (size_type _Idx = 0; _Idx != _Count; ++_Idx) { + _Data[_Idx] = static_cast<_Elem>(_Src_data[_Idx]); + } + } else +#endif // _HAS_CXX20 + { + _CSTD memcpy(_Data, _Src_data, _Count * sizeof(_Elem)); + } + _My_data._Mysize = _Count; + _Data[_Count] = _Elem(); + } else { + for (; _First != _Last; ++_First) { + if constexpr (!is_same_v<_Size, size_type>) { + if (_My_data._Mysize == _My_data._Myres) { // Need to grow + if (_My_data._Mysize == max_size()) { + _Xlen_string(); // result too long + } + + _Elem* const _Old_ptr = _My_data._Myptr(); + size_type _New_capacity = _Calculate_growth(_My_data._Mysize + 1); + 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_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 { + _Construct_in_place(_My_data._Bx._Ptr, _New_ptr); + } + _My_data._Myres = _New_capacity; } - _My_data._Myres = _New_capacity; } + + _Elem* const _Ptr = _My_data._Myptr(); + _Traits::assign(_Ptr[_My_data._Mysize], *_First); + ++_My_data._Mysize; } _Elem* const _Ptr = _My_data._Myptr(); - _Traits::assign(_Ptr[_My_data._Mysize], *_First); - ++_My_data._Mysize; + _Traits::assign(_Ptr[_My_data._Mysize], _Elem()); } - _Elem* const _Ptr = _My_data._Myptr(); - _Traits::assign(_Ptr[_My_data._Mysize], _Elem()); _ASAN_STRING_CREATE(*this); _Guard._Target = nullptr; _Proxy._Release(); diff --git a/tests/std/tests/P1206R7_string_from_range/test.cpp b/tests/std/tests/P1206R7_string_from_range/test.cpp index b94a62dc326..3a86cbab98e 100644 --- a/tests/std/tests/P1206R7_string_from_range/test.cpp +++ b/tests/std/tests/P1206R7_string_from_range/test.cpp @@ -2,7 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception #include +#include #include +#include #include #include #include @@ -44,6 +46,12 @@ constexpr bool test_string(Rng&& rng, const T* expected) { static constexpr char hw[] = "Hello, world!"; static constexpr auto span_hw = span{hw}.first(); +static constexpr signed char hw_s[] = "Hello, world!"; +static constexpr auto span_hw_s = span{hw_s}.first(); + +static constexpr unsigned char hw_u[] = "Hello, world!"; +static constexpr auto span_hw_u = span{hw_u}.first(); + struct string_instantiator { template static void call() { @@ -52,9 +60,76 @@ struct string_instantiator { } }; +#ifdef __cpp_char8_t +static constexpr char8_t hw_u8[] = u8"Hello, world!"; +static constexpr auto span_hw_u8 = span{hw_u8}.first(); + +struct u8string_instantiator { + template + static void call() { + test_string(R{span_hw_u8}, hw_u8); + STATIC_ASSERT(test_string(R{span_hw_u8}, hw_u8)); + } +}; +#endif // defined(__cpp_char8_t) + +static constexpr char16_t hw_u16[] = u"Hello, world!"; +static constexpr auto span_hw_u16 = span{hw_u16}.first(); + +static constexpr int_least16_t hw_u16s[]{ + u'H', u'e', u'l', u'l', u'o', u',', u' ', u'w', u'o', u'r', u'l', u'd', u'!', u'\0'}; +static constexpr auto span_hw_u16s = span{hw_u16s}.first(); + +static constexpr uint_least16_t hw_u16u[]{ + u'H', u'e', u'l', u'l', u'o', u',', u' ', u'w', u'o', u'r', u'l', u'd', u'!', u'\0'}; +static constexpr auto span_hw_u16u = span{hw_u16u}.first(); + +struct u16string_instantiator { + template + static void call() { + test_string(R{span_hw_u16}, hw_u16); + STATIC_ASSERT(test_string(R{span_hw_u16}, hw_u16)); + } +}; + +static constexpr char32_t hw_u32[] = U"Hello, world!"; +static constexpr auto span_hw_u32 = span{hw_u32}.first(); + +static constexpr int_least32_t hw_u32s[]{ + U'H', U'e', U'l', U'l', U'o', U',', U' ', U'w', U'o', U'r', U'l', U'd', U'!', U'\0'}; +static constexpr auto span_hw_u32s = span{hw_u32s}.first(); + +static constexpr uint_least32_t hw_u32u[]{ + U'H', U'e', U'l', U'l', U'o', U',', U' ', U'w', U'o', U'r', U'l', U'd', U'!', U'\0'}; +static constexpr auto span_hw_u32u = span{hw_u32u}.first(); + +static constexpr long hw_slong[]{U'H', U'e', U'l', U'l', U'o', U',', U' ', U'w', U'o', U'r', U'l', U'd', U'!', U'\0'}; +static constexpr auto span_hw_slong = span{hw_slong}.first(); + +static constexpr unsigned long hw_ulong[]{ + U'H', U'e', U'l', U'l', U'o', U',', U' ', U'w', U'o', U'r', U'l', U'd', U'!', U'\0'}; +static constexpr auto span_hw_ulong = span{hw_ulong}.first(); + +struct u32string_instantiator { + template + static void call() { + test_string(R{span_hw_u32}, hw_u32); + STATIC_ASSERT(test_string(R{span_hw_u32}, hw_u32)); + } +}; + +using swchar_t = make_signed_t; +using uwchar_t = make_unsigned_t; + static constexpr wchar_t whw[] = L"Hello, world!"; static constexpr auto span_whw = span{whw}.first(); +static constexpr swchar_t whw_s[]{L'H', L'e', L'l', L'l', L'o', L',', L' ', L'w', L'o', L'r', L'l', L'd', L'!', L'\0'}; +static constexpr auto span_whw_s = span{whw_s}.first(); + +static constexpr uwchar_t whw_u[]{L'H', L'e', L'l', L'l', L'o', L',', L' ', L'w', L'o', L'r', L'l', L'd', L'!', L'\0'}; +static constexpr auto span_whw_u = span{whw_u}.first(); + struct wstring_instantiator { template static void call() { @@ -71,7 +146,32 @@ using move_only_view = test::range(const Span& sp, const CharT* cstr) { + vector vec(sp.data(), sp.data() + sp.size()); + test_string(vec, cstr); + }; + + test_lvalue_vector_helper(span_hw, hw); + test_lvalue_vector_helper(span_hw_s, hw); + test_lvalue_vector_helper(span_hw_u, hw); +#ifdef __cpp_char8_t + test_lvalue_vector_helper(span_hw_u8, hw); + + test_lvalue_vector_helper(span_hw_u8, hw_u8); + test_lvalue_vector_helper(span_hw, hw_u8); + test_lvalue_vector_helper(span_hw_s, hw_u8); + test_lvalue_vector_helper(span_hw_u, hw_u8); +#endif // __cpp_char8_t + + test_lvalue_vector_helper(span_hw_u16, hw_u16); + test_lvalue_vector_helper(span_hw_u16s, hw_u16); + test_lvalue_vector_helper(span_hw_u16u, hw_u16); + test_lvalue_vector_helper(span_whw, hw_u16); + + test_lvalue_vector_helper(span_hw_u32, hw_u32); + test_lvalue_vector_helper(span_hw_u32s, hw_u32); + test_lvalue_vector_helper(span_hw_u32u, hw_u32); + test_lvalue_vector_helper(span_hw_slong, hw_u32); + test_lvalue_vector_helper(span_hw_ulong, hw_u32); + + test_lvalue_vector_helper(span_whw, whw); + test_lvalue_vector_helper(span_whw_s, whw); + test_lvalue_vector_helper(span_whw_u, whw); + test_lvalue_vector_helper(span_hw_u16, whw); return true; } @@ -120,6 +329,20 @@ void test_lvalue_forward_list() { forward_list lst(span_hw.data(), span_hw.data() + span_hw.size()); test_string(lst, hw); } +#ifdef __cpp_char8_t + { + forward_list vec(span_hw.data(), span_hw.data() + span_hw.size()); + test_string(vec, hw); + } +#endif // __cpp_char8_t + { + forward_list vec(span_hw_u16.data(), span_hw_u16.data() + span_hw_u16.size()); + test_string(vec, hw_u16); + } + { + forward_list vec(span_hw_u32.data(), span_hw_u32.data() + span_hw_u32.size()); + test_string(vec, hw_u32); + } { forward_list lst(span_whw.data(), span_whw.data() + span_whw.size()); test_string(lst, whw); @@ -138,6 +361,9 @@ int main() { test_c_array(); STATIC_ASSERT(test_c_array()); + test_std_array(); + STATIC_ASSERT(test_std_array()); + test_lvalue_vector(); STATIC_ASSERT(test_lvalue_vector()); @@ -145,4 +371,10 @@ int main() { test_in(); test_in(); + +#ifdef __cpp_char8_t + test_contiguous(); +#endif // defined(__cpp_char8_t) + test_contiguous(); + test_contiguous(); } From c43439576ebb887c6b3a1013071a82c110a1d921 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 6 Oct 2023 15:02:22 -0700 Subject: [PATCH 2/6] Fix typos, one major. --- .../tests/P1206R7_string_from_range/test.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/std/tests/P1206R7_string_from_range/test.cpp b/tests/std/tests/P1206R7_string_from_range/test.cpp index 3a86cbab98e..047b23630ab 100644 --- a/tests/std/tests/P1206R7_string_from_range/test.cpp +++ b/tests/std/tests/P1206R7_string_from_range/test.cpp @@ -128,7 +128,7 @@ static constexpr swchar_t whw_s[]{L'H', L'e', L'l', L'l', L'o', L',', L' ', L'w' static constexpr auto span_whw_s = span{whw_s}.first(); static constexpr uwchar_t whw_u[]{L'H', L'e', L'l', L'l', L'o', L',', L' ', L'w', L'o', L'r', L'l', L'd', L'!', L'\0'}; -static constexpr auto span_whw_u = span{whw_u}.first(); +static constexpr auto span_whw_u = span{whw_u}.first(); struct wstring_instantiator { template @@ -303,7 +303,7 @@ constexpr bool test_lvalue_vector() { test_lvalue_vector_helper(span_hw, hw_u8); test_lvalue_vector_helper(span_hw_s, hw_u8); test_lvalue_vector_helper(span_hw_u, hw_u8); -#endif // __cpp_char8_t +#endif // defined(__cpp_char8_t) test_lvalue_vector_helper(span_hw_u16, hw_u16); test_lvalue_vector_helper(span_hw_u16s, hw_u16); @@ -331,17 +331,17 @@ void test_lvalue_forward_list() { } #ifdef __cpp_char8_t { - forward_list vec(span_hw.data(), span_hw.data() + span_hw.size()); - test_string(vec, hw); + forward_list lst(span_hw_u8.data(), span_hw_u8.data() + span_hw_u8.size()); + test_string(lst, hw_u8); } -#endif // __cpp_char8_t +#endif // defined(__cpp_char8_t) { - forward_list vec(span_hw_u16.data(), span_hw_u16.data() + span_hw_u16.size()); - test_string(vec, hw_u16); + forward_list lst(span_hw_u16.data(), span_hw_u16.data() + span_hw_u16.size()); + test_string(lst, hw_u16); } { - forward_list vec(span_hw_u32.data(), span_hw_u32.data() + span_hw_u32.size()); - test_string(vec, hw_u32); + forward_list lst(span_hw_u32.data(), span_hw_u32.data() + span_hw_u32.size()); + test_string(lst, hw_u32); } { forward_list lst(span_whw.data(), span_whw.data() + span_whw.size()); From 55c74e2245f831fed8d3bd4f5140e31da437e336 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Sat, 7 Oct 2023 23:53:42 +0800 Subject: [PATCH 3/6] Tweak comments for preconditions of `_Construct_from_iter` Co-authored-by: Casey Carter --- stl/inc/xstring | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index de05094cec7..b80f6d7ce61 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2695,10 +2695,12 @@ private: template _CONSTEXPR20 void _Construct_from_iter(_Iter _First, const _Sent _Last, _Size _Count = {}) { - // Pre: _First models input_iterator or meets the Cpp17InputIterator requirements - // Pre: [_First, _Last) is a valid range + // Pre: _Iter models input_iterator or meets the Cpp17InputIterator requirements. + // Pre: [_First, _Last) is a valid range. + // Pre: if _Iter models forward_iterator or meets the Cpp17ForwardIterator requirements, + // then is_same_v<_Size, size_type> holds. // Pre: if is_same_v<_Size, size_type>, _Count is the length of [_First, _Last). - // Pre: *this is in small 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(); From 67d11b68c9bf89fd428ae21d0db8ca5630834ee3 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Tue, 10 Oct 2023 07:14:26 +0800 Subject: [PATCH 4/6] Reuse `_Copy_n_unchecked4` (which calls `memmove`) Co-authored-by: Casey Carter --- stl/inc/xstring | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index b80f6d7ce61..d9be25f12d5 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2725,27 +2725,12 @@ private: _Tidy_deallocate_guard _Guard{this}; - constexpr bool _Can_construct_by_memcpy = _Is_specialization_v<_Traits, char_traits> && _Is_EcharT<_Elem> - && _Iterator_is_contiguous<_Iter> && !_Iterator_is_volatile<_Iter> - && is_integral_v<_Iter_value_t<_Iter>> - && sizeof(_Iter_value_t<_Iter>) == sizeof(_Elem); + constexpr bool _Can_construct_by_copy = + _Is_specialization_v<_Traits, char_traits> && _Is_EcharT<_Elem> && is_same_v<_Size, size_type>; - if constexpr (_Can_construct_by_memcpy) { - _STL_INTERNAL_STATIC_ASSERT(is_same_v<_Size, size_type>); - - const auto _Data = _My_data._Myptr(); - const auto _Src_data = _STD _To_address(_First); - -#if _HAS_CXX20 - if (_STD is_constant_evaluated()) { - for (size_type _Idx = 0; _Idx != _Count; ++_Idx) { - _Data[_Idx] = static_cast<_Elem>(_Src_data[_Idx]); - } - } else -#endif // _HAS_CXX20 - { - _CSTD memcpy(_Data, _Src_data, _Count * sizeof(_Elem)); - } + if constexpr (_Can_construct_by_copy) { + const auto _Data = _My_data._Myptr(); + _STD _Copy_n_unchecked4(_STD move(_First), _Count, _Data); _My_data._Mysize = _Count; _Data[_Count] = _Elem(); } else { From 3fddbf5e184dae85fe48d064635dc01a199ffc91 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Tue, 10 Oct 2023 10:35:11 +0800 Subject: [PATCH 5/6] Locally suppress warning C4365 --- stl/inc/xstring | 3 +++ 1 file changed, 3 insertions(+) diff --git a/stl/inc/xstring b/stl/inc/xstring index d9be25f12d5..f8d196f4191 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2730,7 +2730,10 @@ private: if constexpr (_Can_construct_by_copy) { const auto _Data = _My_data._Myptr(); +#pragma warning(push) +#pragma warning(disable : 4365) // suppress the warning from signedness mismatch in assignment _STD _Copy_n_unchecked4(_STD move(_First), _Count, _Data); +#pragma warning(pop) _My_data._Mysize = _Count; _Data[_Count] = _Elem(); } else { From da953f898f750a09de9394123e1b86da8b9329f4 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Tue, 10 Oct 2023 15:45:07 +0800 Subject: [PATCH 6/6] Move the suppression to the test --- stl/inc/xstring | 3 --- tests/std/tests/P1206R7_string_from_range/test.cpp | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index f8d196f4191..d9be25f12d5 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -2730,10 +2730,7 @@ private: if constexpr (_Can_construct_by_copy) { const auto _Data = _My_data._Myptr(); -#pragma warning(push) -#pragma warning(disable : 4365) // suppress the warning from signedness mismatch in assignment _STD _Copy_n_unchecked4(_STD move(_First), _Count, _Data); -#pragma warning(pop) _My_data._Mysize = _Count; _Data[_Count] = _Elem(); } else { diff --git a/tests/std/tests/P1206R7_string_from_range/test.cpp b/tests/std/tests/P1206R7_string_from_range/test.cpp index 047b23630ab..f9f4daecb35 100644 --- a/tests/std/tests/P1206R7_string_from_range/test.cpp +++ b/tests/std/tests/P1206R7_string_from_range/test.cpp @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +#pragma warning(disable : 4365) // conversion from 'X' to 'Y', signed/unsigned mismatch + #include #include #include