From 3d857a526d566fa4d271050e3e72d49ad5e125e0 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 23 Sep 2024 01:35:05 +0800 Subject: [PATCH 1/7] Remove allocator-moving tagged internal constructors --- stl/inc/list | 5 - stl/inc/sstream | 2 +- stl/inc/xhash | 2 +- stl/inc/xmemory | 4 - stl/inc/xstring | 11 --- tests/std/test.lst | 1 + .../env.lst | 4 + .../test.cpp | 95 +++++++++++++++++++ 8 files changed, 102 insertions(+), 22 deletions(-) create mode 100644 tests/std/tests/GH_004929_internal_tag_constructors/env.lst create mode 100644 tests/std/tests/GH_004929_internal_tag_constructors/test.cpp diff --git a/stl/inc/list b/stl/inc/list index 033a43c60c7..96c23323db4 100644 --- a/stl/inc/list +++ b/stl/inc/list @@ -804,11 +804,6 @@ public: } private: - template - explicit list(_Move_allocator_tag, _Any_alloc& _Al) : _Mypair(_One_then_variadic_args_t{}, _STD move(_Al)) { - _Alloc_sentinel_and_proxy(); - } - void _Construct_n(_CRT_GUARDOVERFLOW size_type _Count) { auto&& _Alproxy = _GET_PROXY_ALLOCATOR(_Alnode, _Getal()); _Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _Mypair._Myval2); diff --git a/stl/inc/sstream b/stl/inc/sstream index 83e7ad49aa8..b148784abbf 100644 --- a/stl/inc/sstream +++ b/stl/inc/sstream @@ -239,7 +239,7 @@ public: // the buffer may already be full, and the terminating char is not '\0'. // In that case, copy the string as usual. _NODISCARD _Mystr str() && { - _Mystr _Result{_String_constructor_rvalue_allocator_tag{}, _STD move(_Al)}; + _Mystr _Result{_Al}; const auto _View = _Get_buffer_view(); // _Size cannot be larger than _Res, but it could be equal, // because basic_stringbuf doesn't allocate for the terminating '\0'. diff --git a/stl/inc/xhash b/stl/inc/xhash index 0581696c1af..e53d78d755d 100644 --- a/stl/inc/xhash +++ b/stl/inc/xhash @@ -398,7 +398,7 @@ protected: } _Hash(_Hash&& _Right) - : _Traitsobj(_Right._Traitsobj), _List(_Move_allocator_tag{}, _Right._List._Getal()), + : _Traitsobj(_Right._Traitsobj), _List(static_cast(_Right._List._Getal())), _Vec(_STD move(_Right._Vec._Mypair._Get_first())) { _Vec._Assign_grow(_Min_buckets * 2, _Unchecked_end()); _List._Swap_val(_Right._List); diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 3aa11f488c8..0af1a94bdc8 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1535,10 +1535,6 @@ public: } }; -struct _Move_allocator_tag { - explicit _Move_allocator_tag() = default; -}; - template pair<_Ty*, ptrdiff_t> _Get_temporary_buffer(ptrdiff_t _Count) noexcept { if (static_cast(_Count) <= static_cast(-1) / sizeof(_Ty)) { diff --git a/stl/inc/xstring b/stl/inc/xstring index 212d55880a5..e39cb1871a3 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -508,11 +508,6 @@ struct _String_constructor_concat_tag { explicit _String_constructor_concat_tag() = default; }; -struct _String_constructor_rvalue_allocator_tag { - // tag to select constructors used by basic_stringbuf's rvalue str() - explicit _String_constructor_rvalue_allocator_tag() = default; -}; - [[noreturn]] inline void _Xlen_string() { _Xlength_error("string too long"); } @@ -1145,12 +1140,6 @@ public: #endif // _HAS_CXX17 #if _HAS_CXX20 - basic_string(_String_constructor_rvalue_allocator_tag, _Alloc&& _Al) - : _Mypair(_One_then_variadic_args_t{}, _STD move(_Al)) { - // Used exclusively by basic_stringbuf - _Construct_empty(); - } - _NODISCARD bool _Move_assign_from_buffer( _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() diff --git a/tests/std/test.lst b/tests/std/test.lst index 6965091e415..3daaf646130 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -250,6 +250,7 @@ tests\GH_004618_mixed_operator_usage_keeps_statistical_properties tests\GH_004618_normal_distribution_avoids_resets tests\GH_004657_expected_constraints_permissive tests\GH_004845_logical_operator_traits_with_non_bool_constant +tests\GH_004929_internal_tag_constructors tests\GH_004930_char_traits_user_specialization tests\LWG2381_num_get_floating_point tests\LWG2597_complex_branch_cut diff --git a/tests/std/tests/GH_004929_internal_tag_constructors/env.lst b/tests/std/tests/GH_004929_internal_tag_constructors/env.lst new file mode 100644 index 00000000000..19f025bd0e6 --- /dev/null +++ b/tests/std/tests/GH_004929_internal_tag_constructors/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_004929_internal_tag_constructors/test.cpp b/tests/std/tests/GH_004929_internal_tag_constructors/test.cpp new file mode 100644 index 00000000000..523bb2b5abd --- /dev/null +++ b/tests/std/tests/GH_004929_internal_tag_constructors/test.cpp @@ -0,0 +1,95 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include +#include +#include +#if _HAS_CXX17 +#include +#endif // _HAS_CXX17 + +#if _HAS_CXX20 +#define CONSTEXPR20 constexpr +#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv +#define CONSTEXPR20 inline +#endif // ^^^ !_HAS_CXX20 ^^^ + +using namespace std; + +template +constexpr bool is_initializer_list = false; +template +constexpr bool is_initializer_list> = true; + +template +constexpr initializer_list ilist42 = {T{'4'}, T{'2'}}; + +template +constexpr bool is_basic_string_or_cstr_or_view = false; +template +constexpr bool is_basic_string_or_cstr_or_view> = true; +template +constexpr bool is_basic_string_or_cstr_or_view = true; +#if _HAS_CXX17 +template +constexpr bool is_basic_string_or_cstr_or_view> = true; +#endif // _HAS_CXX17 + +struct nasty_string_source { + template , int> = 0> + constexpr operator IList() const { + return ilist42; + } + + template && !is_basic_string_or_cstr_or_view, int> = 0> + constexpr operator T() const { + return T{}; + } +}; + +CONSTEXPR20 bool test_nasty_conversion_to_basic_string() { + using namespace std::literals; + + assert(string(nasty_string_source{}, allocator{}) == "42"s); +#ifdef __cpp_char8_t + assert(u8string(nasty_string_source{}, allocator{}) == u8"42"s); +#endif // defined (__cpp_char8_t) + assert(u16string(nasty_string_source{}, allocator{}) == u"42"s); + assert(u32string(nasty_string_source{}, allocator{}) == U"42"s); + assert(wstring(nasty_string_source{}, allocator{}) == L"42"s); + + return true; +} + +template +constexpr bool is_list = false; +template +constexpr bool is_list> = true; + +struct nasty_list_source { + template , int> = 0> + constexpr operator IList() const { + return ilist42; + } + + template && !is_list && !is_integral_v, int> = 0> + constexpr operator T() const { + return T{}; + } +}; + +void test_nasty_conversion_to_list() { + allocator ator{}; + assert((list{nasty_list_source{}, ator} == list{int{'4'}, int{'2'}})); +} + +#if _HAS_CXX20 +static_assert(test_nasty_conversion_to_basic_string()); +#endif // _HAS_CXX20 + +int main() { + test_nasty_conversion_to_basic_string(); + test_nasty_conversion_to_list(); +} From 425e3ab3f1729932ab600fa67b7589aceaebb883 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 23 Sep 2024 01:43:53 +0800 Subject: [PATCH 2/7] Skip tests --- tests/libcxx/expected_results.txt | 6 ++++++ tests/std/tests/VSO_0102478_moving_allocators/test.cpp | 8 ++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/libcxx/expected_results.txt b/tests/libcxx/expected_results.txt index a9bab3c4f52..09931c23321 100644 --- a/tests/libcxx/expected_results.txt +++ b/tests/libcxx/expected_results.txt @@ -581,6 +581,12 @@ std/utilities/format/format.formatter/format.formatter.locking/enable_nonlocking std/containers/sequences/vector.bool/shrink_to_fit.pass.cpp FAIL std/containers/sequences/vector/vector.capacity/shrink_to_fit.pass.cpp FAIL +# libc++ tests the counts of move constructions of allocators, which is possibly bogus, see GH-4976 +std/containers/unord/unord.map/unord.map.cnstr/move.pass.cpp FAIL +std/containers/unord/unord.multimap/unord.multimap.cnstr/move.pass.cpp FAIL +std/containers/unord/unord.multiset/unord.multiset.cnstr/move.pass.cpp FAIL +std/containers/unord/unord.set/unord.set.cnstr/move.pass.cpp FAIL + # *** LIKELY STL BUGS *** # Not analyzed, likely STL bugs. Various assertions. diff --git a/tests/std/tests/VSO_0102478_moving_allocators/test.cpp b/tests/std/tests/VSO_0102478_moving_allocators/test.cpp index c11a47b51b0..1348d859cc6 100644 --- a/tests/std/tests/VSO_0102478_moving_allocators/test.cpp +++ b/tests/std/tests/VSO_0102478_moving_allocators/test.cpp @@ -436,13 +436,13 @@ int main() { container_test(); container_test(); container_test(); - container_test(); - container_test(); + // container_test(); // skipped, possibly bogus, see GH-4976 + // container_test(); // skipped, possibly bogus, see GH-4976 dictionary_test(); dictionary_test(); - dictionary_test(); - dictionary_test(); + // dictionary_test(); // skipped, possibly bogus, see GH-4976 + // dictionary_test(); // skipped, possibly bogus, see GH-4976 string_test(); From 5147e59ad8b128618cefae96b7067e937ae53d82 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 23 Sep 2024 09:31:04 +0800 Subject: [PATCH 3/7] Revert skipping tests and try another method This reverts commit 425e3ab3f1729932ab600fa67b7589aceaebb883. --- stl/inc/xhash | 5 ++--- tests/libcxx/expected_results.txt | 6 ------ tests/std/tests/VSO_0102478_moving_allocators/test.cpp | 8 ++++---- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/stl/inc/xhash b/stl/inc/xhash index e53d78d755d..c45039e984c 100644 --- a/stl/inc/xhash +++ b/stl/inc/xhash @@ -398,10 +398,9 @@ protected: } _Hash(_Hash&& _Right) - : _Traitsobj(_Right._Traitsobj), _List(static_cast(_Right._List._Getal())), + : _Traitsobj(_Right._Traitsobj), _List(_STD move(_Right._List)), _Vec(_STD move(_Right._Vec._Mypair._Get_first())) { - _Vec._Assign_grow(_Min_buckets * 2, _Unchecked_end()); - _List._Swap_val(_Right._List); + _Vec._Assign_grow(_Min_buckets * 2, _Right._Unchecked_end()); _Vec._Mypair._Myval2._Swap_val(_Right._Vec._Mypair._Myval2); _Mask = _STD exchange(_Right._Mask, _Min_buckets - 1); _Maxidx = _STD exchange(_Right._Maxidx, _Min_buckets); diff --git a/tests/libcxx/expected_results.txt b/tests/libcxx/expected_results.txt index 09931c23321..a9bab3c4f52 100644 --- a/tests/libcxx/expected_results.txt +++ b/tests/libcxx/expected_results.txt @@ -581,12 +581,6 @@ std/utilities/format/format.formatter/format.formatter.locking/enable_nonlocking std/containers/sequences/vector.bool/shrink_to_fit.pass.cpp FAIL std/containers/sequences/vector/vector.capacity/shrink_to_fit.pass.cpp FAIL -# libc++ tests the counts of move constructions of allocators, which is possibly bogus, see GH-4976 -std/containers/unord/unord.map/unord.map.cnstr/move.pass.cpp FAIL -std/containers/unord/unord.multimap/unord.multimap.cnstr/move.pass.cpp FAIL -std/containers/unord/unord.multiset/unord.multiset.cnstr/move.pass.cpp FAIL -std/containers/unord/unord.set/unord.set.cnstr/move.pass.cpp FAIL - # *** LIKELY STL BUGS *** # Not analyzed, likely STL bugs. Various assertions. diff --git a/tests/std/tests/VSO_0102478_moving_allocators/test.cpp b/tests/std/tests/VSO_0102478_moving_allocators/test.cpp index 1348d859cc6..c11a47b51b0 100644 --- a/tests/std/tests/VSO_0102478_moving_allocators/test.cpp +++ b/tests/std/tests/VSO_0102478_moving_allocators/test.cpp @@ -436,13 +436,13 @@ int main() { container_test(); container_test(); container_test(); - // container_test(); // skipped, possibly bogus, see GH-4976 - // container_test(); // skipped, possibly bogus, see GH-4976 + container_test(); + container_test(); dictionary_test(); dictionary_test(); - // dictionary_test(); // skipped, possibly bogus, see GH-4976 - // dictionary_test(); // skipped, possibly bogus, see GH-4976 + dictionary_test(); + dictionary_test(); string_test(); From dd95f98a197bc5aa084112e097880c99d1f54770 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 27 Sep 2024 12:57:08 -0700 Subject: [PATCH 4/7] Restore basic EH guarantee. --- stl/inc/xhash | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/stl/inc/xhash b/stl/inc/xhash index c45039e984c..a62949ccbb6 100644 --- a/stl/inc/xhash +++ b/stl/inc/xhash @@ -398,9 +398,10 @@ protected: } _Hash(_Hash&& _Right) - : _Traitsobj(_Right._Traitsobj), _List(_STD move(_Right._List)), + : _Traitsobj(_Right._Traitsobj), _List(_Right._List._Getal()), _Vec(_STD move(_Right._Vec._Mypair._Get_first())) { - _Vec._Assign_grow(_Min_buckets * 2, _Right._Unchecked_end()); + _Vec._Assign_grow(_Min_buckets * 2, _Unchecked_end()); + _List._Swap_val(_Right._List); _Vec._Mypair._Myval2._Swap_val(_Right._Vec._Mypair._Myval2); _Mask = _STD exchange(_Right._Mask, _Min_buckets - 1); _Maxidx = _STD exchange(_Right._Maxidx, _Min_buckets); From 9dca10b6076d1895f56059822bed3c12fe68ea6b Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 27 Sep 2024 13:00:04 -0700 Subject: [PATCH 5/7] Nitpicks. --- tests/std/tests/GH_004929_internal_tag_constructors/test.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/std/tests/GH_004929_internal_tag_constructors/test.cpp b/tests/std/tests/GH_004929_internal_tag_constructors/test.cpp index 523bb2b5abd..9509fe13142 100644 --- a/tests/std/tests/GH_004929_internal_tag_constructors/test.cpp +++ b/tests/std/tests/GH_004929_internal_tag_constructors/test.cpp @@ -50,12 +50,10 @@ struct nasty_string_source { }; CONSTEXPR20 bool test_nasty_conversion_to_basic_string() { - using namespace std::literals; - assert(string(nasty_string_source{}, allocator{}) == "42"s); #ifdef __cpp_char8_t assert(u8string(nasty_string_source{}, allocator{}) == u8"42"s); -#endif // defined (__cpp_char8_t) +#endif // defined(__cpp_char8_t) assert(u16string(nasty_string_source{}, allocator{}) == u"42"s); assert(u32string(nasty_string_source{}, allocator{}) == U"42"s); assert(wstring(nasty_string_source{}, allocator{}) == L"42"s); From caac20e81fd6d798b4a41a6234eaa05e0d05a1a7 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 27 Sep 2024 14:40:37 -0700 Subject: [PATCH 6/7] Restore list machinery, but make it invisible. --- stl/inc/list | 9 +++++++++ stl/inc/xhash | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/stl/inc/list b/stl/inc/list index 96c23323db4..819e3d7eb96 100644 --- a/stl/inc/list +++ b/stl/inc/list @@ -751,6 +751,10 @@ private: template class _Hash; +struct _Move_allocator_tag { + explicit _Move_allocator_tag() = default; +}; + _EXPORT_STD template > class list { // bidirectional linked list private: @@ -804,6 +808,11 @@ public: } private: + template , int> = 0> + explicit list(_Tag, _Any_alloc& _Al) : _Mypair(_One_then_variadic_args_t{}, _STD move(_Al)) { + _Alloc_sentinel_and_proxy(); + } + void _Construct_n(_CRT_GUARDOVERFLOW size_type _Count) { auto&& _Alproxy = _GET_PROXY_ALLOCATOR(_Alnode, _Getal()); _Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _Mypair._Myval2); diff --git a/stl/inc/xhash b/stl/inc/xhash index a62949ccbb6..0581696c1af 100644 --- a/stl/inc/xhash +++ b/stl/inc/xhash @@ -398,7 +398,7 @@ protected: } _Hash(_Hash&& _Right) - : _Traitsobj(_Right._Traitsobj), _List(_Right._List._Getal()), + : _Traitsobj(_Right._Traitsobj), _List(_Move_allocator_tag{}, _Right._List._Getal()), _Vec(_STD move(_Right._Vec._Mypair._Get_first())) { _Vec._Assign_grow(_Min_buckets * 2, _Unchecked_end()); _List._Swap_val(_Right._List); From 0e5a01460572474a60b1368c623f9f4dfea79e24 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 27 Sep 2024 19:14:14 -0700 Subject: [PATCH 7/7] Fix /clr:pure, the nemesis of humanity. --- tests/std/tests/GH_004929_internal_tag_constructors/env.lst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/GH_004929_internal_tag_constructors/env.lst b/tests/std/tests/GH_004929_internal_tag_constructors/env.lst index 19f025bd0e6..f141421b292 100644 --- a/tests/std/tests/GH_004929_internal_tag_constructors/env.lst +++ b/tests/std/tests/GH_004929_internal_tag_constructors/env.lst @@ -1,4 +1,4 @@ # Copyright (c) Microsoft Corporation. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -RUNALL_INCLUDE ..\usual_matrix.lst +RUNALL_INCLUDE ..\impure_matrix.lst