Skip to content

Commit

Permalink
Make packaged_task accept move-only functors
Browse files Browse the repository at this point in the history
  • Loading branch information
@frederick-vs-ja
frederick-vs-ja committed Sep 10, 2024
1 parent ab20dbd commit 5195dd0
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 17 deletions.
31 changes: 29 additions & 2 deletions stl/inc/functional
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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:
Expand All @@ -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));
}

Expand All @@ -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
Expand Down
55 changes: 41 additions & 14 deletions stl/inc/future
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <class _Fty2>
_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 <class _Fty2, class _Alloc>
_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
Expand All @@ -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;
Expand All @@ -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 <class _Fty2>
_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 <class _Fty2, class _Alloc>
_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
Expand All @@ -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;
Expand All @@ -557,13 +571,17 @@ public:
using _Mybase = _Associated_state<int>;
using _Mydel = typename _Mybase::_Mydel;

explicit _Packaged_state(const function<void(_ArgTypes...)>& _Fnarg) : _Fn(_Fnarg) {}

explicit _Packaged_state(function<void(_ArgTypes...)>&& _Fnarg) noexcept : _Fn(_STD move(_Fnarg)) {}

template <class _Fty2>
_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 <class _Fty2, class _Alloc>
_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
Expand All @@ -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<void(_ArgTypes...)> _Fn;
Expand Down Expand Up @@ -1266,7 +1287,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, 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;

Expand All @@ -1275,7 +1299,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, 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 {
Expand Down Expand Up @@ -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);
}
Expand Down
11 changes: 11 additions & 0 deletions tests/std/tests/Dev10_561430_list_and_tree_leaks/test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
15 changes: 15 additions & 0 deletions tests/std/tests/Dev11_0235721_async_and_packaged_task/test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand Down
1 change: 0 additions & 1 deletion tests/std/tests/GH_000140_adl_proof_construction/test.compile.pass.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{}};

Expand Down
18 changes: 18 additions & 0 deletions tests/std/tests/GH_002488_promise_not_default_constructible_types/test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
4 changes: 4 additions & 0 deletions tests/std/tests/VSO_0000000_instantiate_iterators_misc/test.compile.pass.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 5195dd0

Please sign in to comment.