From 8131df8a3e97715562df6a979d932f4454c6214f Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 25 Nov 2024 21:21:22 +0800 Subject: [PATCH 1/8] Implement LWG-3918 --- stl/inc/execution | 5 - stl/inc/memory | 4 +- stl/inc/xmemory | 32 ++++- .../test.cpp | 119 ++++++++++++++++++ 4 files changed, 151 insertions(+), 9 deletions(-) diff --git a/stl/inc/execution b/stl/inc/execution index a761ef58ed6..9362037496e 100644 --- a/stl/inc/execution +++ b/stl/inc/execution @@ -137,11 +137,6 @@ template <> struct is_execution_policy : true_type {}; #endif // _HAS_CXX20 -template -void _Construct_in_place_by_deref(_Ty& _Val, const _FwdIt& _Iter) { - ::new (static_cast(_STD addressof(_Val))) _Ty(*_Iter); -} - template void _Construct_in_place_by_transform_deref(_Ty& _Val, _UnaryOp _Transform_op, const _FwdIt& _Iter) { ::new (static_cast(_STD addressof(_Val))) _Ty(_Transform_op(*_Iter)); diff --git a/stl/inc/memory b/stl/inc/memory index 67a10039d0b..0e963a59e0a 100644 --- a/stl/inc/memory +++ b/stl/inc/memory @@ -141,7 +141,7 @@ _NoThrowFwdIt uninitialized_copy_n(const _InIt _First, const _Diff _Count_raw, _ _Uninitialized_backout _Backout{_UDest}; for (; _Count > 0; --_Count, (void) ++_UFirst) { - _Backout._Emplace_back(*_UFirst); + _Backout._Emplace_back_deref(_UFirst); } _UDest = _Backout._Release(); @@ -294,7 +294,7 @@ pair<_InIt, _NoThrowFwdIt> uninitialized_move_n(_InIt _First, const _Diff _Count _Uninitialized_backout _Backout{_UDest}; for (; _Count > 0; --_Count, (void) ++_UFirst) { - _Backout._Emplace_back(_STD move(*_UFirst)); + _Backout._Emplace_back_deref_move(_UFirst); } _UDest = _Backout._Release(); diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 6060a08635f..9368adcbe41 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1601,6 +1601,11 @@ void _Return_temporary_buffer(_Ty* const _Pbuf) noexcept { } } +template +void _Construct_in_place_by_deref(_Ty& _Val, const _InIt& _Iter) { + ::new (static_cast(_STD addressof(_Val))) _Ty(*_Iter); +} + template struct _NODISCARD _Uninitialized_backout { // struct to undo partially constructed ranges in _Uninitialized_xxx algorithms @@ -1625,6 +1630,29 @@ struct _NODISCARD _Uninitialized_backout { ++_Last; } + template + _CONSTEXPR20 void _Emplace_back_deref(const _InIt& _Iter) { + // construct a new element at *_Last from the result of dereferencing _Iter and increment. + if constexpr (is_reference_v) { + _STD _Construct_in_place(*_Last, *_Iter); + } else { + _STD _Construct_in_place_by_deref(*_Last, _Iter); + } + ++_Last; + } + + template + _CONSTEXPR20 void _Emplace_back_deref_move(const _InIt& _Iter) { + // construct a new element at *_Last from the result of dereferencing _Iter and increment, + // with lvalue cast to xvalue if necessary for uninitialized_move(_n). + if constexpr (is_reference_v) { + _STD _Construct_in_place(*_Last, _STD move(*_Iter)); + } else { + _STD _Construct_in_place_by_deref(*_Last, _Iter); + } + ++_Last; + } + constexpr _NoThrowFwdIt _Release() { // suppress any exception handling backout and return _Last _First = _Last; return _Last; @@ -1644,7 +1672,7 @@ _CONSTEXPR20 _NoThrowFwdIt _Uninitialized_move_unchecked(_InIt _First, const _In } _Uninitialized_backout<_NoThrowFwdIt> _Backout{_Dest}; for (; _First != _Last; ++_First) { - _Backout._Emplace_back(_STD move(*_First)); + _Backout._Emplace_back_deref_move(_First); } return _Backout._Release(); @@ -1918,7 +1946,7 @@ _CONSTEXPR20 _NoThrowFwdIt _Uninitialized_copy_unchecked(_InIt _First, const _In _Uninitialized_backout<_NoThrowFwdIt> _Backout{_Dest}; for (; _First != _Last; ++_First) { - _Backout._Emplace_back(*_First); + _Backout._Emplace_back_deref(_First); } return _Backout._Release(); diff --git a/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp b/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp index 2e122c02853..410b1cd1560 100644 --- a/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp +++ b/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp @@ -196,6 +196,120 @@ void test_destroy_n() { assert(g_alive == 0); } +struct copy_elision_dest; + +class pinned { +public: + explicit pinned(int n) : n_{n} {} + + pinned(const pinned&) = delete; + pinned& operator=(const pinned&) = delete; + +private: + friend copy_elision_dest; + + int n_; +}; + +class pinned_ioterator { +private: + struct arrow_proxy { + pinned val_; + + pinned* operator->() { + return &val_; + } + }; + +public: + using value_type = pinned; + using difference_type = int; + using reference = pinned; + using pointer = arrow_proxy; + using iterator_category = std::input_iterator_tag; + + explicit pinned_ioterator(int n) : n_{n} {} + + pinned operator*() const { + return pinned{n_}; + } + pinned_ioterator& operator++() { + ++n_; + return *this; + } + pinned_ioterator operator++(int) { + auto old = *this; + ++*this; + return old; + } + + arrow_proxy operator->() const { + return arrow_proxy{pinned{n_}}; + } + + friend bool operator==(pinned_ioterator i, pinned_ioterator j) { + return i.n_ == j.n_; + } +#if !_HAS_CXX20 + friend bool operator!=(pinned_ioterator i, pinned_ioterator j) { + return !(i == j); + } +#endif // !_HAS_CXX20 + +private: + int n_; +}; + +struct copy_elision_dest { +public: + copy_elision_dest() = default; + explicit copy_elision_dest(pinned x) : n_{x.n_} {} + + int n_; +}; + +// std::uninitialized_copy/_n are required to perform guaranteed copy elision since C++17. +void test_guaranteed_copy_elision_uninitialized_copy() { + constexpr int len = 42; + + copy_elision_dest d[len]; + uninitialized_copy(pinned_ioterator{0}, pinned_ioterator{len}, d); + for (int i = 0; i != len; ++i) { + assert(d[i].n_ == i); + } +} + +void test_guaranteed_copy_elision_uninitialized_copy_n() { + constexpr int len = 42; + + copy_elision_dest d[len]; + uninitialized_copy_n(pinned_ioterator{0}, len, d); + for (int i = 0; i != len; ++i) { + assert(d[i].n_ == i); + } +} + +// Also test LWG-3918 "std::uninitialized_move/_n and guaranteed copy elision". +void test_guaranteed_copy_elision_uninitialized_move() { + constexpr int len = 42; + + copy_elision_dest d[len]; + uninitialized_move(pinned_ioterator{0}, pinned_ioterator{len}, d); + for (int i = 0; i != len; ++i) { + assert(d[i].n_ == i); + } +} + +void test_guaranteed_copy_elision_uninitialized_move_n() { + constexpr int len = 42; + + copy_elision_dest d[len]; + uninitialized_move_n(pinned_ioterator{0}, len, d); + for (int i = 0; i != len; ++i) { + assert(d[i].n_ == i); + } +} + int main() { test_uninitialized_move(); test_uninitialized_move_n(); @@ -206,4 +320,9 @@ int main() { test_destroy_at(); test_destroy(); test_destroy_n(); + + test_guaranteed_copy_elision_uninitialized_copy(); + test_guaranteed_copy_elision_uninitialized_copy_n(); + test_guaranteed_copy_elision_uninitialized_move(); + test_guaranteed_copy_elision_uninitialized_move_n(); } From fb30da04e535a9f1a93343a784b5a3dd6e235179 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Tue, 26 Nov 2024 01:42:53 +0800 Subject: [PATCH 2/8] Address review comments on styles --- .../P0040R3_extending_memory_management_tools/test.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp b/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp index 410b1cd1560..e008910189d 100644 --- a/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp +++ b/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp @@ -222,11 +222,11 @@ class pinned_ioterator { }; public: - using value_type = pinned; + using iterator_category = std::input_iterator_tag; using difference_type = int; - using reference = pinned; + using value_type = pinned; using pointer = arrow_proxy; - using iterator_category = std::input_iterator_tag; + using reference = pinned; explicit pinned_ioterator(int n) : n_{n} {} @@ -261,7 +261,6 @@ class pinned_ioterator { }; struct copy_elision_dest { -public: copy_elision_dest() = default; explicit copy_elision_dest(pinned x) : n_{x.n_} {} From 8c12faf30b36476dd076fe8564c176dcc514024f Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Tue, 26 Nov 2024 10:08:44 +0800 Subject: [PATCH 3/8] Drop `_CONSTEXPR20` when there should be `_CONSTEXPR26` --- stl/inc/xmemory | 24 ++++++++----------- .../tests/P0784R7_library_machinery/test.cpp | 6 +++++ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 9368adcbe41..28fac9c627a 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1631,21 +1631,17 @@ struct _NODISCARD _Uninitialized_backout { } template - _CONSTEXPR20 void _Emplace_back_deref(const _InIt& _Iter) { + void _Emplace_back_deref(const _InIt& _Iter) { // construct a new element at *_Last from the result of dereferencing _Iter and increment. - if constexpr (is_reference_v) { - _STD _Construct_in_place(*_Last, *_Iter); - } else { - _STD _Construct_in_place_by_deref(*_Last, _Iter); - } + _STD _Construct_in_place_by_deref(*_Last, _Iter); ++_Last; } template - _CONSTEXPR20 void _Emplace_back_deref_move(const _InIt& _Iter) { + void _Emplace_back_deref_move(const _InIt& _Iter) { // construct a new element at *_Last from the result of dereferencing _Iter and increment, // with lvalue cast to xvalue if necessary for uninitialized_move(_n). - if constexpr (is_reference_v) { + if constexpr (is_lvalue_reference_v) { _STD _Construct_in_place(*_Last, _STD move(*_Iter)); } else { _STD _Construct_in_place_by_deref(*_Last, _Iter); @@ -1660,12 +1656,12 @@ struct _NODISCARD _Uninitialized_backout { }; template -_CONSTEXPR20 _NoThrowFwdIt _Uninitialized_move_unchecked(_InIt _First, const _InIt _Last, _NoThrowFwdIt _Dest) { +_NoThrowFwdIt _Uninitialized_move_unchecked(_InIt _First, const _InIt _Last, _NoThrowFwdIt _Dest) { // move [_First, _Last) to raw [_Dest, ...) if constexpr (_Iter_move_cat<_InIt, _NoThrowFwdIt>::_Bitcopy_constructible) { -#if _HAS_CXX20 +#if 0 // TRANSITION, _HAS_CXX26 if (!_STD is_constant_evaluated()) -#endif // _HAS_CXX20 +#endif // _HAS_CXX26 { return _STD _Copy_memmove(_First, _Last, _Dest); } @@ -1933,12 +1929,12 @@ _CONSTEXPR20 _Alloc_ptr_t<_Alloc> _Uninitialized_copy_n( } template -_CONSTEXPR20 _NoThrowFwdIt _Uninitialized_copy_unchecked(_InIt _First, const _InIt _Last, _NoThrowFwdIt _Dest) { +_NoThrowFwdIt _Uninitialized_copy_unchecked(_InIt _First, const _InIt _Last, _NoThrowFwdIt _Dest) { // copy [_First, _Last) to raw [_Dest, ...) if constexpr (_Iter_copy_cat<_InIt, _NoThrowFwdIt>::_Bitcopy_constructible) { -#if _HAS_CXX20 +#if 0 // TRANSITION, _HAS_CXX26 if (!_STD is_constant_evaluated()) -#endif // _HAS_CXX20 +#endif // _HAS_CXX26 { return _STD _Copy_memmove(_First, _Last, _Dest); } diff --git a/tests/std/tests/P0784R7_library_machinery/test.cpp b/tests/std/tests/P0784R7_library_machinery/test.cpp index 45b50757802..67ec32be837 100644 --- a/tests/std/tests/P0784R7_library_machinery/test.cpp +++ b/tests/std/tests/P0784R7_library_machinery/test.cpp @@ -84,6 +84,9 @@ constexpr bool test() { assert(equal(begin(expected_copy), end(expected_copy), begin(output), end(output))); } +#if 1 // TRANSITION, !_HAS_CXX26 + if (!is_constant_evaluated()) +#endif // !_HAS_CXX26 { // _Uninitialized_copy_unchecked int_wrapper_copy input[] = {1, 2, 3, 4}; int_wrapper_copy output[4]; @@ -133,6 +136,9 @@ constexpr bool test() { } } +#if 1 // TRANSITION, !_HAS_CXX26 + if (!is_constant_evaluated()) +#endif // !_HAS_CXX26 { // _Uninitialized_move_unchecked int_wrapper_move input[] = {1, 2, 3, 4}; int_wrapper_move output[4]; From eefb7b9ed3d405f464d988e48d62271cd9df8330 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Tue, 26 Nov 2024 10:09:24 +0800 Subject: [PATCH 4/8] Strengthen exception specification for consistency --- stl/inc/xmemory | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 28fac9c627a..d57bf3c8cc5 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1602,7 +1602,8 @@ void _Return_temporary_buffer(_Ty* const _Pbuf) noexcept { } template -void _Construct_in_place_by_deref(_Ty& _Val, const _InIt& _Iter) { +void _Construct_in_place_by_deref(_Ty& _Val, const _InIt& _Iter) + noexcept(noexcept(::new (static_cast(_STD addressof(_Val))) _Ty(*_Iter))) { ::new (static_cast(_STD addressof(_Val))) _Ty(*_Iter); } From 9b646be840a4f4b6fb998c9c818d865220e4b776 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Tue, 26 Nov 2024 11:59:45 +0800 Subject: [PATCH 5/8] Unexpected clang-format --- stl/inc/xmemory | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/stl/inc/xmemory b/stl/inc/xmemory index d57bf3c8cc5..a5efe180402 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1663,9 +1663,7 @@ _NoThrowFwdIt _Uninitialized_move_unchecked(_InIt _First, const _InIt _Last, _No #if 0 // TRANSITION, _HAS_CXX26 if (!_STD is_constant_evaluated()) #endif // _HAS_CXX26 - { - return _STD _Copy_memmove(_First, _Last, _Dest); - } + { return _STD _Copy_memmove(_First, _Last, _Dest); } } _Uninitialized_backout<_NoThrowFwdIt> _Backout{_Dest}; for (; _First != _Last; ++_First) { @@ -1936,9 +1934,7 @@ _NoThrowFwdIt _Uninitialized_copy_unchecked(_InIt _First, const _InIt _Last, _No #if 0 // TRANSITION, _HAS_CXX26 if (!_STD is_constant_evaluated()) #endif // _HAS_CXX26 - { - return _STD _Copy_memmove(_First, _Last, _Dest); - } + { return _STD _Copy_memmove(_First, _Last, _Dest); } } _Uninitialized_backout<_NoThrowFwdIt> _Backout{_Dest}; From 610fa4aa3fa83bf19356b087f1319007f87a9b2e Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 26 Nov 2024 14:00:30 -0800 Subject: [PATCH 6/8] Enhance pre-existing `uninitialized_storage` to use `fillChar`. --- .../tests/P0040R3_extending_memory_management_tools/test.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp b/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp index e008910189d..7dff1aff3c4 100644 --- a/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp +++ b/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp @@ -58,6 +58,10 @@ template struct uninitialized_storage { alignas(T) char storage[sizeof(T) * Count]; + uninitialized_storage() { + fill(std::begin(storage), std::end(storage), fillChar); + } + T* begin() { return &reinterpret_cast(storage); } From 31c657e9d78c8266c6ed8b649e9b685e29d6bc2d Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 26 Nov 2024 14:03:37 -0800 Subject: [PATCH 7/8] Use `uninitialized_storage` for `copy_elision_dest`, drop default ctor. --- .../test.cpp | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp b/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp index 7dff1aff3c4..b2c7b85757e 100644 --- a/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp +++ b/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp @@ -265,7 +265,6 @@ class pinned_ioterator { }; struct copy_elision_dest { - copy_elision_dest() = default; explicit copy_elision_dest(pinned x) : n_{x.n_} {} int n_; @@ -275,42 +274,46 @@ struct copy_elision_dest { void test_guaranteed_copy_elision_uninitialized_copy() { constexpr int len = 42; - copy_elision_dest d[len]; - uninitialized_copy(pinned_ioterator{0}, pinned_ioterator{len}, d); + uninitialized_storage us; + uninitialized_copy(pinned_ioterator{0}, pinned_ioterator{len}, us.begin()); for (int i = 0; i != len; ++i) { - assert(d[i].n_ == i); + assert(us.begin()[i].n_ == i); } + destroy(us.begin(), us.end()); } void test_guaranteed_copy_elision_uninitialized_copy_n() { constexpr int len = 42; - copy_elision_dest d[len]; - uninitialized_copy_n(pinned_ioterator{0}, len, d); + uninitialized_storage us; + uninitialized_copy_n(pinned_ioterator{0}, len, us.begin()); for (int i = 0; i != len; ++i) { - assert(d[i].n_ == i); + assert(us.begin()[i].n_ == i); } + destroy(us.begin(), us.end()); } // Also test LWG-3918 "std::uninitialized_move/_n and guaranteed copy elision". void test_guaranteed_copy_elision_uninitialized_move() { constexpr int len = 42; - copy_elision_dest d[len]; - uninitialized_move(pinned_ioterator{0}, pinned_ioterator{len}, d); + uninitialized_storage us; + uninitialized_move(pinned_ioterator{0}, pinned_ioterator{len}, us.begin()); for (int i = 0; i != len; ++i) { - assert(d[i].n_ == i); + assert(us.begin()[i].n_ == i); } + destroy(us.begin(), us.end()); } void test_guaranteed_copy_elision_uninitialized_move_n() { constexpr int len = 42; - copy_elision_dest d[len]; - uninitialized_move_n(pinned_ioterator{0}, len, d); + uninitialized_storage us; + uninitialized_move_n(pinned_ioterator{0}, len, us.begin()); for (int i = 0; i != len; ++i) { - assert(d[i].n_ == i); + assert(us.begin()[i].n_ == i); } + destroy(us.begin(), us.end()); } int main() { From 06fc108c3bbf87b6ade04ada684701b08b8ea16f Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 26 Nov 2024 14:09:32 -0800 Subject: [PATCH 8/8] Drop `std::`. --- .../tests/P0040R3_extending_memory_management_tools/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp b/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp index b2c7b85757e..26b53c0e6e0 100644 --- a/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp +++ b/tests/std/tests/P0040R3_extending_memory_management_tools/test.cpp @@ -226,7 +226,7 @@ class pinned_ioterator { }; public: - using iterator_category = std::input_iterator_tag; + using iterator_category = input_iterator_tag; using difference_type = int; using value_type = pinned; using pointer = arrow_proxy;