<future>: Make packaged_task accept move-only functors

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 _Fty>
 class function : public _Get_function_impl<_Fty>::type { // wrapper for callable objects
 private:
@@ -1086,6 +1095,14 @@ public:
 
     template <class _Fx, typename _Mybase::template _Enable_if_callable_t<_Fx, function> = 0>
     function(_Fx&& _Func) {
+        static_assert(is_copy_constructible_v<decay_t<_Fx>>,
+            "The target function object type must be copy constructible (N4988 [func.wrap.func.con]/10.1).");
+        this->_Reset(_STD forward<_Fx>(_Func));
+    }
+
+    template <class _SecretTag, class _Fx, typename _Mybase::template _Enable_if_callable_t<_Fx, function> = 0,
+        enable_if_t<is_same_v<_SecretTag, _Secret_copyability_ignoring_tag>, int> = 0>
+    explicit function(_SecretTag, _Fx&& _Func) { // used exclusively for packaged_task
         this->_Reset(_STD forward<_Fx>(_Func));
     }
 
@@ -1103,6 +1120,16 @@ public:
 
     template <class _Fx, class _Alloc, typename _Mybase::template _Enable_if_callable_t<_Fx, function> = 0>
     function(allocator_arg_t, const _Alloc& _Ax, _Fx&& _Func) {
+        static_assert(is_copy_constructible_v<decay_t<_Fx>>,
+            "The target function object type must be copy constructible (N4140 [func.wrap.func.con]/7).");
+        this->_Reset_alloc(_STD forward<_Fx>(_Func), _Ax);
+    }
+
+    template <class _SecretTag, class _Fx, class _Alloc,
+        typename _Mybase::template _Enable_if_callable_t<_Fx, function>           = 0,
+        enable_if_t<is_same_v<_SecretTag, _Secret_copyability_ignoring_tag>, 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

stl/inc/future

@@ -466,16 +466,29 @@ template <class _Ret, class... _ArgTypes>
 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 <class _Fty2>
-    _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 <class _Fty2, enable_if_t<!is_same_v<_Remove_cvref_t<_Fty2>, _Function_type>, int> = 0>
+    explicit _Packaged_state(_Fty2&& _Fnarg) : _Fn(_Secret_copyability_ignoring_tag{}, _STD forward<_Fty2>(_Fnarg)) {}
 
 #if _HAS_FUNCTION_ALLOCATOR_SUPPORT
-    template <class _Fty2, class _Alloc>
+    template <class _Alloc>
+    _Packaged_state(const _Function_type& _Fnarg, const _Alloc& _Al, _Mydel* _Dp)
+        : _Mybase(_Dp), _Fn(allocator_arg, _Al, _Fnarg) {}
+
+    template <class _Alloc>
+    _Packaged_state(_Function_type&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp)
+        : _Mybase(_Dp), _Fn(allocator_arg, _Al, _STD move(_Fnarg)) {}
+
+    template <class _Fty2, class _Alloc, enable_if_t<!is_same_v<_Remove_cvref_t<_Fty2>, _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 _Ret, class... _ArgTypes>
 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 <class _Fty2>
-    _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 <class _Fty2, enable_if_t<!is_same_v<_Remove_cvref_t<_Fty2>, _Function_type>, int> = 0>
+    explicit _Packaged_state(_Fty2&& _Fnarg) : _Fn(_Secret_copyability_ignoring_tag{}, _STD forward<_Fty2>(_Fnarg)) {}
 
 #if _HAS_FUNCTION_ALLOCATOR_SUPPORT
-    template <class _Fty2, class _Alloc>
+    template <class _Alloc>
+    _Packaged_state(const _Function_type& _Fnarg, const _Alloc& _Al, _Mydel* _Dp)
+        : _Mybase(_Dp), _Fn(allocator_arg, _Al, _Fnarg) {}
+
+    template <class _Alloc>
+    _Packaged_state(_Function_type&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp)
+        : _Mybase(_Dp), _Fn(allocator_arg, _Al, _STD move(_Fnarg)) {}
+
+    template <class _Fty2, class _Alloc, enable_if_t<!is_same_v<_Remove_cvref_t<_Fty2>, _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... _ArgTypes>
 class _Packaged_state<void(_ArgTypes...)>
     : public _Associated_state<int> { // class for managing associated asynchronous state for packaged_task
 public:
-    using _Mybase = _Associated_state<int>;
-    using _Mydel  = typename _Mybase::_Mydel;
+    using _Mybase        = _Associated_state<int>;
+    using _Mydel         = typename _Mybase::_Mydel;
+    using _Function_type = function<void(_ArgTypes...)>;
 
-    template <class _Fty2>
-    _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 <class _Fty2, enable_if_t<!is_same_v<_Remove_cvref_t<_Fty2>, _Function_type>, int> = 0>
+    explicit _Packaged_state(_Fty2&& _Fnarg) : _Fn(_Secret_copyability_ignoring_tag{}, _STD forward<_Fty2>(_Fnarg)) {}
 
 #if _HAS_FUNCTION_ALLOCATOR_SUPPORT
-    template <class _Fty2, class _Alloc>
+    template <class _Alloc>
+    _Packaged_state(const _Function_type& _Fnarg, const _Alloc& _Al, _Mydel* _Dp)
+        : _Mybase(_Dp), _Fn(allocator_arg, _Al, _Fnarg) {}
+
+    template <class _Alloc>
+    _Packaged_state(_Function_type&& _Fnarg, const _Alloc& _Al, _Mydel* _Dp)
+        : _Mybase(_Dp), _Fn(allocator_arg, _Al, _STD move(_Fnarg)) {}
+
+    template <class _Fty2, class _Alloc, enable_if_t<!is_same_v<_Remove_cvref_t<_Fty2>, _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<void(_ArgTypes...)> _Fn;
+    _Function_type _Fn;
 };
 
 template <class _Ty, class _Alloc>
@@ -1266,7 +1314,10 @@ public:
     packaged_task() = default;
 
     template <class _Fty2, enable_if_t<!is_same_v<_Remove_cvref_t<_Fty2>, 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 <class _Fty2, class _Alloc, enable_if_t<!is_same_v<_Remove_cvref_t<_Fty2>, 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);
     }

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: "<future>: packaged_task can't be constructed from a move-only lambda"
+    {
+        packaged_task<int()> pt(allocator_arg, Mallocator<int>(), [uptr = make_unique<int>(172)] { return *uptr; });
+
+        future<int> f = pt.get_future();
+
+        pt();
+
+        assert(f.get() == 172);
+    }
+
     {
         int n = 4096;
 

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: "<future>: packaged_task can't be constructed from a move-only lambda"
+    auto move_only_lambda = [uptr = make_unique<int>(42)] { return *uptr; };
+
     {
         packaged_task<int()> pt1([] { return 19937; });
         future<int> f = pt1.get_future();
@@ -90,6 +93,18 @@ void test_DevDiv_725337() {
         assert(f.get() == 19937);
     }
 
+    {
+        packaged_task<int()> pt1(move(move_only_lambda));
+        future<int> f = pt1.get_future();
+        packaged_task<int()> pt2(move(pt1));
+        packaged_task<int()> 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<int&()> pt1(ref_lambda);
         future<int&> f = pt1.get_future();

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<void(validator)>{};
-    packaged_task<void(validator)>{nullptr};
     packaged_task<void(validator)>{simple_identity{}};
     packaged_task<void(validator)>{simple_large_identity{}};
 

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: "<future>: 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<T()> pt(MoveOnlyFunctor{});
+        Future f = pt.get_future();
+        pt();
+
+        assert(f.get().x == 172);
+    }
 }
 
 int main() {

tests/std/tests/VSO_0000000_instantiate_iterators_misc/test.compile.pass.cpp

@@ -554,9 +554,13 @@ void future_test() {
     swap_test(pv);
 
     packaged_task<void()> pt([]() {});
+    // GH-321: "<future>: packaged_task can't be constructed from a move-only lambda"
+    packaged_task<void()> pt2([uptr = unique_ptr<int>{}]() { (void) uptr; });
 
 #if _HAS_FUNCTION_ALLOCATOR_SUPPORT
     packaged_task<void()> pta(allocator_arg, allocator<double>{}, []() {});
+    // GH-321: "<future>: packaged_task can't be constructed from a move-only lambda"
+    packaged_task<void()> pta2(allocator_arg, allocator<double>{}, [uptr = unique_ptr<int>{}]() { (void) uptr; });
 #endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT
 
     swap_test(pt);