Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

<future>: Make packaged_task accept move-only functors #4946

Merged
merged 7 commits into from
Oct 12, 2024
Merged

<future>: Make packaged_task accept move-only functors #4946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5195dd0
Make `packaged_task` accept move-only functors
frederick-vs-ja Sep 10, 2024
a57cf70
Constrain constructor overloads to keep behavior as possible
frederick-vs-ja Sep 10, 2024
edc3b57
It's incorrect to use `decay_t` for [futures.task.members]/3
frederick-vs-ja Sep 10, 2024
5e3ea4e
Merge branch 'microsoft:main' into packaged-move-only-task
frederick-vs-ja Sep 12, 2024
0ac6820
Merge branch 'main' into packaged-move-only-task
frederick-vs-ja Sep 20, 2024
fb94c5a
Speculatively implement LWG-4154, reorder constraints
frederick-vs-ja Sep 20, 2024
fcadb1d
Perma-workaround VSO-2279389.
StephanTLavavej Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions stl/inc/functional
View file
Open in desktop
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,15 @@ 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,
enable_if_t<is_same_v<_SecretTag, _Secret_copyability_ignoring_tag>, 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));
}

Expand All @@ -1103,6 +1121,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,
enable_if_t<is_same_v<_SecretTag, _Secret_copyability_ignoring_tag>, 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
Expand Down
182 changes: 64 additions & 118 deletions stl/inc/future
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,118 +459,64 @@ void _State_deleter<_Ty, _Derived, _Alloc>::_Delete(_Associated_state<_Ty>* _Sta
_STD _Delete_plain_internal(_Del_alloc, this);
}

template <class>
class _Packaged_state;

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;

template <class _Fty2>
_Packaged_state(_Fty2&& _Fnarg) : _Fn(_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)) {}
#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 <class _Fret>
struct _P_arg_type { // type for functions returning T
using type = _Fret;
};

const auto& _Get_fn() const {
return _Fn;
}
template <class _Fret>
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<void> { // type for functions returning void
using type = int;
};

template <class>
class _Packaged_state;

// class for managing associated asynchronous state for packaged_task
template <class _Ret, class... _ArgTypes>
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<typename _P_arg_type<_Ret>::type> {
public:
using _Mybase = _Associated_state<_Ret*>;
using _Mydel = typename _Mybase::_Mydel;

template <class _Fty2>
_Packaged_state(_Fty2&& _Fnarg) : _Fn(_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)) {}
#endif // _HAS_FUNCTION_ALLOCATOR_SUPPORT
using _Mybase = _Associated_state<typename _P_arg_type<_Ret>::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 <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)) {}

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;
#if _HAS_FUNCTION_ALLOCATOR_SUPPORT
template <class _Alloc>
_Packaged_state(const _Function_type& _Fnarg, const _Alloc& _Al, _Mydel* _Dp)
: _Mybase(_Dp), _Fn(allocator_arg, _Al, _Fnarg) {}

template <class _Fty2>
_Packaged_state(_Fty2&& _Fnarg) : _Fn(_STD forward<_Fty2>(_Fnarg)) {}
template <class _Alloc>
_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 <class _Fty2, class _Alloc>
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
_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);
Expand All @@ -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<void(_ArgTypes...)> _Fn;
_Function_type _Fn;
};

template <class _Ty, class _Alloc>
Expand Down Expand Up @@ -1235,21 +1190,6 @@ void swap(promise<_Ty>& _Left, promise<_Ty>& _Right) noexcept {
_Left.swap(_Right);
}

template <class _Fret>
struct _P_arg_type { // type for functions returning T
using type = _Fret;
};

template <class _Fret>
struct _P_arg_type<_Fret&> { // type for functions returning reference to T
using type = _Fret*;
};

template <>
struct _P_arg_type<void> { // type for functions returning void
using type = int;
};

_EXPORT_STD template <class>
class packaged_task; // not defined

Expand All @@ -1266,7 +1206,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, // 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;

Expand All @@ -1275,7 +1218,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, // 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 {
Expand Down Expand Up @@ -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()));
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
_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
Open in desktop
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
Open in desktop
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
Open in desktop
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};
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
packaged_task<void(validator)>{simple_identity{}};
packaged_task<void(validator)>{simple_large_identity{}};

Expand Down
Loading