From b0f24017086093dcfe3aa81d36454ed2b591731e Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 26 Aug 2022 11:04:30 +0800 Subject: [PATCH 01/28] Implement P2438R2 and (un)fix allocator propagation and update references to Working Draft --- stl/inc/xstring | 85 ++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 67 insertions(+), 18 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 41302d912d..b82dc248e1 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -606,7 +606,7 @@ constexpr size_t _Traits_find(_In_reads_(_Hay_size) const _Traits_ptr_t<_Traits> // search [_Haystack, _Haystack + _Hay_size) for [_Needle, _Needle + _Needle_size), at/after _Start_at if (_Needle_size > _Hay_size || _Start_at > _Hay_size - _Needle_size) { // xpos cannot exist, report failure - // N4659 24.3.2.7.2 [string.find]/1 says: + // N4910 23.3.3.8 [string.view.find]/3 says: // 1. _Start_at <= xpos // 2. xpos + _Needle_size <= _Hay_size; // therefore: @@ -1200,12 +1200,12 @@ template class basic_string_view { // wrapper for any kind of contiguous character buffer public: static_assert(is_same_v<_Elem, typename _Traits::char_type>, - "Bad char_traits for basic_string_view; " - "N4659 24.4.2 [string.view.template]/1 \"the type traits::char_type shall name the same type as charT.\""); + "Bad char_traits for basic_string_view; N4910 23.3.3.1 [string.view.template.general]/1 " + "\"The program is ill-formed if traits::char_type is not the same type as charT.\""); static_assert(!is_array_v<_Elem> && is_trivial_v<_Elem> && is_standard_layout_v<_Elem>, - "The character type of basic_string_view must be a non-array trivial standard-layout type. See N4861 " - "[strings.general]/1."); + "The character type of basic_string must be a non-array trivial standard-layout type. See N4910 " + "23.1 [strings.general]/1."); using traits_type = _Traits; using value_type = _Elem; @@ -1224,7 +1224,7 @@ public: constexpr basic_string_view() noexcept : _Mydata(), _Mysize(0) {} - constexpr basic_string_view(const basic_string_view&) noexcept = default; + constexpr basic_string_view(const basic_string_view&) noexcept = default; constexpr basic_string_view& operator=(const basic_string_view&) noexcept = default; /* implicit */ constexpr basic_string_view(_In_z_ const const_pointer _Ntcts) noexcept // strengthened @@ -1806,7 +1806,7 @@ struct _Get_comparison_category<_Traits, void_t, - "N4878 [string.view.comparison]/4: Mandates: R denotes a comparison category type."); + "N4910 23.3.5 [string.view.comparison]/4: Mandates: R denotes a comparison category type."); }; template @@ -2440,12 +2440,12 @@ private: _MISMATCHED_ALLOCATOR_MESSAGE("basic_string", "T")); static_assert(is_same_v<_Elem, typename _Traits::char_type>, - "N4659 24.3.2.1 [string.require]/3 requires that the supplied " + "N4910 23.4.3.2 [string.require]/3 requires that the supplied " "char_traits character type match the string's character type."); static_assert(!is_array_v<_Elem> && is_trivial_v<_Elem> && is_standard_layout_v<_Elem>, - "The character type of basic_string must be a non-array trivial standard-layout type. See N4861 " - "[strings.general]/1."); + "The character type of basic_string must be a non-array trivial standard-layout type. See N4910 " + "23.1 [strings.general]/1."); public: using traits_type = _Traits; @@ -2474,7 +2474,7 @@ private: // _String_val::_Bx::_Ptr (type is pointer) // _String_val::_Mysize (type is size_type) // _String_val::_Myres (type is size_type) - // N4810 21.1 [strings.general]/1 says _Elem must be trivial standard-layout, so memcpy is safe. + // N4910 23.1 [strings.general]/1 says _Elem must be trivial standard-layout, so memcpy is safe. // We need to ask if pointer is safe to memcpy. // size_type must be an unsigned integral type so memcpy is safe. // We also need to disable memcpy if the user has supplied _Traits, since @@ -2629,6 +2629,19 @@ public: _Right._Mypair._Myval2._Myptr() + _Roff, _Right._Mypair._Myval2._Clamp_suffix_size(_Roff, _Count)); } +#if _HAS_CXX23 + constexpr basic_string(basic_string&& _Right, const size_type _Roff, const _Alloc& _Al = _Alloc()) + : _Mypair(_One_then_variadic_args_t{}, _Al) { // construct from _Right [_Roff, ), potentially move + _Move_construct_from_substr(_Right, _Roff, npos, _Al); + } + + constexpr basic_string( + basic_string&& _Right, const size_type _Roff, const size_type _Count, const _Alloc& _Al = _Alloc()) + : _Mypair(_One_then_variadic_args_t{}, _Al) { // construct from _Right [_Roff, _Roff + _Count), potentially move + _Move_construct_from_substr(_Right, _Roff, _Count, _Al); + } +#endif // _HAS_CXX23 + _CONSTEXPR20 basic_string(_In_reads_(_Count) const _Elem* const _Ptr, _CRT_GUARDOVERFLOW const size_type _Count) : _Mypair(_Zero_then_variadic_args_t{}) { _Construct<_Construct_strategy::_From_ptr>(_Ptr, _Count); @@ -3165,6 +3178,29 @@ private: _Right._Tidy_init(); } +#if _HAS_CXX23 + constexpr void _Move_construct_from_substr( + basic_string& _Right, const size_type _Roff, const size_type _Size_max, const _Alloc& _Al) { + auto& _Right_data = _Right._Mypair._Myval2; + _Right_data._Check_offset(_Roff); + + const auto _Result_size = _Right_data._Clamp_suffix_size(_Roff, _Size_max); + const auto _Right_ptr = _Right_data._Myptr(); + if (_Allocators_equal(_Al, _Right._Getal()) && _Result_size >= _Scary_val::_BUF_SIZE) { + if (_Roff != 0) { + _Traits::move(_Right_ptr, _Right_ptr + _Roff, _Result_size); + } + _Right._Eos(_Result_size); + _Right_data._Mysize = _Result_size; + + _Mypair._Myval2._Alloc_proxy(_GET_PROXY_ALLOCATOR(_Alty, _Getal())); + _Take_contents(_Right); + } else { + _Construct<_Construct_strategy::_From_ptr>(_Right_ptr + _Roff, _Result_size); + } + } +#endif // _HAS_CXX23 + public: _CONSTEXPR20 basic_string(initializer_list<_Elem> _Ilist, const _Alloc& _Al = allocator_type()) : _Mypair(_One_then_variadic_args_t{}, _Al) { @@ -4675,10 +4711,23 @@ public: } #endif // _HAS_CXX17 - _NODISCARD _CONSTEXPR20 basic_string substr(const size_type _Off = 0, const size_type _Count = npos) const { - // return [_Off, _Off + _Count) as new string - return basic_string{*this, _Off, _Count, get_allocator()}; + _NODISCARD _CONSTEXPR20 basic_string substr(const size_type _Off = 0, const size_type _Count = npos) +#if _HAS_CXX23 + const& +#else + const +#endif // _HAS_CXX23 + { + // return [_Off, _Off + _Count) as new string, default-constructing its allocator + return basic_string{*this, _Off, _Count}; + } + +#if _HAS_CXX23 + _NODISCARD constexpr basic_string substr(const size_type _Off = 0, const size_type _Count = npos) && { + // return [_Off, _Off + _Count) as new string, potentially moving, default-constructing its allocator + return basic_string{_STD move(*this), _Off, _Count}; } +#endif // _HAS_CXX23 _CONSTEXPR20 bool _Equal(const basic_string& _Right) const noexcept { // compare [0, size()) with _Right for equality @@ -5082,10 +5131,10 @@ _NODISCARD _CONSTEXPR20 basic_string<_Elem, _Traits, _Alloc> operator+( basic_string<_Elem, _Traits, _Alloc>&& _Left, basic_string<_Elem, _Traits, _Alloc>&& _Right) { #if _ITERATOR_DEBUG_LEVEL == 2 _STL_VERIFY(_STD addressof(_Left) != _STD addressof(_Right), - "You cannot concatenate the same moved string to itself. See " - "N4849 [res.on.arguments]/1.3: If a function argument binds to an rvalue reference " - "parameter, the implementation may assume that this parameter is a unique reference " - "to this argument"); + "You cannot concatenate the same moved string to itself. See N4910 16.4.5.9 [res.on.arguments]/1.3: " + "If a function argument is bound to an rvalue reference parameter, the implementation may assume that " + "this parameter is a unique reference to this argument, except that the argument passed to " + "a move-assignment operator may be a reference to *this (16.4.6.15 [lib.types.movedfrom])."); #endif // _ITERATOR_DEBUG_LEVEL == 2 return {_String_constructor_concat_tag{}, _Left, _Right}; } From 7b283ce90ee3ebac41964b44e6ab5ac0c5029695 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 26 Aug 2022 11:06:31 +0800 Subject: [PATCH 02/28] Add test files for GH-3022 --- .../tests/GH_003022_substr_allocator/env.lst | 4 + .../tests/GH_003022_substr_allocator/test.cpp | 181 ++++++++++++++++++ 2 files changed, 185 insertions(+) create mode 100644 tests/std/tests/GH_003022_substr_allocator/env.lst create mode 100644 tests/std/tests/GH_003022_substr_allocator/test.cpp diff --git a/tests/std/tests/GH_003022_substr_allocator/env.lst b/tests/std/tests/GH_003022_substr_allocator/env.lst new file mode 100644 index 0000000000..19f025bd0e --- /dev/null +++ b/tests/std/tests/GH_003022_substr_allocator/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\usual_matrix.lst diff --git a/tests/std/tests/GH_003022_substr_allocator/test.cpp b/tests/std/tests/GH_003022_substr_allocator/test.cpp new file mode 100644 index 0000000000..31f6f164de --- /dev/null +++ b/tests/std/tests/GH_003022_substr_allocator/test.cpp @@ -0,0 +1,181 @@ +#include +#include +#include +#include + +using namespace std; + +#if _HAS_CXX20 +#define CONSTEXPR20 constexpr +#else // ^^^ _HAS_CXX20 / vvv !_HAS_CXX20 +#define CONSTEXPR20 inline +#endif // _HAS_CXX20 + +#define TEST_ASSERT(...) assert((__VA_ARGS__)) + +#ifdef __cpp_char8_t +using char8_type = char8_t; +#else // ^^^ defined(__cpp_char8_t) / vvv !defined(__cpp_char8_t) +using char8_type = unsigned char; +#endif // __cpp_char8_t + +template , int> = 0> +constexpr auto statically_widen_impl( + const char* s, const char8_type*, const char16_t*, const char32_t*, const wchar_t*) noexcept { + return s; +} + +template , int> = 0> +constexpr auto statically_widen_impl( + const char*, const char8_type* s8, const char16_t*, const char32_t*, const wchar_t*) noexcept { + return s8; +} + +template , int> = 0> +constexpr auto statically_widen_impl( + const char*, const char8_type*, const char16_t* s16, const char32_t*, const wchar_t*) noexcept { + return s16; +} + +template , int> = 0> +constexpr auto statically_widen_impl( + const char*, const char8_type*, const char16_t*, const char32_t* s32, const wchar_t*) noexcept { + return s32; +} + +template , int> = 0> +constexpr auto statically_widen_impl( + const char*, const char8_type*, const char16_t*, const char32_t*, const wchar_t* sw) noexcept { + return sw; +} + +#ifdef __cpp_char8_t +#define STATICALLY_WIDEN(CT, S) (statically_widen_impl(S, u8##S, u##S, U##S, L##S)) +#else // ^^^ defined(__cpp_char8_t) / vvv !defined(__cpp_char8_t) +#define STATICALLY_WIDEN(CT, S) (statically_widen_impl(S, nullptr, u##S, U##S, L##S)) +#endif // __cpp_char8_t + +template +class payloaded_allocator { +private: + int payload_ = 0; + +public: + using value_type = T; + using size_type = size_t; + using difference_type = ptrdiff_t; + using propagate_on_container_move_assignment = true_type; + using propagate_on_container_swap = true_type; + + payloaded_allocator() = default; + explicit constexpr payloaded_allocator(int payload) noexcept : payload_{payload} {} + + template + constexpr payloaded_allocator(const payloaded_allocator& other) noexcept : payload_{other.get_payload()} {} + + template + friend constexpr bool operator==(const payloaded_allocator& lhs, const payloaded_allocator& rhs) noexcept { + return lhs.get_payload() == rhs.get_payload(); + } + +#if !_HAS_CXX20 + template + friend constexpr bool operator!=(const payloaded_allocator& lhs, const payloaded_allocator& rhs) noexcept { + return !(lhs == rhs); + } +#endif // !_HAS_CXX20 + + CONSTEXPR20 T* allocate(const size_t n) { + return allocator{}.allocate(n); + } + + CONSTEXPR20 void deallocate(T* const p, const size_t n) { + return allocator{}.deallocate(p, n); + } + + constexpr int get_payload() const noexcept { + return payload_; + } +}; + +template +CONSTEXPR20 bool test_substr_allocator() { + struct str_test_case { + const CharT* ntcts; + size_t offset; + size_t count; + }; + + constexpr str_test_case substring_test_cases[]{ + {STATICALLY_WIDEN(CharT, "nul"), 1, 0}, + {STATICALLY_WIDEN(CharT, "str"), 1, 2}, + {STATICALLY_WIDEN(CharT, "ful"), 0, 3}, + {STATICALLY_WIDEN(CharT, "Empty"), 4, 0}, + {STATICALLY_WIDEN(CharT, "Shorter"), 5, 2}, + {STATICALLY_WIDEN(CharT, "Longer"), 1, 5}, + {STATICALLY_WIDEN(CharT, "FullStr"), 0, 7}, + {STATICALLY_WIDEN(CharT, "Empty substring"), 12, 0}, + {STATICALLY_WIDEN(CharT, "Short substring"), 9, 3}, + {STATICALLY_WIDEN(CharT, "Mid substring"), 7, 5}, + {STATICALLY_WIDEN(CharT, "Longer string"), 3, 9}, + {STATICALLY_WIDEN(CharT, "Maximal SSO len"), 0, 15}, + {STATICALLY_WIDEN(CharT, "Take an empty substring from a long string"), 35, 0}, + {STATICALLY_WIDEN(CharT, "Take a short substring from a long string"), 21, 2}, + {STATICALLY_WIDEN(CharT, "Take some substring from a long string"), 15, 6}, + {STATICALLY_WIDEN(CharT, "Take a longer substring from a long string"), 20, 13}, + {STATICALLY_WIDEN(CharT, "Take a quiet longer substring..."), 10, 20}, + {STATICALLY_WIDEN(CharT, "Take the whole long string as a substring"), 0, 41}, + }; + + using string_type = basic_string, payloaded_allocator>; + + auto myator = payloaded_allocator{42}; + + for (const auto& test_case : substring_test_cases) { + auto full_str = string_type{test_case.ntcts, myator}; + + // Test const lvalue overloads + TEST_ASSERT(string_type{full_str, test_case.offset, test_case.count}.get_allocator().get_payload() == 0); + TEST_ASSERT(string_type{full_str, test_case.offset}.get_allocator().get_payload() == 0); + + TEST_ASSERT(full_str.substr(test_case.offset, test_case.count).get_allocator().get_payload() == 0); + + TEST_ASSERT( + string_type{full_str, test_case.offset, test_case.count, myator}.get_allocator().get_payload() == 42); + TEST_ASSERT(string_type{full_str, test_case.offset, myator}.get_allocator().get_payload() == 42); + + // Test non-const rvalue overloads + TEST_ASSERT( + string_type{string_type{full_str}, test_case.offset, test_case.count}.get_allocator().get_payload() == 0); + TEST_ASSERT(string_type{string_type{full_str}, test_case.offset}.get_allocator().get_payload() == 0); + + TEST_ASSERT(string_type{full_str}.substr(test_case.offset, test_case.count).get_allocator().get_payload() == 0); + + TEST_ASSERT( + string_type{string_type{full_str}, test_case.offset, test_case.count, myator}.get_allocator().get_payload() + == 42); + TEST_ASSERT(string_type{string_type{full_str}, test_case.offset, myator}.get_allocator().get_payload() == 42); + } + + return true; +} + +#if _HAS_CXX20 +static_assert(test_substr_allocator()); +#ifdef __cpp_char8_t +static_assert(test_substr_allocator()); +#endif // __cpp_char8_t +static_assert(test_substr_allocator()); +static_assert(test_substr_allocator()); +static_assert(test_substr_allocator()); +#endif // _HAS_CXX20 + +int main() { + (void) test_substr_allocator(); +#ifdef __cpp_char8_t + (void) test_substr_allocator(); +#endif // __cpp_char8_t + (void) test_substr_allocator(); + (void) test_substr_allocator(); + (void) test_substr_allocator(); +} From f623376826074e6c31f9f49f53cb2ad9beaf354d Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 26 Aug 2022 11:10:15 +0800 Subject: [PATCH 03/28] Add test files for P2438R2 --- tests/std/tests/P2438R2_substr_rvalue/env.lst | 4 + .../std/tests/P2438R2_substr_rvalue/test.cpp | 234 ++++++++++++++++++ 2 files changed, 238 insertions(+) create mode 100644 tests/std/tests/P2438R2_substr_rvalue/env.lst create mode 100644 tests/std/tests/P2438R2_substr_rvalue/test.cpp diff --git a/tests/std/tests/P2438R2_substr_rvalue/env.lst b/tests/std/tests/P2438R2_substr_rvalue/env.lst new file mode 100644 index 0000000000..642f530ffa --- /dev/null +++ b/tests/std/tests/P2438R2_substr_rvalue/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\usual_latest_matrix.lst diff --git a/tests/std/tests/P2438R2_substr_rvalue/test.cpp b/tests/std/tests/P2438R2_substr_rvalue/test.cpp new file mode 100644 index 0000000000..3c1da6cfa5 --- /dev/null +++ b/tests/std/tests/P2438R2_substr_rvalue/test.cpp @@ -0,0 +1,234 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include +#include +#include + +#define TEST_ASSERT(...) assert((__VA_ARGS__)) + +using namespace std; + +template +class payloaded_allocator { +private: + int payload_ = 0; + +public: + using value_type = T; + using size_type = size_t; + using difference_type = ptrdiff_t; + using propagate_on_container_move_assignment = true_type; + using propagate_on_container_swap = true_type; + + payloaded_allocator() = default; + explicit constexpr payloaded_allocator(int payload) noexcept : payload_{payload} {} + + template + constexpr payloaded_allocator(const payloaded_allocator& other) noexcept : payload_{other.get_payload()} {} + + template + friend constexpr bool operator==(const payloaded_allocator& lhs, const payloaded_allocator& rhs) noexcept { + return lhs.get_payload() == rhs.get_payload(); + } + + constexpr T* allocate(const size_t n) { + return allocator{}.allocate(n); + } + + constexpr void deallocate(T* const p, const size_t n) { + return allocator{}.deallocate(p, n); + } + + constexpr int get_payload() const noexcept { + return payload_; + } +}; + +template +consteval auto statically_widen_impl(string_view sv, +#ifdef __cpp_char8_t + u8string_view svu8, +#endif // __cpp_char8_t + u16string_view svu16, u32string_view svu32, wstring_view svw) noexcept { + if constexpr (is_same_v) { + return sv; + } +#ifdef __cpp_char8_t + else if constexpr (is_same_v) { + return svu8; + } +#endif // __cpp_char8_t + else if constexpr (is_same_v) { + return svu16; + } else if constexpr (is_same_v) { + return svu32; + } else if constexpr (is_same_v) { + return svw; + } else { + static_assert(!is_same_v, "Invalid character type"); + } +} + +#ifdef __cpp_char8_t +#define STATICALLY_WIDEN(CT, S) (statically_widen_impl(S##sv, u8##S##sv, u##S##sv, U##S##sv, L##S##sv)) +#else // ^^^ defined(__cpp_char8_t) / vvv !defined(__cpp_char8_t) +#define STATICALLY_WIDEN(CT, S) (statically_widen_impl(S##sv, u##S##sv, U##S##sv, L##S##sv)) +#endif // __cpp_char8_t + +template +constexpr bool test_rvalue_substr() { + struct str_test_case { + basic_string_view str_view; + size_t offset; + size_t count; + }; + + constexpr str_test_case substring_test_cases[]{ + {STATICALLY_WIDEN(CharT, "nul"), 1, 0}, + {STATICALLY_WIDEN(CharT, "str"), 1, 2}, + {STATICALLY_WIDEN(CharT, "ful"), 0, 3}, + {STATICALLY_WIDEN(CharT, "Empty"), 4, 0}, + {STATICALLY_WIDEN(CharT, "Shorter"), 5, 2}, + {STATICALLY_WIDEN(CharT, "Longer"), 1, 5}, + {STATICALLY_WIDEN(CharT, "FullStr"), 0, 7}, + {STATICALLY_WIDEN(CharT, "Empty substring"), 12, 0}, + {STATICALLY_WIDEN(CharT, "Short substring"), 9, 3}, + {STATICALLY_WIDEN(CharT, "Mid substring"), 7, 5}, + {STATICALLY_WIDEN(CharT, "Longer string"), 3, 9}, + {STATICALLY_WIDEN(CharT, "Maximal SSO len"), 0, 15}, + {STATICALLY_WIDEN(CharT, "Take an empty substring from a long string"), 35, 0}, + {STATICALLY_WIDEN(CharT, "Take a short substring from a long string"), 21, 2}, + {STATICALLY_WIDEN(CharT, "Take some substring from a long string"), 15, 6}, + {STATICALLY_WIDEN(CharT, "Take a longer substring from a long string"), 20, 13}, + {STATICALLY_WIDEN(CharT, "Take a quiet longer substring..."), 10, 20}, + {STATICALLY_WIDEN(CharT, "Take the whole long string as a substring"), 0, 41}, + }; + + for (const auto& [str_view, offset, count] : substring_test_cases) { + using string_type = basic_string; + + const auto substring_view = str_view.substr(offset, count); + const auto suffix_view = str_view.substr(offset); + + // Non-portable implementation details + constexpr size_t sso_buffer_capacity = basic_string{}.capacity(); + + TEST_ASSERT(string_type{string_type{str_view}, offset, count} == substring_view); + TEST_ASSERT(string_type{str_view}.substr(offset, count) == substring_view); + TEST_ASSERT(string_type{string_type{str_view}, offset} == suffix_view); + TEST_ASSERT(string_type{str_view}.substr(offset) == suffix_view); + + // Test non-portable implementation details + { + auto source_str = string_type{str_view}; + const auto old_data = source_str.data(); + auto dest_str = string_type{move(source_str), offset, count}; + + TEST_ASSERT((dest_str.data() == old_data) == (dest_str.size() > sso_buffer_capacity)); + } + + { + auto source_str = string_type{str_view}; + const auto old_data = source_str.data(); + auto dest_str = string_type{move(source_str), offset}; + + TEST_ASSERT((dest_str.data() == old_data) == (dest_str.size() > sso_buffer_capacity)); + } + + { + auto source_str = string_type{str_view}; + const auto old_data = source_str.data(); + auto dest_str = move(source_str).substr(offset, count); + + TEST_ASSERT((dest_str.data() == old_data) == (dest_str.size() > sso_buffer_capacity)); + } + + // Test non-portable implementation details for stateful allocators + + using payloaded_string_type = basic_string, payloaded_allocator>; + + auto non_default_allocator = payloaded_allocator{42}; + { + auto source_str = payloaded_string_type{str_view}; + const auto old_data = source_str.data(); + auto dest_str = payloaded_string_type{move(source_str), offset, count}; + + TEST_ASSERT((dest_str.data() == old_data) == (dest_str.size() > sso_buffer_capacity)); + } + { + auto source_str = payloaded_string_type{str_view, non_default_allocator}; + const auto old_data = source_str.data(); + auto dest_str = payloaded_string_type{move(source_str), offset, count}; + + TEST_ASSERT(dest_str.data() != old_data); + } + { + auto source_str = payloaded_string_type{str_view, non_default_allocator}; + const auto old_data = source_str.data(); + auto dest_str = payloaded_string_type{move(source_str), offset, count, non_default_allocator}; + + TEST_ASSERT((dest_str.data() == old_data) == (dest_str.size() > sso_buffer_capacity)); + } + + { + auto source_str = payloaded_string_type{str_view}; + const auto old_data = source_str.data(); + auto dest_str = payloaded_string_type{move(source_str), offset}; + + TEST_ASSERT((dest_str.data() == old_data) == (dest_str.size() > sso_buffer_capacity)); + } + { + auto source_str = payloaded_string_type{str_view, non_default_allocator}; + const auto old_data = source_str.data(); + auto dest_str = payloaded_string_type{move(source_str), offset}; + + TEST_ASSERT(dest_str.data() != old_data); + } + { + auto source_str = payloaded_string_type{str_view, non_default_allocator}; + const auto old_data = source_str.data(); + auto dest_str = payloaded_string_type{move(source_str), offset, non_default_allocator}; + + TEST_ASSERT((dest_str.data() == old_data) == (dest_str.size() > sso_buffer_capacity)); + } + + { + auto source_str = payloaded_string_type{str_view}; + const auto old_data = source_str.data(); + auto dest_str = move(source_str).substr(offset, count); + + TEST_ASSERT((dest_str.data() == old_data) == (dest_str.size() > sso_buffer_capacity)); + } + { + auto source_str = payloaded_string_type{str_view, non_default_allocator}; + + const auto old_data = source_str.data(); + auto dest_str = move(source_str).substr(offset, count); + + TEST_ASSERT(dest_str.data() != old_data); + } + } + + return true; +} + +static_assert(test_rvalue_substr()); +#ifdef __cpp_char8_t +static_assert(test_rvalue_substr()); +#endif // __cpp_char8_t +static_assert(test_rvalue_substr()); +static_assert(test_rvalue_substr()); +static_assert(test_rvalue_substr()); + +int main() { + (void) test_rvalue_substr(); +#ifdef __cpp_char8_t + (void) test_rvalue_substr(); +#endif // __cpp_char8_t + (void) test_rvalue_substr(); + (void) test_rvalue_substr(); + (void) test_rvalue_substr(); +} From 518cd1be9207ca203b6ac549e82bdfc147ac5528 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 26 Aug 2022 11:11:00 +0800 Subject: [PATCH 04/28] Add missing license --- tests/std/tests/GH_003022_substr_allocator/test.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/std/tests/GH_003022_substr_allocator/test.cpp b/tests/std/tests/GH_003022_substr_allocator/test.cpp index 31f6f164de..628dec84d5 100644 --- a/tests/std/tests/GH_003022_substr_allocator/test.cpp +++ b/tests/std/tests/GH_003022_substr_allocator/test.cpp @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + #include #include #include From e184af54eb9690253d582dd4e1b633f014e6e7ea Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 26 Aug 2022 11:12:26 +0800 Subject: [PATCH 05/28] Add tests for GH-3022 and P2438R2 to list --- tests/std/test.lst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/std/test.lst b/tests/std/test.lst index 55b7738804..55372b528e 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -211,6 +211,7 @@ tests\GH_002760_syncstream_memory_leak tests\GH_002769_handle_deque_block_pointers tests\GH_002789_Hash_vec_Tidy tests\GH_002989_nothrow_unwrappable +tests\GH_003022_substr_allocator tests\LWG2597_complex_branch_cut tests\LWG3018_shared_ptr_function tests\LWG3121_constrained_tuple_forwarding_ctor @@ -549,6 +550,7 @@ tests\P2401R0_conditional_noexcept_for_exchange tests\P2408R5_ranges_iterators_to_classic_algorithms tests\P2415R2_owning_view tests\P2417R2_constexpr_bitset +tests\P2438R2_substr_rvalue tests\P2440R1_ranges_alg_shift_left tests\P2440R1_ranges_alg_shift_right tests\P2440R1_ranges_numeric_iota From f60413dc75c8ae614e2025c3ed62b7c10a6c4194 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 26 Aug 2022 11:32:33 +0800 Subject: [PATCH 06/28] Also test well-formedness change before/after P2438R2 --- .../tests/GH_003022_substr_allocator/test.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/std/tests/GH_003022_substr_allocator/test.cpp b/tests/std/tests/GH_003022_substr_allocator/test.cpp index 628dec84d5..8366fc236f 100644 --- a/tests/std/tests/GH_003022_substr_allocator/test.cpp +++ b/tests/std/tests/GH_003022_substr_allocator/test.cpp @@ -160,6 +160,25 @@ CONSTEXPR20 bool test_substr_allocator() { TEST_ASSERT(string_type{string_type{full_str}, test_case.offset, myator}.get_allocator().get_payload() == 42); } + // Also test well-formedness change before/after P2438R2 + struct ambiguator { + string_type str; + + operator const string_type&() { + return str; + } + + operator string_type&&() { + return move(str); + } + }; + +#if _HAS_CXX23 + static_assert(!is_constructible_v, "Ambiguous"); +#else + static_assert(is_constructible_v, "Not ambiguous"); +#endif // _HAS_CXX23 + return true; } From bd6e75ed0a640ea7f659c3130913d11d4c2fd3f1 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 26 Aug 2022 11:54:30 +0800 Subject: [PATCH 07/28] Fix mis-formatting --- stl/inc/xstring | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index b82dc248e1..a0421a8e10 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -1224,7 +1224,7 @@ public: constexpr basic_string_view() noexcept : _Mydata(), _Mysize(0) {} - constexpr basic_string_view(const basic_string_view&) noexcept = default; + constexpr basic_string_view(const basic_string_view&) noexcept = default; constexpr basic_string_view& operator=(const basic_string_view&) noexcept = default; /* implicit */ constexpr basic_string_view(_In_z_ const const_pointer _Ntcts) noexcept // strengthened From 5ab204a3324b0c5a9349b21627ce145bb5df332d Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 26 Aug 2022 11:56:14 +0800 Subject: [PATCH 08/28] should be included --- tests/std/tests/GH_003022_substr_allocator/test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/std/tests/GH_003022_substr_allocator/test.cpp b/tests/std/tests/GH_003022_substr_allocator/test.cpp index 8366fc236f..fbaacd4fbe 100644 --- a/tests/std/tests/GH_003022_substr_allocator/test.cpp +++ b/tests/std/tests/GH_003022_substr_allocator/test.cpp @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception #include +#include #include #include #include From 36ca7d10e1b1fa5d9c2ce93b8149280a18f8d642 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 26 Aug 2022 11:56:34 +0800 Subject: [PATCH 09/28] should be included --- tests/std/tests/P2438R2_substr_rvalue/test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/std/tests/P2438R2_substr_rvalue/test.cpp b/tests/std/tests/P2438R2_substr_rvalue/test.cpp index 3c1da6cfa5..7e7f9a7a01 100644 --- a/tests/std/tests/P2438R2_substr_rvalue/test.cpp +++ b/tests/std/tests/P2438R2_substr_rvalue/test.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #define TEST_ASSERT(...) assert((__VA_ARGS__)) From c88d4b7fe30f6b255cf6db8992e845f839bcfba7 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 26 Aug 2022 13:50:11 +0800 Subject: [PATCH 10/28] Workaround for EDG's __builtin_memcmp --- stl/inc/xstring | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/stl/inc/xstring b/stl/inc/xstring index a0421a8e10..46453d2bad 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -376,7 +376,23 @@ public: _In_reads_(_Count) const _Elem* const _First2, const size_t _Count) noexcept /* strengthened */ { // compare [_First1, _First1 + _Count) with [_First2, ...) #if _HAS_CXX17 +#ifdef __EDG__ // TRANSITION, EDG's __builtin_memcmp is buggy for constexpr dynamic allocation + if (_Is_constant_evaluated()) { + for (size_t _Ind = 0; _Ind != _Count; ++_Ind) { + if (static_cast(_First1[_Ind]) < static_cast(_First2[_Ind])) { + return -1; + } + if (static_cast(_First2[_Ind]) < static_cast(_First1[_Ind])) { + return 1; + } + } + return 0; + } else { + return _CSTD memcmp(_First1, _First2, _Count); + } +#else // ^^^ workaround for EDG / vvv no workaround needed for MSVC and Clang return __builtin_memcmp(_First1, _First2, _Count); +#endif // __EDG__ #else // _HAS_CXX17 return _CSTD memcmp(_First1, _First2, _Count); #endif // _HAS_CXX17 From 9f0f6ed05f75996aaa1c0b2ec19fbd263ed983a3 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 26 Aug 2022 14:08:45 +0800 Subject: [PATCH 11/28] Skip P2438R2 test for EDG --- tests/std/tests/GH_003022_substr_allocator/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/GH_003022_substr_allocator/test.cpp b/tests/std/tests/GH_003022_substr_allocator/test.cpp index fbaacd4fbe..7d1124e12f 100644 --- a/tests/std/tests/GH_003022_substr_allocator/test.cpp +++ b/tests/std/tests/GH_003022_substr_allocator/test.cpp @@ -174,11 +174,11 @@ CONSTEXPR20 bool test_substr_allocator() { } }; -#if _HAS_CXX23 +#if _HAS_CXX23 && !defined(__EDG__) // TRANSITION, EDG doesn't perform overload resolution correctly static_assert(!is_constructible_v, "Ambiguous"); #else static_assert(is_constructible_v, "Not ambiguous"); -#endif // _HAS_CXX23 +#endif // _HAS_CXX23 && !defined(__EDG__) return true; } From 28db86e485b9588d38d495aff7bcfb02c5b639d3 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Wed, 31 Aug 2022 01:19:13 +0800 Subject: [PATCH 12/28] Only use workaround for EDG since C++20 And conventionally use arrow comments. --- stl/inc/xstring | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 46453d2bad..1f71097ca0 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -376,7 +376,7 @@ public: _In_reads_(_Count) const _Elem* const _First2, const size_t _Count) noexcept /* strengthened */ { // compare [_First1, _First1 + _Count) with [_First2, ...) #if _HAS_CXX17 -#ifdef __EDG__ // TRANSITION, EDG's __builtin_memcmp is buggy for constexpr dynamic allocation +#if _HAS_CXX20 && defined(__EDG__) // TRANSITION, EDG's __builtin_memcmp is buggy for constexpr dynamic allocation if (_Is_constant_evaluated()) { for (size_t _Ind = 0; _Ind != _Count; ++_Ind) { if (static_cast(_First1[_Ind]) < static_cast(_First2[_Ind])) { @@ -390,9 +390,9 @@ public: } else { return _CSTD memcmp(_First1, _First2, _Count); } -#else // ^^^ workaround for EDG / vvv no workaround needed for MSVC and Clang +#else // ^^^ workaround for EDG / no workaround needed for MSVC and Clang vvv return __builtin_memcmp(_First1, _First2, _Count); -#endif // __EDG__ +#endif // ^^^ no workaround needed for MSVC and Clang ^^^ #else // _HAS_CXX17 return _CSTD memcmp(_First1, _First2, _Count); #endif // _HAS_CXX17 From d5fe2380703ed8220e5baef3b97ca5b560a0a02f Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 30 Aug 2022 16:35:04 -0700 Subject: [PATCH 13/28] Mention basic_string_view in static_assert. --- stl/inc/xstring | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index ad672f6cc1..8cf0436af7 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -1220,7 +1220,7 @@ public: "\"The program is ill-formed if traits::char_type is not the same type as charT.\""); static_assert(!is_array_v<_Elem> && is_trivial_v<_Elem> && is_standard_layout_v<_Elem>, - "The character type of basic_string must be a non-array trivial standard-layout type. See N4910 " + "The character type of basic_string_view must be a non-array trivial standard-layout type. See N4910 " "23.1 [strings.general]/1."); using traits_type = _Traits; From d9f588ea3ae102c8747255bb41ad4a31b83b31ba Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 30 Aug 2022 16:35:45 -0700 Subject: [PATCH 14/28] Mention unqualified _BUF_SIZE. --- stl/inc/xstring | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 8cf0436af7..db52629db5 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -3202,7 +3202,7 @@ private: const auto _Result_size = _Right_data._Clamp_suffix_size(_Roff, _Size_max); const auto _Right_ptr = _Right_data._Myptr(); - if (_Allocators_equal(_Al, _Right._Getal()) && _Result_size >= _Scary_val::_BUF_SIZE) { + if (_Allocators_equal(_Al, _Right._Getal()) && _Result_size >= _BUF_SIZE) { if (_Roff != 0) { _Traits::move(_Right_ptr, _Right_ptr + _Roff, _Result_size); } From 7983801797ed44135a357e17888a24967c0ea022 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 30 Aug 2022 16:36:41 -0700 Subject: [PATCH 15/28] _Eos() takes care of updating _Mysize. --- stl/inc/xstring | 1 - 1 file changed, 1 deletion(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index db52629db5..3154b7b61f 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -3207,7 +3207,6 @@ private: _Traits::move(_Right_ptr, _Right_ptr + _Roff, _Result_size); } _Right._Eos(_Result_size); - _Right_data._Mysize = _Result_size; _Mypair._Myval2._Alloc_proxy(_GET_PROXY_ALLOCATOR(_Alty, _Getal())); _Take_contents(_Right); From 5df2855b5cd554b9d7de36cfdc892dc28144defd Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 30 Aug 2022 16:37:56 -0700 Subject: [PATCH 16/28] Mention new feature in yvals_core.h. --- stl/inc/yvals_core.h | 1 + 1 file changed, 1 insertion(+) diff --git a/stl/inc/yvals_core.h b/stl/inc/yvals_core.h index 213263eced..623ef9ab97 100644 --- a/stl/inc/yvals_core.h +++ b/stl/inc/yvals_core.h @@ -327,6 +327,7 @@ // (changes to pair, tuple, and vector::reference only) // P2387R3 Pipe Support For User-Defined Range Adaptors // P2417R2 More constexpr bitset +// P2438R2 string::substr() && // P2440R1 ranges::iota, ranges::shift_left, ranges::shift_right // P2441R2 views::join_with // P2442R1 Windowing Range Adaptors: views::chunk, views::slide From 6b10c8b0af7c963f76f468094049d5d5e38d0da5 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 30 Aug 2022 16:38:27 -0700 Subject: [PATCH 17/28] Include for move(). --- tests/std/tests/GH_003022_substr_allocator/test.cpp | 1 + tests/std/tests/P2438R2_substr_rvalue/test.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/std/tests/GH_003022_substr_allocator/test.cpp b/tests/std/tests/GH_003022_substr_allocator/test.cpp index 7d1124e12f..0b17f1a620 100644 --- a/tests/std/tests/GH_003022_substr_allocator/test.cpp +++ b/tests/std/tests/GH_003022_substr_allocator/test.cpp @@ -6,6 +6,7 @@ #include #include #include +#include using namespace std; diff --git a/tests/std/tests/P2438R2_substr_rvalue/test.cpp b/tests/std/tests/P2438R2_substr_rvalue/test.cpp index 7e7f9a7a01..34e367c01f 100644 --- a/tests/std/tests/P2438R2_substr_rvalue/test.cpp +++ b/tests/std/tests/P2438R2_substr_rvalue/test.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #define TEST_ASSERT(...) assert((__VA_ARGS__)) From ef33954ed0a940a7ed8a65fe65762ac421f342ff Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 30 Aug 2022 16:38:48 -0700 Subject: [PATCH 18/28] Style: constexpr explicit. --- tests/std/tests/GH_003022_substr_allocator/test.cpp | 2 +- tests/std/tests/P2438R2_substr_rvalue/test.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/GH_003022_substr_allocator/test.cpp b/tests/std/tests/GH_003022_substr_allocator/test.cpp index 0b17f1a620..e1991579a4 100644 --- a/tests/std/tests/GH_003022_substr_allocator/test.cpp +++ b/tests/std/tests/GH_003022_substr_allocator/test.cpp @@ -73,7 +73,7 @@ class payloaded_allocator { using propagate_on_container_swap = true_type; payloaded_allocator() = default; - explicit constexpr payloaded_allocator(int payload) noexcept : payload_{payload} {} + constexpr explicit payloaded_allocator(int payload) noexcept : payload_{payload} {} template constexpr payloaded_allocator(const payloaded_allocator& other) noexcept : payload_{other.get_payload()} {} diff --git a/tests/std/tests/P2438R2_substr_rvalue/test.cpp b/tests/std/tests/P2438R2_substr_rvalue/test.cpp index 34e367c01f..6f5f258b19 100644 --- a/tests/std/tests/P2438R2_substr_rvalue/test.cpp +++ b/tests/std/tests/P2438R2_substr_rvalue/test.cpp @@ -26,7 +26,7 @@ class payloaded_allocator { using propagate_on_container_swap = true_type; payloaded_allocator() = default; - explicit constexpr payloaded_allocator(int payload) noexcept : payload_{payload} {} + constexpr explicit payloaded_allocator(int payload) noexcept : payload_{payload} {} template constexpr payloaded_allocator(const payloaded_allocator& other) noexcept : payload_{other.get_payload()} {} From 7ca2319f7078de3db5d9c62dc1ec1a7885c17bb9 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 30 Aug 2022 16:39:54 -0700 Subject: [PATCH 19/28] Typo: quiet => quite. --- tests/std/tests/GH_003022_substr_allocator/test.cpp | 2 +- tests/std/tests/P2438R2_substr_rvalue/test.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/GH_003022_substr_allocator/test.cpp b/tests/std/tests/GH_003022_substr_allocator/test.cpp index e1991579a4..1a6568bdf1 100644 --- a/tests/std/tests/GH_003022_substr_allocator/test.cpp +++ b/tests/std/tests/GH_003022_substr_allocator/test.cpp @@ -128,7 +128,7 @@ CONSTEXPR20 bool test_substr_allocator() { {STATICALLY_WIDEN(CharT, "Take a short substring from a long string"), 21, 2}, {STATICALLY_WIDEN(CharT, "Take some substring from a long string"), 15, 6}, {STATICALLY_WIDEN(CharT, "Take a longer substring from a long string"), 20, 13}, - {STATICALLY_WIDEN(CharT, "Take a quiet longer substring..."), 10, 20}, + {STATICALLY_WIDEN(CharT, "Take a quite longer substring..."), 10, 20}, {STATICALLY_WIDEN(CharT, "Take the whole long string as a substring"), 0, 41}, }; diff --git a/tests/std/tests/P2438R2_substr_rvalue/test.cpp b/tests/std/tests/P2438R2_substr_rvalue/test.cpp index 6f5f258b19..5b9533aa04 100644 --- a/tests/std/tests/P2438R2_substr_rvalue/test.cpp +++ b/tests/std/tests/P2438R2_substr_rvalue/test.cpp @@ -105,7 +105,7 @@ constexpr bool test_rvalue_substr() { {STATICALLY_WIDEN(CharT, "Take a short substring from a long string"), 21, 2}, {STATICALLY_WIDEN(CharT, "Take some substring from a long string"), 15, 6}, {STATICALLY_WIDEN(CharT, "Take a longer substring from a long string"), 20, 13}, - {STATICALLY_WIDEN(CharT, "Take a quiet longer substring..."), 10, 20}, + {STATICALLY_WIDEN(CharT, "Take a quite longer substring..."), 10, 20}, {STATICALLY_WIDEN(CharT, "Take the whole long string as a substring"), 0, 41}, }; From 92d2ac9f80b81d8aa47472da0a06772334545444 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 30 Aug 2022 16:40:22 -0700 Subject: [PATCH 20/28] Clarify static_assert messages. --- tests/std/tests/GH_003022_substr_allocator/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/GH_003022_substr_allocator/test.cpp b/tests/std/tests/GH_003022_substr_allocator/test.cpp index 1a6568bdf1..adef618196 100644 --- a/tests/std/tests/GH_003022_substr_allocator/test.cpp +++ b/tests/std/tests/GH_003022_substr_allocator/test.cpp @@ -176,9 +176,9 @@ CONSTEXPR20 bool test_substr_allocator() { }; #if _HAS_CXX23 && !defined(__EDG__) // TRANSITION, EDG doesn't perform overload resolution correctly - static_assert(!is_constructible_v, "Ambiguous"); + static_assert(!is_constructible_v, "This should be ambiguous"); #else - static_assert(is_constructible_v, "Not ambiguous"); + static_assert(is_constructible_v, "This should be unambiguous"); #endif // _HAS_CXX23 && !defined(__EDG__) return true; From 2520e87424d837ad534d1772dc7a57879feda284 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 30 Aug 2022 16:42:00 -0700 Subject: [PATCH 21/28] Extract offset/count for clarity. --- .../tests/GH_003022_substr_allocator/test.cpp | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/tests/std/tests/GH_003022_substr_allocator/test.cpp b/tests/std/tests/GH_003022_substr_allocator/test.cpp index adef618196..ba5fa46c30 100644 --- a/tests/std/tests/GH_003022_substr_allocator/test.cpp +++ b/tests/std/tests/GH_003022_substr_allocator/test.cpp @@ -139,27 +139,26 @@ CONSTEXPR20 bool test_substr_allocator() { for (const auto& test_case : substring_test_cases) { auto full_str = string_type{test_case.ntcts, myator}; + const auto offset = test_case.offset; + const auto count = test_case.count; + // Test const lvalue overloads - TEST_ASSERT(string_type{full_str, test_case.offset, test_case.count}.get_allocator().get_payload() == 0); - TEST_ASSERT(string_type{full_str, test_case.offset}.get_allocator().get_payload() == 0); + TEST_ASSERT(string_type{full_str, offset, count}.get_allocator().get_payload() == 0); + TEST_ASSERT(string_type{full_str, offset}.get_allocator().get_payload() == 0); - TEST_ASSERT(full_str.substr(test_case.offset, test_case.count).get_allocator().get_payload() == 0); + TEST_ASSERT(full_str.substr(offset, count).get_allocator().get_payload() == 0); - TEST_ASSERT( - string_type{full_str, test_case.offset, test_case.count, myator}.get_allocator().get_payload() == 42); - TEST_ASSERT(string_type{full_str, test_case.offset, myator}.get_allocator().get_payload() == 42); + TEST_ASSERT(string_type{full_str, offset, count, myator}.get_allocator().get_payload() == 42); + TEST_ASSERT(string_type{full_str, offset, myator}.get_allocator().get_payload() == 42); // Test non-const rvalue overloads - TEST_ASSERT( - string_type{string_type{full_str}, test_case.offset, test_case.count}.get_allocator().get_payload() == 0); - TEST_ASSERT(string_type{string_type{full_str}, test_case.offset}.get_allocator().get_payload() == 0); + TEST_ASSERT(string_type{string_type{full_str}, offset, count}.get_allocator().get_payload() == 0); + TEST_ASSERT(string_type{string_type{full_str}, offset}.get_allocator().get_payload() == 0); - TEST_ASSERT(string_type{full_str}.substr(test_case.offset, test_case.count).get_allocator().get_payload() == 0); + TEST_ASSERT(string_type{full_str}.substr(offset, count).get_allocator().get_payload() == 0); - TEST_ASSERT( - string_type{string_type{full_str}, test_case.offset, test_case.count, myator}.get_allocator().get_payload() - == 42); - TEST_ASSERT(string_type{string_type{full_str}, test_case.offset, myator}.get_allocator().get_payload() == 42); + TEST_ASSERT(string_type{string_type{full_str}, offset, count, myator}.get_allocator().get_payload() == 42); + TEST_ASSERT(string_type{string_type{full_str}, offset, myator}.get_allocator().get_payload() == 42); } // Also test well-formedness change before/after P2438R2 From cb643eda4719e1abca12b51ae656fd9b99088cc1 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 30 Aug 2022 16:43:23 -0700 Subject: [PATCH 22/28] Style: _Ind => _Idx. --- stl/inc/xstring | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 3154b7b61f..a3992b8ade 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -378,11 +378,11 @@ public: #if _HAS_CXX17 #if _HAS_CXX20 && defined(__EDG__) // TRANSITION, EDG's __builtin_memcmp is buggy for constexpr dynamic allocation if (_Is_constant_evaluated()) { - for (size_t _Ind = 0; _Ind != _Count; ++_Ind) { - if (static_cast(_First1[_Ind]) < static_cast(_First2[_Ind])) { + for (size_t _Idx = 0; _Idx != _Count; ++_Idx) { + if (static_cast(_First1[_Idx]) < static_cast(_First2[_Idx])) { return -1; } - if (static_cast(_First2[_Ind]) < static_cast(_First1[_Ind])) { + if (static_cast(_First2[_Idx]) < static_cast(_First1[_Idx])) { return 1; } } From 53ca1969ee78d2bdfa47a85cd4a5b34bdde321d9 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 30 Aug 2022 16:44:05 -0700 Subject: [PATCH 23/28] Use _STD is_constant_evaluated() in C++20 mode. --- stl/inc/xstring | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index a3992b8ade..7249015fff 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -377,7 +377,7 @@ public: // compare [_First1, _First1 + _Count) with [_First2, ...) #if _HAS_CXX17 #if _HAS_CXX20 && defined(__EDG__) // TRANSITION, EDG's __builtin_memcmp is buggy for constexpr dynamic allocation - if (_Is_constant_evaluated()) { + if (_STD is_constant_evaluated()) { for (size_t _Idx = 0; _Idx != _Count; ++_Idx) { if (static_cast(_First1[_Idx]) < static_cast(_First2[_Idx])) { return -1; From c6337f01060ff3f67cd789d534f94243974e710d Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 30 Aug 2022 16:45:37 -0700 Subject: [PATCH 24/28] Newline between non-chained if-statements. --- stl/inc/xstring | 1 + 1 file changed, 1 insertion(+) diff --git a/stl/inc/xstring b/stl/inc/xstring index 7249015fff..cd40fbc54f 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -382,6 +382,7 @@ public: if (static_cast(_First1[_Idx]) < static_cast(_First2[_Idx])) { return -1; } + if (static_cast(_First2[_Idx]) < static_cast(_First1[_Idx])) { return 1; } From 52481f524f656097e1a22fb84f5baa7f702ed2c4 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 30 Aug 2022 16:51:40 -0700 Subject: [PATCH 25/28] Use direct-init. --- .../tests/GH_003022_substr_allocator/test.cpp | 4 +- .../std/tests/P2438R2_substr_rvalue/test.cpp | 41 +++++++++---------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/tests/std/tests/GH_003022_substr_allocator/test.cpp b/tests/std/tests/GH_003022_substr_allocator/test.cpp index ba5fa46c30..b708e67dfb 100644 --- a/tests/std/tests/GH_003022_substr_allocator/test.cpp +++ b/tests/std/tests/GH_003022_substr_allocator/test.cpp @@ -134,10 +134,10 @@ CONSTEXPR20 bool test_substr_allocator() { using string_type = basic_string, payloaded_allocator>; - auto myator = payloaded_allocator{42}; + payloaded_allocator myator{42}; for (const auto& test_case : substring_test_cases) { - auto full_str = string_type{test_case.ntcts, myator}; + string_type full_str{test_case.ntcts, myator}; const auto offset = test_case.offset; const auto count = test_case.count; diff --git a/tests/std/tests/P2438R2_substr_rvalue/test.cpp b/tests/std/tests/P2438R2_substr_rvalue/test.cpp index 5b9533aa04..4462f095c2 100644 --- a/tests/std/tests/P2438R2_substr_rvalue/test.cpp +++ b/tests/std/tests/P2438R2_substr_rvalue/test.cpp @@ -125,23 +125,23 @@ constexpr bool test_rvalue_substr() { // Test non-portable implementation details { - auto source_str = string_type{str_view}; + string_type source_str{str_view}; const auto old_data = source_str.data(); - auto dest_str = string_type{move(source_str), offset, count}; + string_type dest_str{move(source_str), offset, count}; TEST_ASSERT((dest_str.data() == old_data) == (dest_str.size() > sso_buffer_capacity)); } { - auto source_str = string_type{str_view}; + string_type source_str{str_view}; const auto old_data = source_str.data(); - auto dest_str = string_type{move(source_str), offset}; + string_type dest_str{move(source_str), offset}; TEST_ASSERT((dest_str.data() == old_data) == (dest_str.size() > sso_buffer_capacity)); } { - auto source_str = string_type{str_view}; + string_type source_str{str_view}; const auto old_data = source_str.data(); auto dest_str = move(source_str).substr(offset, count); @@ -152,61 +152,60 @@ constexpr bool test_rvalue_substr() { using payloaded_string_type = basic_string, payloaded_allocator>; - auto non_default_allocator = payloaded_allocator{42}; + payloaded_allocator non_default_allocator{42}; { - auto source_str = payloaded_string_type{str_view}; + payloaded_string_type source_str{str_view}; const auto old_data = source_str.data(); - auto dest_str = payloaded_string_type{move(source_str), offset, count}; + payloaded_string_type dest_str{move(source_str), offset, count}; TEST_ASSERT((dest_str.data() == old_data) == (dest_str.size() > sso_buffer_capacity)); } { - auto source_str = payloaded_string_type{str_view, non_default_allocator}; + payloaded_string_type source_str{str_view, non_default_allocator}; const auto old_data = source_str.data(); - auto dest_str = payloaded_string_type{move(source_str), offset, count}; + payloaded_string_type dest_str{move(source_str), offset, count}; TEST_ASSERT(dest_str.data() != old_data); } { - auto source_str = payloaded_string_type{str_view, non_default_allocator}; + payloaded_string_type source_str{str_view, non_default_allocator}; const auto old_data = source_str.data(); - auto dest_str = payloaded_string_type{move(source_str), offset, count, non_default_allocator}; + payloaded_string_type dest_str{move(source_str), offset, count, non_default_allocator}; TEST_ASSERT((dest_str.data() == old_data) == (dest_str.size() > sso_buffer_capacity)); } { - auto source_str = payloaded_string_type{str_view}; + payloaded_string_type source_str{str_view}; const auto old_data = source_str.data(); - auto dest_str = payloaded_string_type{move(source_str), offset}; + payloaded_string_type dest_str{move(source_str), offset}; TEST_ASSERT((dest_str.data() == old_data) == (dest_str.size() > sso_buffer_capacity)); } { - auto source_str = payloaded_string_type{str_view, non_default_allocator}; + payloaded_string_type source_str{str_view, non_default_allocator}; const auto old_data = source_str.data(); - auto dest_str = payloaded_string_type{move(source_str), offset}; + payloaded_string_type dest_str{move(source_str), offset}; TEST_ASSERT(dest_str.data() != old_data); } { - auto source_str = payloaded_string_type{str_view, non_default_allocator}; + payloaded_string_type source_str{str_view, non_default_allocator}; const auto old_data = source_str.data(); - auto dest_str = payloaded_string_type{move(source_str), offset, non_default_allocator}; + payloaded_string_type dest_str{move(source_str), offset, non_default_allocator}; TEST_ASSERT((dest_str.data() == old_data) == (dest_str.size() > sso_buffer_capacity)); } { - auto source_str = payloaded_string_type{str_view}; + payloaded_string_type source_str{str_view}; const auto old_data = source_str.data(); auto dest_str = move(source_str).substr(offset, count); TEST_ASSERT((dest_str.data() == old_data) == (dest_str.size() > sso_buffer_capacity)); } { - auto source_str = payloaded_string_type{str_view, non_default_allocator}; - + payloaded_string_type source_str{str_view, non_default_allocator}; const auto old_data = source_str.data(); auto dest_str = move(source_str).substr(offset, count); From 198554b5d05c35e126cb58c3fbe32679802ebce9 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 30 Aug 2022 16:37:33 -0700 Subject: [PATCH 26/28] Preprocessor comment nitpicks. --- stl/inc/xstring | 2 +- .../std/tests/GH_003022_substr_allocator/test.cpp | 14 +++++++------- tests/std/tests/P2438R2_substr_rvalue/test.cpp | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index cd40fbc54f..6305ca6f30 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -4730,7 +4730,7 @@ public: _NODISCARD _CONSTEXPR20 basic_string substr(const size_type _Off = 0, const size_type _Count = npos) #if _HAS_CXX23 const& -#else +#else // _HAS_CXX23 const #endif // _HAS_CXX23 { diff --git a/tests/std/tests/GH_003022_substr_allocator/test.cpp b/tests/std/tests/GH_003022_substr_allocator/test.cpp index b708e67dfb..33dd702472 100644 --- a/tests/std/tests/GH_003022_substr_allocator/test.cpp +++ b/tests/std/tests/GH_003022_substr_allocator/test.cpp @@ -12,17 +12,17 @@ using namespace std; #if _HAS_CXX20 #define CONSTEXPR20 constexpr -#else // ^^^ _HAS_CXX20 / vvv !_HAS_CXX20 +#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv #define CONSTEXPR20 inline -#endif // _HAS_CXX20 +#endif // ^^^ !_HAS_CXX20 ^^^ #define TEST_ASSERT(...) assert((__VA_ARGS__)) #ifdef __cpp_char8_t using char8_type = char8_t; -#else // ^^^ defined(__cpp_char8_t) / vvv !defined(__cpp_char8_t) +#else // ^^^ defined(__cpp_char8_t) / !defined(__cpp_char8_t) vvv using char8_type = unsigned char; -#endif // __cpp_char8_t +#endif // ^^^ !defined(__cpp_char8_t) ^^^ template , int> = 0> constexpr auto statically_widen_impl( @@ -56,9 +56,9 @@ constexpr auto statically_widen_impl( #ifdef __cpp_char8_t #define STATICALLY_WIDEN(CT, S) (statically_widen_impl(S, u8##S, u##S, U##S, L##S)) -#else // ^^^ defined(__cpp_char8_t) / vvv !defined(__cpp_char8_t) +#else // ^^^ defined(__cpp_char8_t) / !defined(__cpp_char8_t) vvv #define STATICALLY_WIDEN(CT, S) (statically_widen_impl(S, nullptr, u##S, U##S, L##S)) -#endif // __cpp_char8_t +#endif // ^^^ !defined(__cpp_char8_t) ^^^ template class payloaded_allocator { @@ -176,7 +176,7 @@ CONSTEXPR20 bool test_substr_allocator() { #if _HAS_CXX23 && !defined(__EDG__) // TRANSITION, EDG doesn't perform overload resolution correctly static_assert(!is_constructible_v, "This should be ambiguous"); -#else +#else // _HAS_CXX23 && !defined(__EDG__) static_assert(is_constructible_v, "This should be unambiguous"); #endif // _HAS_CXX23 && !defined(__EDG__) diff --git a/tests/std/tests/P2438R2_substr_rvalue/test.cpp b/tests/std/tests/P2438R2_substr_rvalue/test.cpp index 4462f095c2..772c3e5701 100644 --- a/tests/std/tests/P2438R2_substr_rvalue/test.cpp +++ b/tests/std/tests/P2438R2_substr_rvalue/test.cpp @@ -76,9 +76,9 @@ consteval auto statically_widen_impl(string_view sv, #ifdef __cpp_char8_t #define STATICALLY_WIDEN(CT, S) (statically_widen_impl(S##sv, u8##S##sv, u##S##sv, U##S##sv, L##S##sv)) -#else // ^^^ defined(__cpp_char8_t) / vvv !defined(__cpp_char8_t) +#else // ^^^ defined(__cpp_char8_t) / !defined(__cpp_char8_t) vvv #define STATICALLY_WIDEN(CT, S) (statically_widen_impl(S##sv, u##S##sv, U##S##sv, L##S##sv)) -#endif // __cpp_char8_t +#endif // ^^^ !defined(__cpp_char8_t) ^^^ template constexpr bool test_rvalue_substr() { From 87b6012629670534f1526fefee25cf2c3edf9b6e Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 30 Aug 2022 18:06:50 -0700 Subject: [PATCH 27/28] Reported VSO-1601168. --- stl/inc/xstring | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 6305ca6f30..a5e2a3c0f7 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -376,7 +376,7 @@ public: _In_reads_(_Count) const _Elem* const _First2, const size_t _Count) noexcept /* strengthened */ { // compare [_First1, _First1 + _Count) with [_First2, ...) #if _HAS_CXX17 -#if _HAS_CXX20 && defined(__EDG__) // TRANSITION, EDG's __builtin_memcmp is buggy for constexpr dynamic allocation +#if _HAS_CXX20 && defined(__EDG__) // TRANSITION, VSO-1601168 if (_STD is_constant_evaluated()) { for (size_t _Idx = 0; _Idx != _Count; ++_Idx) { if (static_cast(_First1[_Idx]) < static_cast(_First2[_Idx])) { From 3fd829ea61e161a88eb1a2177f5b99905f62d4d5 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 30 Aug 2022 18:24:43 -0700 Subject: [PATCH 28/28] Reported VSO-1601179. --- tests/std/tests/GH_003022_substr_allocator/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/GH_003022_substr_allocator/test.cpp b/tests/std/tests/GH_003022_substr_allocator/test.cpp index 33dd702472..d6d26b3637 100644 --- a/tests/std/tests/GH_003022_substr_allocator/test.cpp +++ b/tests/std/tests/GH_003022_substr_allocator/test.cpp @@ -174,7 +174,7 @@ CONSTEXPR20 bool test_substr_allocator() { } }; -#if _HAS_CXX23 && !defined(__EDG__) // TRANSITION, EDG doesn't perform overload resolution correctly +#if _HAS_CXX23 && !defined(__EDG__) // TRANSITION, VSO-1601179 static_assert(!is_constructible_v, "This should be ambiguous"); #else // _HAS_CXX23 && !defined(__EDG__) static_assert(is_constructible_v, "This should be unambiguous");