diff --git a/stl/inc/functional b/stl/inc/functional index 1115020987..50b0449241 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..5aa044394f 100644 --- a/stl/inc/future +++ b/stl/inc/future @@ -466,16 +466,29 @@ 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...)>; - template - _Packaged_state(_Fty2&& _Fnarg) : _Fn(_STD forward<_Fty2>(_Fnarg)) {} + 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 + 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(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,28 +511,44 @@ 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; + _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...)>; - template - _Packaged_state(_Fty2&& _Fnarg) : _Fn(_STD forward<_Fty2>(_Fnarg)) {} + 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 + 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(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,28 +571,44 @@ 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; + _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; - template - _Packaged_state(_Fty2&& _Fnarg) : _Fn(_STD forward<_Fty2>(_Fnarg)) {} + 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 + 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(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,12 +633,15 @@ 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; + _Function_type _Fn; }; template @@ -1266,7 +1314,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, _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 +1326,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, _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 +1373,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);