From 902549576ac2cb811f6c6e9217b6844bd6ced337 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 12 Mar 2024 03:52:14 -0700 Subject: [PATCH 01/11] `std::` => `_STD` --- stl/inc/generator | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/generator b/stl/inc/generator index 20a3b4d4046..b55fe21141a 100644 --- a/stl/inc/generator +++ b/stl/inc/generator @@ -231,7 +231,7 @@ public: requires same_as<_Gen_yield_t<_Gen_reference_t<_Rty, _Vty>>, _Yielded> _NODISCARD auto yield_value(_RANGES elements_of&&, _Unused> _Elem) noexcept { using _Nested_awaitable = _Nested_awaitable_provider<_Rty, _Vty, _Alloc>::_Awaitable; - return _Nested_awaitable{std::move(_Elem.range)}; + return _Nested_awaitable{_STD move(_Elem.range)}; } template <_RANGES input_range _Rng, class _Alloc> From e0c04c3c0a55799b588537623d8dee36a0c36538 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 12 Mar 2024 04:06:32 -0700 Subject: [PATCH 02/11] Include `` for `std::nullptr_t`. --- tests/std/tests/P2502R2_generator/test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/std/tests/P2502R2_generator/test.cpp b/tests/std/tests/P2502R2_generator/test.cpp index 75dcad61af8..ddc3af912ed 100644 --- a/tests/std/tests/P2502R2_generator/test.cpp +++ b/tests/std/tests/P2502R2_generator/test.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include From 0922ef55a2beddd686d065f91ff950239fbbc215 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 12 Mar 2024 04:13:11 -0700 Subject: [PATCH 03/11] Extract a verbose immediately-invoked lambda for clarity. --- stl/inc/generator | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/stl/inc/generator b/stl/inc/generator index b55fe21141a..5bb968256f4 100644 --- a/stl/inc/generator +++ b/stl/inc/generator @@ -239,12 +239,15 @@ public: _NODISCARD auto yield_value(_RANGES elements_of<_Rng, _Alloc> _Elem) { using _Vty = _RANGES range_value_t<_Rng>; using _Nested_awaitable = _Nested_awaitable_provider<_Yielded, _Vty, _Alloc>::_Awaitable; - return _Nested_awaitable{[](allocator_arg_t, _Alloc, _RANGES iterator_t<_Rng> _It, - const _RANGES sentinel_t<_Rng> _Se) -> generator<_Yielded, _Vty, _Alloc> { + + auto _Lambda = [](allocator_arg_t, _Alloc, _RANGES iterator_t<_Rng> _It, + const _RANGES sentinel_t<_Rng> _Se) -> generator<_Yielded, _Vty, _Alloc> { for (; _It != _Se; ++_It) { co_yield static_cast<_Yielded>(*_It); } - }(allocator_arg, _Elem.allocator, _RANGES begin(_Elem.range), _RANGES end(_Elem.range))}; + }; + return _Nested_awaitable{ + _Lambda(allocator_arg, _Elem.allocator, _RANGES begin(_Elem.range), _RANGES end(_Elem.range))}; } void await_transform() = delete; From 4694b3032b1863d12b9d2af5e5eca899fd4f7621 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 12 Mar 2024 04:16:00 -0700 Subject: [PATCH 04/11] Bugfix: Use `__stdcall` to make `` immune to default calling conventions. --- stl/inc/generator | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/generator b/stl/inc/generator index 5bb968256f4..53765134ba9 100644 --- a/stl/inc/generator +++ b/stl/inc/generator @@ -102,7 +102,7 @@ public: template <> class _Promise_allocator { // type-erased allocator private: - using _Dealloc_fn = void (*)(void*, size_t); + using _Dealloc_fn = void(__stdcall*)(void*, size_t); template static void* _Allocate(const _ProtoAlloc& _Proto, size_t _Size) { From 7fcd087483a7e3ee11d4c0bae61a1c9bedb724e1 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 12 Mar 2024 04:21:06 -0700 Subject: [PATCH 05/11] Scalar `constexpr size_t _Align` doesn't need to be `static`. --- stl/inc/generator | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/stl/inc/generator b/stl/inc/generator index 53765134ba9..d5884a40e9c 100644 --- a/stl/inc/generator +++ b/stl/inc/generator @@ -49,9 +49,9 @@ private: return _Al.allocate(_Count); } else { // store stateful allocator - static constexpr size_t _Align = (_STD max)(alignof(_Alloc), sizeof(_Aligned_block)); - const size_t _Count = (_Size + sizeof(_Alloc) + _Align - 1) / sizeof(_Aligned_block); - void* const _Ptr = _Al.allocate(_Count); + constexpr size_t _Align = (_STD max)(alignof(_Alloc), sizeof(_Aligned_block)); + const size_t _Count = (_Size + sizeof(_Alloc) + _Align - 1) / sizeof(_Aligned_block); + void* const _Ptr = _Al.allocate(_Count); const auto _Al_address = (reinterpret_cast(_Ptr) + _Size + alignof(_Alloc) - 1) & ~(alignof(_Alloc) - 1); ::new (reinterpret_cast(_Al_address)) _Alloc(_STD move(_Al)); @@ -92,8 +92,8 @@ public: _Alloc _Al{_STD move(_Stored_al)}; _Stored_al.~_Alloc(); - static constexpr size_t _Align = (_STD max)(alignof(_Alloc), sizeof(_Aligned_block)); - const size_t _Count = (_Size + sizeof(_Alloc) + _Align - 1) / sizeof(_Aligned_block); + constexpr size_t _Align = (_STD max)(alignof(_Alloc), sizeof(_Aligned_block)); + const size_t _Count = (_Size + sizeof(_Alloc) + _Align - 1) / sizeof(_Aligned_block); _Al.deallocate(static_cast<_Aligned_block*>(_Ptr), _Count); } } @@ -124,7 +124,7 @@ private: return _Ptr; } else { // store stateful allocator - static constexpr size_t _Align = (_STD max)(alignof(_Alloc), sizeof(_Aligned_block)); + constexpr size_t _Align = (_STD max)(alignof(_Alloc), sizeof(_Aligned_block)); const _Dealloc_fn _Dealloc = [](void* const _Ptr, size_t _Size) { _Size += sizeof(_Dealloc_fn); From ff0c920bc3ad664731505cafe6ad846b55d0bb68 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 12 Mar 2024 04:28:53 -0700 Subject: [PATCH 06/11] Fix unintentional move from `const _Alloc&`. This is now consistent with the other occurrence of `auto& _Stored_al`. --- stl/inc/generator | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/generator b/stl/inc/generator index d5884a40e9c..9dd8e8df617 100644 --- a/stl/inc/generator +++ b/stl/inc/generator @@ -130,7 +130,7 @@ private: _Size += sizeof(_Dealloc_fn); const auto _Al_address = (reinterpret_cast(_Ptr) + _Size + alignof(_Alloc) - 1) & ~(alignof(_Alloc) - 1); - auto& _Stored_al = *reinterpret_cast(_Al_address); + auto& _Stored_al = *reinterpret_cast<_Alloc*>(_Al_address); _Alloc _Al{_STD move(_Stored_al)}; _Stored_al.~_Alloc(); From dfbc9bf4681758fd8df8fd3484ff9a395c1f23e2 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 12 Mar 2024 04:41:42 -0700 Subject: [PATCH 07/11] Consistently use `_RANGES`. --- stl/inc/generator | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/generator b/stl/inc/generator index 9dd8e8df617..004e56f236b 100644 --- a/stl/inc/generator +++ b/stl/inc/generator @@ -412,7 +412,7 @@ private: }; _EXPORT_STD template -class generator : public ranges::view_interface> { +class generator : public _RANGES view_interface> { private: using _Value = _Gen_value_t<_Rty, _Vty>; static_assert(same_as, _Value> && is_object_v<_Value>, From f6f79c22060ce2b48c010a8e45f0263011036e3e Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 12 Mar 2024 04:51:33 -0700 Subject: [PATCH 08/11] `_Promise` => `_CoroPromise` to avoid intolerable shadowing of `` --- stl/inc/coroutine | 14 +++++++------- stl/inc/generator | 18 +++++++++--------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/stl/inc/coroutine b/stl/inc/coroutine index c41e6b36f59..94933610fbe 100644 --- a/stl/inc/coroutine +++ b/stl/inc/coroutine @@ -92,12 +92,12 @@ private: void* _Ptr = nullptr; }; -_EXPORT_STD template +_EXPORT_STD template struct coroutine_handle { constexpr coroutine_handle() noexcept = default; constexpr coroutine_handle(nullptr_t) noexcept {} - _NODISCARD static coroutine_handle from_promise(_Promise& _Prom) noexcept { // strengthened + _NODISCARD static coroutine_handle from_promise(_CoroPromise& _Prom) noexcept { // strengthened const auto _Prom_ptr = const_cast(static_cast(_STD addressof(_Prom))); const auto _Frame_ptr = __builtin_coro_promise(_Prom_ptr, 0, true); coroutine_handle _Result; @@ -144,8 +144,8 @@ struct coroutine_handle { __builtin_coro_destroy(_Ptr); } - _NODISCARD _Promise& promise() const noexcept { // strengthened - return *reinterpret_cast<_Promise*>(__builtin_coro_promise(_Ptr, 0, false)); + _NODISCARD _CoroPromise& promise() const noexcept { // strengthened + return *reinterpret_cast<_CoroPromise*>(__builtin_coro_promise(_Ptr, 0, false)); } private: @@ -184,10 +184,10 @@ _NODISCARD constexpr bool operator>=(const coroutine_handle<> _Left, const corou } #endif // ^^^ !_HAS_CXX20 ^^^ -template -struct hash> { +template +struct hash> { _NODISCARD _STATIC_CALL_OPERATOR size_t operator()( - const coroutine_handle<_Promise>& _Coro) _CONST_CALL_OPERATOR noexcept { + const coroutine_handle<_CoroPromise>& _Coro) _CONST_CALL_OPERATOR noexcept { return _Hash_representation(_Coro.address()); } }; diff --git a/stl/inc/generator b/stl/inc/generator index 004e56f236b..3a51c045562 100644 --- a/stl/inc/generator +++ b/stl/inc/generator @@ -270,10 +270,10 @@ private: return false; } - template - constexpr void await_suspend(coroutine_handle<_Promise> _Handle) noexcept { + template + constexpr void await_suspend(coroutine_handle<_CoroPromise> _Handle) noexcept { #ifdef __cpp_lib_is_pointer_interconvertible // TRANSITION, LLVM-48860 - _STL_INTERNAL_STATIC_ASSERT(is_pointer_interconvertible_base_of_v<_Base, _Promise>); + _STL_INTERNAL_STATIC_ASSERT(is_pointer_interconvertible_base_of_v<_Base, _CoroPromise>); #endif // ^^^ no workaround ^^^ _Base& _Current = _Handle.promise(); @@ -294,10 +294,10 @@ private: return false; } - template - _NODISCARD coroutine_handle<> await_suspend(coroutine_handle<_Promise> _Handle) noexcept { + template + _NODISCARD coroutine_handle<> await_suspend(coroutine_handle<_CoroPromise> _Handle) noexcept { #ifdef __cpp_lib_is_pointer_interconvertible // TRANSITION, LLVM-48860 - _STL_INTERNAL_STATIC_ASSERT(is_pointer_interconvertible_base_of_v<_Base, _Promise>); + _STL_INTERNAL_STATIC_ASSERT(is_pointer_interconvertible_base_of_v<_Base, _CoroPromise>); #endif // ^^^ no workaround ^^^ _Base& _Current = _Handle.promise(); @@ -328,10 +328,10 @@ private: return !_Gen._Coro; } - template - _NODISCARD coroutine_handle<_Base> await_suspend(coroutine_handle<_Promise> _Current) noexcept { + template + _NODISCARD coroutine_handle<_Base> await_suspend(coroutine_handle<_CoroPromise> _Current) noexcept { #ifdef __cpp_lib_is_pointer_interconvertible // TRANSITION, LLVM-48860 - _STL_INTERNAL_STATIC_ASSERT(is_pointer_interconvertible_base_of_v<_Base, _Promise>); + _STL_INTERNAL_STATIC_ASSERT(is_pointer_interconvertible_base_of_v<_Base, _CoroPromise>); #endif // ^^^ no workaround ^^^ auto _Target = coroutine_handle<_Base>::from_address(_Gen._Coro.address()); _Nested._Parent = coroutine_handle<_Base>::from_address(_Current.address()); From 99bd6cf264925960bb7746b5bedb81bf40059db4 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 12 Mar 2024 04:53:09 -0700 Subject: [PATCH 09/11] `throw;` => `_RERAISE;` --- stl/inc/generator | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/generator b/stl/inc/generator index 3a51c045562..c3dc71f6c47 100644 --- a/stl/inc/generator +++ b/stl/inc/generator @@ -258,7 +258,7 @@ public: if (_Info) { _Info->_Except = _STD current_exception(); } else { - throw; + _RERAISE; } } From 15f887d3dddee63e64433a7d26b649358c86936d Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 12 Mar 2024 05:28:06 -0700 Subject: [PATCH 10/11] Overhaul `generator`'s `static_assert`s. Add Standard citations. "generator's second argument" was bogus; use "generator's selected reference type" to describe italic-monospace-reference. Then say "an actual reference type" for clarity. "an iterator with \[...\] cannot model indirectly_readable" was confusing because it was describing a bad situation, but the other messages are describing how situations must be good. Rephrase this for clarity. --- stl/inc/generator | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/stl/inc/generator b/stl/inc/generator index c3dc71f6c47..9260b47810d 100644 --- a/stl/inc/generator +++ b/stl/inc/generator @@ -416,21 +416,23 @@ class generator : public _RANGES view_interface> { private: using _Value = _Gen_value_t<_Rty, _Vty>; static_assert(same_as, _Value> && is_object_v<_Value>, - "generator's value type must be a cv-unqualified object type"); + "generator's value type must be a cv-unqualified object type (N4971 [coro.generator.class]/1.2)"); using _Ref = _Gen_reference_t<_Rty, _Vty>; static_assert( is_reference_v<_Ref> || (is_object_v<_Ref> && same_as, _Ref> && copy_constructible<_Ref>), - "generator's second argument must be a reference type or a cv-unqualified " - "copy-constructible object type"); + "generator's selected reference type must be an actual reference type " + "or a cv-unqualified copy-constructible object type (N4971 [coro.generator.class]/1.3)"); using _RRef = conditional_t, remove_reference_t<_Ref>&&, _Ref>; static_assert(common_reference_with<_Ref&&, _Value&> && common_reference_with<_Ref&&, _RRef&&> && common_reference_with<_RRef&&, const _Value&>, - "an iterator with the selected value and reference types cannot model indirectly_readable"); + "generator's iterator type must model indirectly_readable, " + "but that's impossible with the selected value and reference types (N4971 [coro.generator.class]/1.4)"); - static_assert(_Has_real_pointers<_Alloc>, "generator allocators must use raw pointers"); + static_assert(_Has_real_pointers<_Alloc>, "generator allocators must use raw pointers " + "(N4971 [coro.generator.class]/1.1)"); friend _Gen_promise_base<_Gen_yield_t<_Ref>>; From 1b2b8bd0cbee56d23384f0bf1a14603f96bed7cc Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 12 Mar 2024 08:19:26 -0700 Subject: [PATCH 11/11] Mark `_Dealloc_fn` as `_NOEXCEPT_FNPTR`. Use `static` member functions instead of lambdas to make Clang happy. --- stl/inc/generator | 51 ++++++++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/stl/inc/generator b/stl/inc/generator index 9260b47810d..10777a59e3e 100644 --- a/stl/inc/generator +++ b/stl/inc/generator @@ -102,7 +102,33 @@ public: template <> class _Promise_allocator { // type-erased allocator private: - using _Dealloc_fn = void(__stdcall*)(void*, size_t); + using _Dealloc_fn = void(__stdcall*)(void*, size_t) _NOEXCEPT_FNPTR; + + template + static void __stdcall _Dealloc_stateless(void* const _Ptr, const size_t _Size) noexcept { + _Alloc _Al{}; + const size_t _Count = (_Size + sizeof(_Dealloc_fn) + sizeof(_Aligned_block) - 1) / sizeof(_Aligned_block); + _Al.deallocate(static_cast<_Aligned_block*>(_Ptr), _Count); + } + + template + static void __stdcall _Dealloc_stateful(void* const _Ptr, size_t _Size) noexcept { + constexpr size_t _Align = (_STD max)(alignof(_Alloc), sizeof(_Aligned_block)); + + _Size += sizeof(_Dealloc_fn); + const auto _Al_address = + (reinterpret_cast(_Ptr) + _Size + alignof(_Alloc) - 1) & ~(alignof(_Alloc) - 1); + auto& _Stored_al = *reinterpret_cast<_Alloc*>(_Al_address); + _Alloc _Al{_STD move(_Stored_al)}; + _Stored_al.~_Alloc(); + + const size_t _Count = (_Size + sizeof(_Al) + _Align - 1) / sizeof(_Aligned_block); + _Al.deallocate(static_cast<_Aligned_block*>(_Ptr), _Count); + } + + static void __stdcall _Dealloc_delete(void* const _Ptr, const size_t _Size) noexcept { + ::operator delete[](_Ptr, _Size + sizeof(_Dealloc_fn)); + } template static void* _Allocate(const _ProtoAlloc& _Proto, size_t _Size) { @@ -111,12 +137,7 @@ private: if constexpr (default_initializable<_Alloc> && allocator_traits<_Alloc>::is_always_equal::value) { // don't store stateless allocator - const _Dealloc_fn _Dealloc = [](void* const _Ptr, const size_t _Size) { - _Alloc _Al{}; - const size_t _Count = - (_Size + sizeof(_Dealloc_fn) + sizeof(_Aligned_block) - 1) / sizeof(_Aligned_block); - _Al.deallocate(static_cast<_Aligned_block*>(_Ptr), _Count); - }; + const _Dealloc_fn _Dealloc = _Dealloc_stateless<_Alloc>; const size_t _Count = (_Size + sizeof(_Dealloc_fn) + sizeof(_Aligned_block) - 1) / sizeof(_Aligned_block); void* const _Ptr = _Al.allocate(_Count); @@ -126,17 +147,7 @@ private: // store stateful allocator constexpr size_t _Align = (_STD max)(alignof(_Alloc), sizeof(_Aligned_block)); - const _Dealloc_fn _Dealloc = [](void* const _Ptr, size_t _Size) { - _Size += sizeof(_Dealloc_fn); - const auto _Al_address = - (reinterpret_cast(_Ptr) + _Size + alignof(_Alloc) - 1) & ~(alignof(_Alloc) - 1); - auto& _Stored_al = *reinterpret_cast<_Alloc*>(_Al_address); - _Alloc _Al{_STD move(_Stored_al)}; - _Stored_al.~_Alloc(); - - const size_t _Count = (_Size + sizeof(_Al) + _Align - 1) / sizeof(_Aligned_block); - _Al.deallocate(static_cast<_Aligned_block*>(_Ptr), _Count); - }; + const _Dealloc_fn _Dealloc = _Dealloc_stateful<_Alloc>; const size_t _Count = (_Size + sizeof(_Dealloc_fn) + sizeof(_Al) + _Align - 1) / sizeof(_Aligned_block); void* const _Ptr = _Al.allocate(_Count); @@ -152,9 +163,7 @@ private: public: static void* operator new(const size_t _Size) { // default: new/delete void* const _Ptr = ::operator new[](_Size + sizeof(_Dealloc_fn)); - const _Dealloc_fn _Dealloc = [](void* const _Ptr, const size_t _Size) { - ::operator delete[](_Ptr, _Size + sizeof(_Dealloc_fn)); - }; + const _Dealloc_fn _Dealloc = _Dealloc_delete; _CSTD memcpy(static_cast(_Ptr) + _Size, &_Dealloc, sizeof(_Dealloc_fn)); return _Ptr; }