From dce03b0bd9dfc36890665000bfd79334f7668670 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Tue, 10 Sep 2024 12:34:10 +0800 Subject: [PATCH] Make `packaged_task` accept move-only functors --- stl/inc/functional | 31 +++++++++++++++++-- stl/inc/future | 12 +++---- .../Dev10_561430_list_and_tree_leaks/test.cpp | 11 +++++++ .../test.cpp | 15 +++++++++ .../test.cpp | 18 +++++++++++ .../test.compile.pass.cpp | 4 +++ 6 files changed, 83 insertions(+), 8 deletions(-) diff --git a/stl/inc/functional b/stl/inc/functional index d06f4e12917..7caef032d95 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 7d385549d6b..349ae820a29 100644 --- a/stl/inc/future +++ b/stl/inc/future @@ -470,12 +470,12 @@ public: using _Mydel = typename _Mybase::_Mydel; template - _Packaged_state(_Fty2&& _Fnarg) : _Fn(_STD forward<_Fty2>(_Fnarg)) {} + _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 @@ -514,12 +514,12 @@ public: using _Mydel = typename _Mybase::_Mydel; template - _Packaged_state(_Fty2&& _Fnarg) : _Fn(_STD forward<_Fty2>(_Fnarg)) {} + _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 @@ -558,12 +558,12 @@ public: using _Mydel = typename _Mybase::_Mydel; template - _Packaged_state(_Fty2&& _Fnarg) : _Fn(_STD forward<_Fty2>(_Fnarg)) {} + _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 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 5a2ea09e851..159da7d9d99 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 e4672fe3ed0..c5ba041829c 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_002488_promise_not_default_constructible_types/test.cpp b/tests/std/tests/GH_002488_promise_not_default_constructible_types/test.cpp index 51b5f20438e..161ca0a24ae 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 fd092034744..284e238b141 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);