<future>: Make packaged_task accept move-only functors
#4946
Conversation
frederick-vs-ja
force-pushed
the
packaged-move-only-task
branch
from
dce03b0
to
5195dd0
Compare
I think it worth creating vNext issue.
|
| _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())); |
There was a problem hiding this comment.
It's perhaps necessary to move per [futures.task.members]/26. However, the behavior of formerly valid code will change. Not sure whether this is acceptable.
There was a problem hiding this comment.
It seems possible to keep old behavior for existing types while enable correct behavior for newly allowed move-only functors - make the new branch of _Copy do const_cast and then call _Move. But that's somehow weird.
| @@ -122,7 +122,6 @@ void test_function() { | |||
|
|
|||
| void test_packaged_task() { | |||
| packaged_task<void(validator)>{}; | |||
| packaged_task<void(validator)>{nullptr}; | |||
There was a problem hiding this comment.
This line was wrong per [futures.task.members]/3 but I didn't realize that when it was added.
Or maybe |
| 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)."); |
There was a problem hiding this comment.
Now the change is exactly reflecting [futures.task.members]/3, but the Mandates doesn't seem very correct when remove_reference_t<_Fty2> is cv-qualified.
Should we use decay_t<_Fty2>& or _Remove_cvref_t<_Fty2>&? I've mailed to LWG Chair to submit an LWG issue to say decay_t in [futures.task.members]/3.
This comment was marked as off-topic.
This comment was marked as off-topic.
| 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 |
There was a problem hiding this comment.
Why is _SecretTag a template parameter? Could it be just a parameter of type _Secret_copyability_ignoring_tag ?
There was a problem hiding this comment.
I guess this is necessary for defending against some convertible-to-almost-all types. But perhaps we don't need it as of C++17 since the allocator_arg_t ctors are removed... although users can restore them.
Fixes #321.
The issue was considered ABI-breaking, but I it can be resolved in an ABI-preserving way like #2568. This PR adds internal constructors to
functionto accept non-copy-constructible functors and addstatic_assertto keep the copyability checking for standard constructors. Valid user codes won't be able to call these internal constructors.Also implements the previously missing Mandates in [futures.task.members]/3, and switches to use move construction in
resetper [futures.task.members]/26.Notes:
function.function<R(Args...)>is a program-defined specialization, this approach doesn't work. I don't think any user should specializestd::function, but this is allowed by the standard.