From 5195dd0b20ac97ff6a586e9c870ea060e030ae52 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Tue, 10 Sep 2024 15:06:23 +0800 Subject: [PATCH 1/5] Make `packaged_task` accept move-only functors --- stl/inc/functional | 31 ++++++++++- stl/inc/future | 55 ++++++++++++++----- .../Dev10_561430_list_and_tree_leaks/test.cpp | 11 ++++ .../test.cpp | 15 +++++ .../test.compile.pass.cpp | 1 - .../test.cpp | 18 ++++++ .../test.compile.pass.cpp | 4 ++ 7 files changed, 118 insertions(+), 17 deletions(-) diff --git a/stl/inc/functional b/stl/inc/functional index d06f4e1291..7caef032d9 100644 --- a/stl/inc/functional +++ b/stl/inc/functional @@ -767,7 +767,10 @@ public: private: _Mybase* _Copy(void* _Where) const override { auto& _Myax = _Mypair._Get_first(); - if constexpr (_Is_large<_Func_impl>) { + if constexpr (!is_copy_constructible_v<_Callable>) { // used exclusively for packaged_task + (void) _Myax; + _CSTD abort(); // shouldn't be called, see GH-3888 + } else if constexpr (_Is_large<_Func_impl>) { _Myalty _Rebound(_Myax); _Alloc_construct_ptr<_Myalty> _Constructor{_Rebound}; _Constructor._Allocate(); @@ -854,7 +857,9 @@ public: private: _Mybase* _Copy(void* _Where) const override { - if constexpr (_Is_large<_Func_impl_no_alloc>) { + if constexpr (!is_copy_constructible_v<_Callable>) { // used exclusively for packaged_task + _CSTD abort(); // shouldn't be called, see GH-3888 + } else if constexpr (_Is_large<_Func_impl_no_alloc>) { return _STD _Global_new<_Func_impl_no_alloc>(_Callee); } else { return ::new (_Where) _Func_impl_no_alloc(_Callee); @@ -1070,6 +1075,10 @@ _NON_MEMBER_CALL(_GET_FUNCTION_IMPL_NOEXCEPT, X1, X2, X3) #undef _GET_FUNCTION_IMPL_NOEXCEPT #endif // defined(__cpp_noexcept_function_type) +struct _Secret_copyability_ignoring_tag { // used exclusively for packaged_task + explicit _Secret_copyability_ignoring_tag() = default; +}; + _EXPORT_STD template class function : public _Get_function_impl<_Fty>::type { // wrapper for callable objects private: @@ -1086,6 +1095,14 @@ public: template = 0> function(_Fx&& _Func) { + static_assert(is_copy_constructible_v>, + "The target function object type must be copy constructible (N4988 [func.wrap.func.con]/10.1)."); + this->_Reset(_STD forward<_Fx>(_Func)); + } + + template = 0, + enable_if_t, int> = 0> + explicit function(_SecretTag, _Fx&& _Func) { // used exclusively for packaged_task this->_Reset(_STD forward<_Fx>(_Func)); } @@ -1103,6 +1120,16 @@ public: template = 0> function(allocator_arg_t, const _Alloc& _Ax, _Fx&& _Func) { + static_assert(is_copy_constructible_v>, + "The target function object type must be copy constructible (N4140 [func.wrap.func.con]/7)."); + this->_Reset_alloc(_STD forward<_Fx>(_Func), _Ax); + } + + template = 0, + enable_if_t, int> = 0> + explicit function(_SecretTag, allocator_arg_t, const _Alloc& _Ax, _Fx&& _Func) { + // used exclusively for packaged_task this->_Reset_alloc(_STD forward<_Fx>(_Func), _Ax); } #endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT diff --git a/stl/inc/future b/stl/inc/future index 7d385549d6..1ac4b55df4 100644 --- a/stl/inc/future +++ b/stl/inc/future @@ -469,13 +469,17 @@ public: using _Mybase = _Associated_state<_Ret>; using _Mydel = typename _Mybase::_Mydel; + explicit _Packaged_state(const function<_Ret(_ArgTypes...)>& _Fnarg) : _Fn(_Fnarg) {} + + explicit _Packaged_state(function<_Ret(_ArgTypes...)>&& _Fnarg) noexcept : _Fn(_STD move(_Fnarg)) {} + template - _Packaged_state(_Fty2&& _Fnarg) : _Fn(_STD forward<_Fty2>(_Fnarg)) {} + explicit _Packaged_state(_Fty2&& _Fnarg) : _Fn(_Secret_copyability_ignoring_tag{}, _STD forward<_Fty2>(_Fnarg)) {} #if _HAS_FUNCTION_ALLOCATOR_SUPPORT template _Packaged_state(_Fty2&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) - : _Mybase(_Dp), _Fn(allocator_arg, _Al, _STD forward<_Fty2>(_Fnarg)) {} + : _Mybase(_Dp), _Fn(_Secret_copyability_ignoring_tag{}, allocator_arg, _Al, _STD forward<_Fty2>(_Fnarg)) {} #endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT void _Call_deferred(_ArgTypes... _Args) { // set deferred call @@ -498,9 +502,12 @@ public: _CATCH_END } - const auto& _Get_fn() const { + const auto& _Get_fn() const& { return _Fn; } + auto&& _Get_fn() && noexcept { + return _STD move(_Fn); + } private: function<_Ret(_ArgTypes...)> _Fn; @@ -513,13 +520,17 @@ public: using _Mybase = _Associated_state<_Ret*>; using _Mydel = typename _Mybase::_Mydel; + explicit _Packaged_state(const function<_Ret&(_ArgTypes...)>& _Fnarg) : _Fn(_Fnarg) {} + + explicit _Packaged_state(function<_Ret&(_ArgTypes...)>&& _Fnarg) noexcept : _Fn(_STD move(_Fnarg)) {} + template - _Packaged_state(_Fty2&& _Fnarg) : _Fn(_STD forward<_Fty2>(_Fnarg)) {} + explicit _Packaged_state(_Fty2&& _Fnarg) : _Fn(_Secret_copyability_ignoring_tag{}, _STD forward<_Fty2>(_Fnarg)) {} #if _HAS_FUNCTION_ALLOCATOR_SUPPORT template _Packaged_state(_Fty2&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) - : _Mybase(_Dp), _Fn(allocator_arg, _Al, _STD forward<_Fty2>(_Fnarg)) {} + : _Mybase(_Dp), _Fn(_Secret_copyability_ignoring_tag{}, allocator_arg, _Al, _STD forward<_Fty2>(_Fnarg)) {} #endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT void _Call_deferred(_ArgTypes... _Args) { // set deferred call @@ -542,9 +553,12 @@ public: _CATCH_END } - const auto& _Get_fn() const { + const auto& _Get_fn() const& { return _Fn; } + auto&& _Get_fn() && noexcept { + return _STD move(_Fn); + } private: function<_Ret&(_ArgTypes...)> _Fn; @@ -557,13 +571,17 @@ public: using _Mybase = _Associated_state; using _Mydel = typename _Mybase::_Mydel; + explicit _Packaged_state(const function& _Fnarg) : _Fn(_Fnarg) {} + + explicit _Packaged_state(function&& _Fnarg) noexcept : _Fn(_STD move(_Fnarg)) {} + template - _Packaged_state(_Fty2&& _Fnarg) : _Fn(_STD forward<_Fty2>(_Fnarg)) {} + explicit _Packaged_state(_Fty2&& _Fnarg) : _Fn(_Secret_copyability_ignoring_tag{}, _STD forward<_Fty2>(_Fnarg)) {} #if _HAS_FUNCTION_ALLOCATOR_SUPPORT template _Packaged_state(_Fty2&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) - : _Mybase(_Dp), _Fn(allocator_arg, _Al, _STD forward<_Fty2>(_Fnarg)) {} + : _Mybase(_Dp), _Fn(_Secret_copyability_ignoring_tag{}, allocator_arg, _Al, _STD forward<_Fty2>(_Fnarg)) {} #endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT void _Call_deferred(_ArgTypes... _Args) { // set deferred call @@ -588,9 +606,12 @@ public: _CATCH_END } - const auto& _Get_fn() const { + const auto& _Get_fn() const& { return _Fn; } + auto&& _Get_fn() && noexcept { + return _STD move(_Fn); + } private: function _Fn; @@ -1266,7 +1287,10 @@ public: packaged_task() = default; template , packaged_task>, int> = 0> - explicit packaged_task(_Fty2&& _Fnarg) : _MyPromise(new _MyStateType(_STD forward<_Fty2>(_Fnarg))) {} + explicit packaged_task(_Fty2&& _Fnarg) : _MyPromise(new _MyStateType(_STD forward<_Fty2>(_Fnarg))) { + static_assert(_Is_invocable_r<_Ret, decay_t<_Fty2>&, _ArgTypes...>::value, + "The function object must be callable with _ArgTypes... and return _Ret (N4988 [futures.task.members]/3)."); + } packaged_task(packaged_task&&) noexcept = default; @@ -1275,7 +1299,10 @@ public: #if _HAS_FUNCTION_ALLOCATOR_SUPPORT template , packaged_task>, int> = 0> packaged_task(allocator_arg_t, const _Alloc& _Al, _Fty2&& _Fnarg) - : _MyPromise(_STD _Make_packaged_state<_MyStateType>(_STD forward<_Fty2>(_Fnarg), _Al)) {} + : _MyPromise(_STD _Make_packaged_state<_MyStateType>(_STD forward<_Fty2>(_Fnarg), _Al)) { + static_assert(_Is_invocable_r<_Ret, decay_t<_Fty2>&, _ArgTypes...>::value, + "The function object must be callable with _ArgTypes... and return _Ret (N4140 [futures.task.members]/2)."); + } #endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT ~packaged_task() noexcept { @@ -1319,9 +1346,9 @@ public: } void reset() { // reset to newly constructed state - _MyStateManagerType& _State = _MyPromise._Get_state_for_set(); - _MyStateType* _MyState = static_cast<_MyStateType*>(_State._Ptr()); - _MyPromiseType _New_promise(new _MyStateType(_MyState->_Get_fn())); + _MyStateManagerType& _State_mgr = _MyPromise._Get_state_for_set(); + _MyStateType& _MyState = *static_cast<_MyStateType*>(_State_mgr._Ptr()); + _MyPromiseType _New_promise(new _MyStateType(_STD move(_MyState)._Get_fn())); _MyPromise._Get_state()._Abandon(); _MyPromise._Swap(_New_promise); } diff --git a/tests/std/tests/Dev10_561430_list_and_tree_leaks/test.cpp b/tests/std/tests/Dev10_561430_list_and_tree_leaks/test.cpp index 5a2ea09e85..159da7d9d9 100644 --- a/tests/std/tests/Dev10_561430_list_and_tree_leaks/test.cpp +++ b/tests/std/tests/Dev10_561430_list_and_tree_leaks/test.cpp @@ -164,6 +164,17 @@ int main() { assert(f.get() == 1234); } + // Also test GH-321: ": packaged_task can't be constructed from a move-only lambda" + { + packaged_task pt(allocator_arg, Mallocator(), [uptr = make_unique(172)] { return *uptr; }); + + future f = pt.get_future(); + + pt(); + + assert(f.get() == 172); + } + { int n = 4096; diff --git a/tests/std/tests/Dev11_0235721_async_and_packaged_task/test.cpp b/tests/std/tests/Dev11_0235721_async_and_packaged_task/test.cpp index e4672fe3ed..c5ba041829 100644 --- a/tests/std/tests/Dev11_0235721_async_and_packaged_task/test.cpp +++ b/tests/std/tests/Dev11_0235721_async_and_packaged_task/test.cpp @@ -78,6 +78,9 @@ void test_DevDiv_725337() { int i = 1729; auto ref_lambda = [&]() -> int& { return i; }; + // GH-321: ": packaged_task can't be constructed from a move-only lambda" + auto move_only_lambda = [uptr = make_unique(42)] { return *uptr; }; + { packaged_task pt1([] { return 19937; }); future f = pt1.get_future(); @@ -90,6 +93,18 @@ void test_DevDiv_725337() { assert(f.get() == 19937); } + { + packaged_task pt1(move(move_only_lambda)); + future f = pt1.get_future(); + packaged_task pt2(move(pt1)); + packaged_task pt3; + pt3 = move(pt2); + assert(f.wait_for(0s) == future_status::timeout); + pt3(); + assert(f.wait_for(0s) == future_status::ready); + assert(f.get() == 42); + } + { packaged_task pt1(ref_lambda); future f = pt1.get_future(); diff --git a/tests/std/tests/GH_000140_adl_proof_construction/test.compile.pass.cpp b/tests/std/tests/GH_000140_adl_proof_construction/test.compile.pass.cpp index f4c56c1748..0bc1d7ba29 100644 --- a/tests/std/tests/GH_000140_adl_proof_construction/test.compile.pass.cpp +++ b/tests/std/tests/GH_000140_adl_proof_construction/test.compile.pass.cpp @@ -122,7 +122,6 @@ void test_function() { void test_packaged_task() { packaged_task{}; - packaged_task{nullptr}; packaged_task{simple_identity{}}; packaged_task{simple_large_identity{}}; diff --git a/tests/std/tests/GH_002488_promise_not_default_constructible_types/test.cpp b/tests/std/tests/GH_002488_promise_not_default_constructible_types/test.cpp index 51b5f20438..161ca0a24a 100644 --- a/tests/std/tests/GH_002488_promise_not_default_constructible_types/test.cpp +++ b/tests/std/tests/GH_002488_promise_not_default_constructible_types/test.cpp @@ -176,6 +176,24 @@ void run_tests() { assert(f.get().x == 7); } + + // Also test GH-321: ": packaged_task can't be constructed from a move-only lambda" + { + struct MoveOnlyFunctor { + MoveOnlyFunctor() = default; + MoveOnlyFunctor(MoveOnlyFunctor&&) = default; + MoveOnlyFunctor& operator=(MoveOnlyFunctor&&) = default; + + T operator()() const { + return T{172}; + } + }; + std::packaged_task pt(MoveOnlyFunctor{}); + Future f = pt.get_future(); + pt(); + + assert(f.get().x == 172); + } } int main() { diff --git a/tests/std/tests/VSO_0000000_instantiate_iterators_misc/test.compile.pass.cpp b/tests/std/tests/VSO_0000000_instantiate_iterators_misc/test.compile.pass.cpp index fd09203474..284e238b14 100644 --- a/tests/std/tests/VSO_0000000_instantiate_iterators_misc/test.compile.pass.cpp +++ b/tests/std/tests/VSO_0000000_instantiate_iterators_misc/test.compile.pass.cpp @@ -554,9 +554,13 @@ void future_test() { swap_test(pv); packaged_task pt([]() {}); + // GH-321: ": packaged_task can't be constructed from a move-only lambda" + packaged_task pt2([uptr = unique_ptr{}]() { (void) uptr; }); #if _HAS_FUNCTION_ALLOCATOR_SUPPORT packaged_task pta(allocator_arg, allocator{}, []() {}); + // GH-321: ": packaged_task can't be constructed from a move-only lambda" + packaged_task pta2(allocator_arg, allocator{}, [uptr = unique_ptr{}]() { (void) uptr; }); #endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT swap_test(pt); From a57cf70b542d01e8c71444cf3ca7a6c352a0bd19 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Tue, 10 Sep 2024 15:51:06 +0800 Subject: [PATCH 2/5] Constrain constructor overloads to keep behavior as possible --- stl/inc/future | 69 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 21 deletions(-) diff --git a/stl/inc/future b/stl/inc/future index 1ac4b55df4..002fc578be 100644 --- a/stl/inc/future +++ b/stl/inc/future @@ -466,18 +466,27 @@ template class _Packaged_state<_Ret(_ArgTypes...)> : public _Associated_state<_Ret> { // class for managing associated asynchronous state for packaged_task public: - using _Mybase = _Associated_state<_Ret>; - using _Mydel = typename _Mybase::_Mydel; + using _Mybase = _Associated_state<_Ret>; + using _Mydel = typename _Mybase::_Mydel; + using _Function_type = function<_Ret(_ArgTypes...)>; - explicit _Packaged_state(const function<_Ret(_ArgTypes...)>& _Fnarg) : _Fn(_Fnarg) {} + explicit _Packaged_state(const _Function_type& _Fnarg) : _Fn(_Fnarg) {} - explicit _Packaged_state(function<_Ret(_ArgTypes...)>&& _Fnarg) noexcept : _Fn(_STD move(_Fnarg)) {} + explicit _Packaged_state(_Function_type&& _Fnarg) noexcept : _Fn(_STD move(_Fnarg)) {} - template + template , _Function_type>, int> = 0> explicit _Packaged_state(_Fty2&& _Fnarg) : _Fn(_Secret_copyability_ignoring_tag{}, _STD forward<_Fty2>(_Fnarg)) {} #if _HAS_FUNCTION_ALLOCATOR_SUPPORT - template + template + _Packaged_state(const _Function_type& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) + : _Mybase(_Dp), _Fn(allocator_arg, _Al, _Fnarg) {} + + template + _Packaged_state(_Function_type&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) + : _Mybase(_Dp), _Fn(allocator_arg, _Al, _STD move(_Fnarg)) {} + + template , _Function_type>, int> = 0> _Packaged_state(_Fty2&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) : _Mybase(_Dp), _Fn(_Secret_copyability_ignoring_tag{}, allocator_arg, _Al, _STD forward<_Fty2>(_Fnarg)) {} #endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT @@ -510,25 +519,34 @@ public: } private: - function<_Ret(_ArgTypes...)> _Fn; + _Function_type _Fn; }; template class _Packaged_state<_Ret&(_ArgTypes...)> : public _Associated_state<_Ret*> { // class for managing associated asynchronous state for packaged_task public: - using _Mybase = _Associated_state<_Ret*>; - using _Mydel = typename _Mybase::_Mydel; + using _Mybase = _Associated_state<_Ret*>; + using _Mydel = typename _Mybase::_Mydel; + using _Function_type = function<_Ret&(_ArgTypes...)>; - explicit _Packaged_state(const function<_Ret&(_ArgTypes...)>& _Fnarg) : _Fn(_Fnarg) {} + explicit _Packaged_state(const _Function_type& _Fnarg) : _Fn(_Fnarg) {} - explicit _Packaged_state(function<_Ret&(_ArgTypes...)>&& _Fnarg) noexcept : _Fn(_STD move(_Fnarg)) {} + explicit _Packaged_state(_Function_type&& _Fnarg) noexcept : _Fn(_STD move(_Fnarg)) {} - template + template , _Function_type>, int> = 0> explicit _Packaged_state(_Fty2&& _Fnarg) : _Fn(_Secret_copyability_ignoring_tag{}, _STD forward<_Fty2>(_Fnarg)) {} #if _HAS_FUNCTION_ALLOCATOR_SUPPORT - template + template + _Packaged_state(const _Function_type& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) + : _Mybase(_Dp), _Fn(allocator_arg, _Al, _Fnarg) {} + + template + _Packaged_state(_Function_type&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) + : _Mybase(_Dp), _Fn(allocator_arg, _Al, _STD move(_Fnarg)) {} + + template , _Function_type>, int> = 0> _Packaged_state(_Fty2&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) : _Mybase(_Dp), _Fn(_Secret_copyability_ignoring_tag{}, allocator_arg, _Al, _STD forward<_Fty2>(_Fnarg)) {} #endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT @@ -561,25 +579,34 @@ public: } private: - function<_Ret&(_ArgTypes...)> _Fn; + _Function_type _Fn; }; template class _Packaged_state : public _Associated_state { // class for managing associated asynchronous state for packaged_task public: - using _Mybase = _Associated_state; - using _Mydel = typename _Mybase::_Mydel; + using _Mybase = _Associated_state; + using _Mydel = typename _Mybase::_Mydel; + using _Function_type = function; - explicit _Packaged_state(const function& _Fnarg) : _Fn(_Fnarg) {} + explicit _Packaged_state(const _Function_type& _Fnarg) : _Fn(_Fnarg) {} - explicit _Packaged_state(function&& _Fnarg) noexcept : _Fn(_STD move(_Fnarg)) {} + explicit _Packaged_state(_Function_type&& _Fnarg) noexcept : _Fn(_STD move(_Fnarg)) {} - template + template , _Function_type>, int> = 0> explicit _Packaged_state(_Fty2&& _Fnarg) : _Fn(_Secret_copyability_ignoring_tag{}, _STD forward<_Fty2>(_Fnarg)) {} #if _HAS_FUNCTION_ALLOCATOR_SUPPORT - template + template + _Packaged_state(const _Function_type& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) + : _Mybase(_Dp), _Fn(allocator_arg, _Al, _Fnarg) {} + + template + _Packaged_state(_Function_type&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) + : _Mybase(_Dp), _Fn(allocator_arg, _Al, _STD move(_Fnarg)) {} + + template , _Function_type>, int> = 0> _Packaged_state(_Fty2&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) : _Mybase(_Dp), _Fn(_Secret_copyability_ignoring_tag{}, allocator_arg, _Al, _STD forward<_Fty2>(_Fnarg)) {} #endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT @@ -614,7 +641,7 @@ public: } private: - function _Fn; + _Function_type _Fn; }; template From edc3b575fa718f79e422e5d18d1a13842b53fc7c Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Tue, 10 Sep 2024 21:23:07 +0800 Subject: [PATCH 3/5] It's incorrect to use `decay_t` for [futures.task.members]/3 --- stl/inc/future | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/future b/stl/inc/future index 002fc578be..5aa044394f 100644 --- a/stl/inc/future +++ b/stl/inc/future @@ -1315,7 +1315,7 @@ public: template , packaged_task>, int> = 0> explicit packaged_task(_Fty2&& _Fnarg) : _MyPromise(new _MyStateType(_STD forward<_Fty2>(_Fnarg))) { - static_assert(_Is_invocable_r<_Ret, decay_t<_Fty2>&, _ArgTypes...>::value, + static_assert(_Is_invocable_r<_Ret, _Fty2&, _ArgTypes...>::value, "The function object must be callable with _ArgTypes... and return _Ret (N4988 [futures.task.members]/3)."); } @@ -1327,7 +1327,7 @@ public: template , packaged_task>, int> = 0> packaged_task(allocator_arg_t, const _Alloc& _Al, _Fty2&& _Fnarg) : _MyPromise(_STD _Make_packaged_state<_MyStateType>(_STD forward<_Fty2>(_Fnarg), _Al)) { - static_assert(_Is_invocable_r<_Ret, decay_t<_Fty2>&, _ArgTypes...>::value, + static_assert(_Is_invocable_r<_Ret, _Fty2&, _ArgTypes...>::value, "The function object must be callable with _ArgTypes... and return _Ret (N4140 [futures.task.members]/2)."); } #endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT From fb94c5a6e70fc854f0eef9868b96bc97ec9064b9 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 20 Sep 2024 18:05:07 +0800 Subject: [PATCH 4/5] Speculatively implement LWG-4154, reorder constraints and merge `_Packaged_state` specializations --- stl/inc/functional | 9 ++- stl/inc/future | 180 +++++++++------------------------------------ 2 files changed, 41 insertions(+), 148 deletions(-) diff --git a/stl/inc/functional b/stl/inc/functional index 50b0449241..c1a7d6fa54 100644 --- a/stl/inc/functional +++ b/stl/inc/functional @@ -1100,8 +1100,9 @@ public: this->_Reset(_STD forward<_Fx>(_Func)); } - template = 0, - enable_if_t, int> = 0> + template , int> = 0, + typename _Mybase::template _Enable_if_callable_t<_Fx, function> = 0> explicit function(_SecretTag, _Fx&& _Func) { // used exclusively for packaged_task this->_Reset(_STD forward<_Fx>(_Func)); } @@ -1126,8 +1127,8 @@ public: } template = 0, - enable_if_t, int> = 0> + enable_if_t, int> = 0, + typename _Mybase::template _Enable_if_callable_t<_Fx, function> = 0> explicit function(_SecretTag, allocator_arg_t, const _Alloc& _Ax, _Fx&& _Func) { // used exclusively for packaged_task this->_Reset_alloc(_STD forward<_Fx>(_Func), _Ax); diff --git a/stl/inc/future b/stl/inc/future index 5aa044394f..49c7b44338 100644 --- a/stl/inc/future +++ b/stl/inc/future @@ -459,136 +459,31 @@ void _State_deleter<_Ty, _Derived, _Alloc>::_Delete(_Associated_state<_Ty>* _Sta _STD _Delete_plain_internal(_Del_alloc, this); } -template -class _Packaged_state; - -template -class _Packaged_state<_Ret(_ArgTypes...)> - : public _Associated_state<_Ret> { // class for managing associated asynchronous state for packaged_task -public: - using _Mybase = _Associated_state<_Ret>; - using _Mydel = typename _Mybase::_Mydel; - using _Function_type = function<_Ret(_ArgTypes...)>; - - explicit _Packaged_state(const _Function_type& _Fnarg) : _Fn(_Fnarg) {} - - explicit _Packaged_state(_Function_type&& _Fnarg) noexcept : _Fn(_STD move(_Fnarg)) {} - - template , _Function_type>, int> = 0> - explicit _Packaged_state(_Fty2&& _Fnarg) : _Fn(_Secret_copyability_ignoring_tag{}, _STD forward<_Fty2>(_Fnarg)) {} - -#if _HAS_FUNCTION_ALLOCATOR_SUPPORT - template - _Packaged_state(const _Function_type& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) - : _Mybase(_Dp), _Fn(allocator_arg, _Al, _Fnarg) {} - - template - _Packaged_state(_Function_type&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) - : _Mybase(_Dp), _Fn(allocator_arg, _Al, _STD move(_Fnarg)) {} - - template , _Function_type>, int> = 0> - _Packaged_state(_Fty2&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) - : _Mybase(_Dp), _Fn(_Secret_copyability_ignoring_tag{}, allocator_arg, _Al, _STD forward<_Fty2>(_Fnarg)) {} -#endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT - - void _Call_deferred(_ArgTypes... _Args) { // set deferred call - _TRY_BEGIN - // call function object and catch exceptions - this->_Set_value(_Fn(_STD forward<_ArgTypes>(_Args)...), true); - _CATCH_ALL - // function object threw exception; record result - this->_Set_exception(_STD current_exception(), true); - _CATCH_END - } - - void _Call_immediate(_ArgTypes... _Args) { // call function object - _TRY_BEGIN - // call function object and catch exceptions - this->_Set_value(_Fn(_STD forward<_ArgTypes>(_Args)...), false); - _CATCH_ALL - // function object threw exception; record result - this->_Set_exception(_STD current_exception(), false); - _CATCH_END - } - - const auto& _Get_fn() const& { - return _Fn; - } - auto&& _Get_fn() && noexcept { - return _STD move(_Fn); - } - -private: - _Function_type _Fn; +template +struct _P_arg_type { // type for functions returning T + using type = _Fret; }; -template -class _Packaged_state<_Ret&(_ArgTypes...)> - : public _Associated_state<_Ret*> { // class for managing associated asynchronous state for packaged_task -public: - using _Mybase = _Associated_state<_Ret*>; - using _Mydel = typename _Mybase::_Mydel; - using _Function_type = function<_Ret&(_ArgTypes...)>; - - explicit _Packaged_state(const _Function_type& _Fnarg) : _Fn(_Fnarg) {} - - explicit _Packaged_state(_Function_type&& _Fnarg) noexcept : _Fn(_STD move(_Fnarg)) {} - - template , _Function_type>, int> = 0> - explicit _Packaged_state(_Fty2&& _Fnarg) : _Fn(_Secret_copyability_ignoring_tag{}, _STD forward<_Fty2>(_Fnarg)) {} - -#if _HAS_FUNCTION_ALLOCATOR_SUPPORT - template - _Packaged_state(const _Function_type& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) - : _Mybase(_Dp), _Fn(allocator_arg, _Al, _Fnarg) {} - - template - _Packaged_state(_Function_type&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) - : _Mybase(_Dp), _Fn(allocator_arg, _Al, _STD move(_Fnarg)) {} - - template , _Function_type>, int> = 0> - _Packaged_state(_Fty2&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) - : _Mybase(_Dp), _Fn(_Secret_copyability_ignoring_tag{}, allocator_arg, _Al, _STD forward<_Fty2>(_Fnarg)) {} -#endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT - - void _Call_deferred(_ArgTypes... _Args) { // set deferred call - _TRY_BEGIN - // call function object and catch exceptions - this->_Set_value(_STD addressof(_Fn(_STD forward<_ArgTypes>(_Args)...)), true); - _CATCH_ALL - // function object threw exception; record result - this->_Set_exception(_STD current_exception(), true); - _CATCH_END - } - - void _Call_immediate(_ArgTypes... _Args) { // call function object - _TRY_BEGIN - // call function object and catch exceptions - this->_Set_value(_STD addressof(_Fn(_STD forward<_ArgTypes>(_Args)...)), false); - _CATCH_ALL - // function object threw exception; record result - this->_Set_exception(_STD current_exception(), false); - _CATCH_END - } - - const auto& _Get_fn() const& { - return _Fn; - } - auto&& _Get_fn() && noexcept { - return _STD move(_Fn); - } +template +struct _P_arg_type<_Fret&> { // type for functions returning reference to T + using type = _Fret*; +}; -private: - _Function_type _Fn; +template <> +struct _P_arg_type { // type for functions returning void + using type = int; }; -template -class _Packaged_state - : public _Associated_state { // class for managing associated asynchronous state for packaged_task +template +class _Packaged_state; + +// class for managing associated asynchronous state for packaged_task +template +class _Packaged_state<_Ret(_ArgTypes...)> : public _Associated_state::type> { public: - using _Mybase = _Associated_state; + using _Mybase = _Associated_state::type>; using _Mydel = typename _Mybase::_Mydel; - using _Function_type = function; + using _Function_type = function<_Ret(_ArgTypes...)>; // TRANSITION, ABI, should not use std::function explicit _Packaged_state(const _Function_type& _Fnarg) : _Fn(_Fnarg) {} @@ -614,8 +509,14 @@ public: void _Call_deferred(_ArgTypes... _Args) { // set deferred call _TRY_BEGIN // call function object and catch exceptions - _Fn(_STD forward<_ArgTypes>(_Args)...); - this->_Set_value(1, true); + if constexpr (is_same_v<_Ret, void>) { + _Fn(_STD forward<_ArgTypes>(_Args)...); + this->_Set_value(1, true); + } else if constexpr (is_lvalue_reference_v<_Ret>) { + this->_Set_value(_STD addressof(_Fn(_STD forward<_ArgTypes>(_Args)...)), true); + } else { + this->_Set_value(_Fn(_STD forward<_ArgTypes>(_Args)...), true); + } _CATCH_ALL // function object threw exception; record result this->_Set_exception(_STD current_exception(), true); @@ -625,8 +526,14 @@ public: void _Call_immediate(_ArgTypes... _Args) { // call function object _TRY_BEGIN // call function object and catch exceptions - _Fn(_STD forward<_ArgTypes>(_Args)...); - this->_Set_value(1, false); + if constexpr (is_same_v<_Ret, void>) { + _Fn(_STD forward<_ArgTypes>(_Args)...); + this->_Set_value(1, false); + } else if constexpr (is_lvalue_reference_v<_Ret>) { + this->_Set_value(_STD addressof(_Fn(_STD forward<_ArgTypes>(_Args)...)), false); + } else { + this->_Set_value(_Fn(_STD forward<_ArgTypes>(_Args)...), false); + } _CATCH_ALL // function object threw exception; record result this->_Set_exception(_STD current_exception(), false); @@ -1283,21 +1190,6 @@ void swap(promise<_Ty>& _Left, promise<_Ty>& _Right) noexcept { _Left.swap(_Right); } -template -struct _P_arg_type { // type for functions returning T - using type = _Fret; -}; - -template -struct _P_arg_type<_Fret&> { // type for functions returning reference to T - using type = _Fret*; -}; - -template <> -struct _P_arg_type { // type for functions returning void - using type = int; -}; - _EXPORT_STD template class packaged_task; // not defined @@ -1315,7 +1207,7 @@ public: template , packaged_task>, int> = 0> explicit packaged_task(_Fty2&& _Fnarg) : _MyPromise(new _MyStateType(_STD forward<_Fty2>(_Fnarg))) { - static_assert(_Is_invocable_r<_Ret, _Fty2&, _ArgTypes...>::value, + static_assert(_Is_invocable_r<_Ret, decay_t<_Fty2>&, _ArgTypes...>::value, // per LWG-4154 "The function object must be callable with _ArgTypes... and return _Ret (N4988 [futures.task.members]/3)."); } @@ -1327,7 +1219,7 @@ public: template , packaged_task>, int> = 0> packaged_task(allocator_arg_t, const _Alloc& _Al, _Fty2&& _Fnarg) : _MyPromise(_STD _Make_packaged_state<_MyStateType>(_STD forward<_Fty2>(_Fnarg), _Al)) { - static_assert(_Is_invocable_r<_Ret, _Fty2&, _ArgTypes...>::value, + static_assert(_Is_invocable_r<_Ret, decay_t<_Fty2>&, _ArgTypes...>::value, // per LWG-4154 "The function object must be callable with _ArgTypes... and return _Ret (N4140 [futures.task.members]/2)."); } #endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT From fcadb1d5d2fcc6e5b5e1aaa105360d9c87d47aa3 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 11 Oct 2024 06:44:15 -0700 Subject: [PATCH 5/5] Perma-workaround VSO-2279389. VSO-2279389 "/clr C++20 can't handle struct MoveOnlyFunctor defined in a function template, emitting fatal error C1193: an error expected in yyaction.cpp(2899) not reached" --- .../test.cpp | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/tests/std/tests/GH_002488_promise_not_default_constructible_types/test.cpp b/tests/std/tests/GH_002488_promise_not_default_constructible_types/test.cpp index 161ca0a24a..1ad736f805 100644 --- a/tests/std/tests/GH_002488_promise_not_default_constructible_types/test.cpp +++ b/tests/std/tests/GH_002488_promise_not_default_constructible_types/test.cpp @@ -78,6 +78,17 @@ void assert_throws_future_error(F f, std::error_code expected_code) { assert(false); } +template +struct MoveOnlyFunctor { + MoveOnlyFunctor() = default; + MoveOnlyFunctor(MoveOnlyFunctor&&) = default; + MoveOnlyFunctor& operator=(MoveOnlyFunctor&&) = default; + + T operator()() const { + return T{172}; + } +}; + template void run_tests() { using Promise = std::promise; @@ -179,16 +190,7 @@ void run_tests() { // Also test GH-321: ": packaged_task can't be constructed from a move-only lambda" { - struct MoveOnlyFunctor { - MoveOnlyFunctor() = default; - MoveOnlyFunctor(MoveOnlyFunctor&&) = default; - MoveOnlyFunctor& operator=(MoveOnlyFunctor&&) = default; - - T operator()() const { - return T{172}; - } - }; - std::packaged_task pt(MoveOnlyFunctor{}); + std::packaged_task pt(MoveOnlyFunctor{}); Future f = pt.get_future(); pt();