diff --git a/stl/inc/functional b/stl/inc/functional index 1115020987..c1a7d6fa54 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,15 @@ 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 , 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)); } @@ -1103,6 +1121,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 , 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); } #endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT diff --git a/stl/inc/future b/stl/inc/future index 7d385549d6..49c7b44338 100644 --- a/stl/inc/future +++ b/stl/inc/future @@ -459,118 +459,64 @@ 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; - - template - _Packaged_state(_Fty2&& _Fnarg) : _Fn(_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)) {} -#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 - } +template +struct _P_arg_type { // type for functions returning T + using type = _Fret; +}; - const auto& _Get_fn() const { - return _Fn; - } +template +struct _P_arg_type<_Fret&> { // type for functions returning reference to T + using type = _Fret*; +}; -private: - function<_Ret(_ArgTypes...)> _Fn; +template <> +struct _P_arg_type { // type for functions returning void + using type = int; }; +template +class _Packaged_state; + +// class for managing associated asynchronous state for packaged_task template -class _Packaged_state<_Ret&(_ArgTypes...)> - : public _Associated_state<_Ret*> { // class for managing associated asynchronous state for packaged_task +class _Packaged_state<_Ret(_ArgTypes...)> : public _Associated_state::type> { public: - using _Mybase = _Associated_state<_Ret*>; - using _Mydel = typename _Mybase::_Mydel; - - template - _Packaged_state(_Fty2&& _Fnarg) : _Fn(_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)) {} -#endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT + using _Mybase = _Associated_state::type>; + using _Mydel = typename _Mybase::_Mydel; + using _Function_type = function<_Ret(_ArgTypes...)>; // TRANSITION, ABI, should not use std::function - 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 - } + explicit _Packaged_state(const _Function_type& _Fnarg) : _Fn(_Fnarg) {} - const auto& _Get_fn() const { - return _Fn; - } + explicit _Packaged_state(_Function_type&& _Fnarg) noexcept : _Fn(_STD move(_Fnarg)) {} -private: - function<_Ret&(_ArgTypes...)> _Fn; -}; + template , _Function_type>, int> = 0> + explicit _Packaged_state(_Fty2&& _Fnarg) : _Fn(_Secret_copyability_ignoring_tag{}, _STD forward<_Fty2>(_Fnarg)) {} -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; +#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(_Fty2&& _Fnarg) : _Fn(_STD forward<_Fty2>(_Fnarg)) {} + template + _Packaged_state(_Function_type&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp) + : _Mybase(_Dp), _Fn(allocator_arg, _Al, _STD move(_Fnarg)) {} -#if _HAS_FUNCTION_ALLOCATOR_SUPPORT - template + 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 _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); @@ -580,20 +526,29 @@ 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); _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 @@ -1235,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 @@ -1266,7 +1206,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, // per LWG-4154 + "The function object must be callable with _ArgTypes... and return _Ret (N4988 [futures.task.members]/3)."); + } packaged_task(packaged_task&&) noexcept = default; @@ -1275,7 +1218,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, // per LWG-4154 + "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 +1265,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..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; @@ -176,6 +187,15 @@ void run_tests() { assert(f.get().x == 7); } + + // Also test GH-321: ": packaged_task can't be constructed from a move-only lambda" + { + 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);