From e708c1a9d7304633fa204a1baf3abfd419a0cccd Mon Sep 17 00:00:00 2001 From: AntonBikineev Date: Wed, 6 Jan 2016 04:02:31 +0300 Subject: [PATCH 1/5] Fixing .then to work with movable-only types --- Release/include/pplx/pplxtasks.h | 82 ++++++++++++++----- .../pplx/pplx_test/pplxtask_tests.cpp | 15 ++++ 2 files changed, 76 insertions(+), 21 deletions(-) diff --git a/Release/include/pplx/pplxtasks.h b/Release/include/pplx/pplxtasks.h index e82a14e17..6145fe08f 100644 --- a/Release/include/pplx/pplxtasks.h +++ b/Release/include/pplx/pplxtasks.h @@ -3093,6 +3093,45 @@ namespace details template static std::false_type _IsValidCreateAsync(_Ty _Param, ...); #endif /* defined (__cplusplus_winrt) */ + +/// +/// A helper class template that makes only movable functions be able to be passed to std::function +/// + template + struct _NonCopyableFunctorWrapper + { + template + _NonCopyableFunctorWrapper(_Tx&& f) + : _M_functor{std::make_shared<_Ty>(std::forward<_Tx>(f))} + {} + + template + auto operator()(_Args&&... args) -> decltype(std::declval<_Ty>()(std::forward<_Args>(args)...)) + { + return _M_functor->operator()(std::forward<_Args>(args)...); + } + + template + auto operator()(_Args&&... args) const -> decltype(std::declval<_Ty>()(std::forward<_Args>(args)...)) + { + return _M_functor->operator()(std::forward<_Args>(args)...); + } + + std::shared_ptr<_Ty> _M_functor; + }; + + template + struct _CopyableFunctor + { + typedef _Ty _Type; + }; + + template + struct _CopyableFunctor<_Ty, typename std::enable_if< + std::is_move_constructible<_Ty>::value && !std::is_copy_constructible<_Ty>::value>::type> + { + typedef _NonCopyableFunctorWrapper<_Ty> _Type; + }; } /// /// A helper class template that transforms a continuation lambda that either takes or returns void, or both, into a lambda that takes and returns a @@ -3424,11 +3463,11 @@ public: /**/ template __declspec(noinline) // Ask for no inlining so that the _CAPTURE_CALLSTACK gives us the expected result - auto then(const _Function& _Func) const -> typename details::_ContinuationTypeTraits<_Function, _ReturnType>::_TaskOfType + auto then(_Function&& _Func) const -> typename details::_ContinuationTypeTraits<_Function, _ReturnType>::_TaskOfType { task_options _TaskOptions; details::_get_internal_task_options(_TaskOptions)._set_creation_callstack(_CAPTURE_CALLSTACK()); - return _ThenImpl<_ReturnType, _Function>(_Func, _TaskOptions); + return _ThenImpl<_ReturnType, _Function>(std::forward<_Function>(_Func), _TaskOptions); } /// @@ -3457,10 +3496,10 @@ public: /**/ template __declspec(noinline) // Ask for no inlining so that the _CAPTURE_CALLSTACK gives us the expected result - auto then(const _Function& _Func, task_options _TaskOptions) const -> typename details::_ContinuationTypeTraits<_Function, _ReturnType>::_TaskOfType + auto then(_Function&& _Func, task_options _TaskOptions) const -> typename details::_ContinuationTypeTraits<_Function, _ReturnType>::_TaskOfType { details::_get_internal_task_options(_TaskOptions)._set_creation_callstack(_CAPTURE_CALLSTACK()); - return _ThenImpl<_ReturnType, _Function>(_Func, _TaskOptions); + return _ThenImpl<_ReturnType, _Function>(std::forward<_Function>(_Func), _TaskOptions); } /// @@ -3493,11 +3532,11 @@ public: /**/ template __declspec(noinline) // Ask for no inlining so that the _CAPTURE_CALLSTACK gives us the expected result - auto then(const _Function& _Func, cancellation_token _CancellationToken, task_continuation_context _ContinuationContext) const -> typename details::_ContinuationTypeTraits<_Function, _ReturnType>::_TaskOfType + auto then(_Function&& _Func, cancellation_token _CancellationToken, task_continuation_context _ContinuationContext) const -> typename details::_ContinuationTypeTraits<_Function, _ReturnType>::_TaskOfType { task_options _TaskOptions(_CancellationToken, _ContinuationContext); details::_get_internal_task_options(_TaskOptions)._set_creation_callstack(_CAPTURE_CALLSTACK()); - return _ThenImpl<_ReturnType, _Function>(_Func, _TaskOptions); + return _ThenImpl<_ReturnType, _Function>(std::forward<_Function>(_Func), _TaskOptions); } /// @@ -3682,13 +3721,13 @@ public: /// This function is Used for runtime internal continuations only. /// template - auto _Then(const _Function& _Func, details::_CancellationTokenState *_PTokenState, + auto _Then(_Function&& _Func, details::_CancellationTokenState *_PTokenState, details::_TaskInliningMode_t _InliningMode = details::_ForceInline) const -> typename details::_ContinuationTypeTraits<_Function, _ReturnType>::_TaskOfType { // inherit from antecedent auto _Scheduler = _GetImpl()->_GetScheduler(); - return _ThenImpl<_ReturnType, _Function>(_Func, _PTokenState, task_continuation_context::use_default(), _Scheduler, _CAPTURE_CALLSTACK(), _InliningMode); + return _ThenImpl<_ReturnType, _Function>(std::forward<_Function>(_Func), _PTokenState, task_continuation_context::use_default(), _Scheduler, _CAPTURE_CALLSTACK(), _InliningMode); } private: @@ -3801,16 +3840,17 @@ private: typedef typename details::_NormalizeVoidToUnitType<_ContinuationReturnType>::_Type _NormalizedContinuationReturnType; typename details::_Task_ptr<_ReturnType>::_Type _M_ancestorTaskImpl; - _Function _M_function; + typename details::_CopyableFunctor<_Function>::_Type _M_function; + template _ContinuationTaskHandle(const typename details::_Task_ptr<_ReturnType>::_Type & _AncestorImpl, const typename details::_Task_ptr<_NormalizedContinuationReturnType>::_Type & _ContinuationImpl, - const _Function & _Func, const task_continuation_context & _Context, details::_TaskInliningMode_t _InliningMode) + _ForwardedFunction&& _Func, const task_continuation_context & _Context, details::_TaskInliningMode_t _InliningMode) : details::_PPLTaskHandle::_Type, _ContinuationTaskHandle<_InternalReturnType, _ContinuationReturnType, _Function, _IsTaskBased, _TypeSelection>, details::_ContinuationTaskHandleBase> ::_PPLTaskHandle(_ContinuationImpl) , _M_ancestorTaskImpl(_AncestorImpl) - , _M_function(_Func) + , _M_function(std::forward<_ForwardedFunction>(_Func)) { this->_M_isTaskBasedContinuation = _IsTaskBased::value; this->_M_continuationContext = _Context; @@ -4095,7 +4135,7 @@ private: } template - auto _ThenImpl(const _Function& _Func, const task_options& _TaskOptions) const -> typename details::_ContinuationTypeTraits<_Function, _InternalReturnType>::_TaskOfType + auto _ThenImpl(_Function&& _Func, const task_options& _TaskOptions) const -> typename details::_ContinuationTypeTraits<_Function, _InternalReturnType>::_TaskOfType { if (!_M_Impl) { @@ -4105,14 +4145,14 @@ private: details::_CancellationTokenState *_PTokenState = _TaskOptions.has_cancellation_token() ? _TaskOptions.get_cancellation_token()._GetImplValue() : nullptr; auto _Scheduler = _TaskOptions.has_scheduler() ? _TaskOptions.get_scheduler() : _GetImpl()->_GetScheduler(); auto _CreationStack = details::_get_internal_task_options(_TaskOptions)._M_hasPresetCreationCallstack ? details::_get_internal_task_options(_TaskOptions)._M_presetCreationCallstack : details::_TaskCreationCallstack(); - return _ThenImpl<_InternalReturnType, _Function>(_Func, _PTokenState, _TaskOptions.get_continuation_context(), _Scheduler, _CreationStack); + return _ThenImpl<_InternalReturnType, _Function>(std::forward<_Function>(_Func), _PTokenState, _TaskOptions.get_continuation_context(), _Scheduler, _CreationStack); } /// /// The one and only implementation of then for void and non-void tasks. /// template - auto _ThenImpl(const _Function& _Func, details::_CancellationTokenState *_PTokenState, const task_continuation_context& _ContinuationContext, scheduler_ptr _Scheduler, details::_TaskCreationCallstack _CreationStack, + auto _ThenImpl(_Function&& _Func, details::_CancellationTokenState *_PTokenState, const task_continuation_context& _ContinuationContext, scheduler_ptr _Scheduler, details::_TaskCreationCallstack _CreationStack, details::_TaskInliningMode_t _InliningMode = details::_NoInline) const -> typename details::_ContinuationTypeTraits<_Function, _InternalReturnType>::_TaskOfType { if (!_M_Impl) @@ -4149,7 +4189,7 @@ private: _ContinuationTask._SetTaskCreationCallstack(_CreationStack); _GetImpl()->_ScheduleContinuation(new _ContinuationTaskHandle<_InternalReturnType, _TaskType, _Function, typename _Function_type_traits::_Takes_task, typename _Async_type_traits::_AsyncKind>( - _GetImpl(), _ContinuationTask._GetImpl(), _Func, _ContinuationContext, _InliningMode)); + _GetImpl(), _ContinuationTask._GetImpl(), std::forward<_Function>(_Func), _ContinuationContext, _InliningMode)); return _ContinuationTask; } @@ -4371,10 +4411,10 @@ public: /**/ template __declspec(noinline) // Ask for no inlining so that the _CAPTURE_CALLSTACK gives us the expected result - auto then(const _Function& _Func, task_options _TaskOptions = task_options()) const -> typename details::_ContinuationTypeTraits<_Function, void>::_TaskOfType + auto then(_Function&& _Func, task_options _TaskOptions = task_options()) const -> typename details::_ContinuationTypeTraits<_Function, void>::_TaskOfType { details::_get_internal_task_options(_TaskOptions)._set_creation_callstack(_CAPTURE_CALLSTACK()); - return _M_unitTask._ThenImpl(_Func, _TaskOptions); + return _M_unitTask._ThenImpl(std::forward<_Function>(_Func), _TaskOptions); } /// @@ -4407,11 +4447,11 @@ public: /**/ template __declspec(noinline) // Ask for no inlining so that the _CAPTURE_CALLSTACK gives us the expected result - auto then(const _Function& _Func, cancellation_token _CancellationToken, task_continuation_context _ContinuationContext) const -> typename details::_ContinuationTypeTraits<_Function, void>::_TaskOfType + auto then(_Function&& _Func, cancellation_token _CancellationToken, task_continuation_context _ContinuationContext) const -> typename details::_ContinuationTypeTraits<_Function, void>::_TaskOfType { task_options _TaskOptions(_CancellationToken, _ContinuationContext); details::_get_internal_task_options(_TaskOptions)._set_creation_callstack(_CAPTURE_CALLSTACK()); - return _M_unitTask._ThenImpl(_Func, _TaskOptions); + return _M_unitTask._ThenImpl(std::forward<_Function>(_Func), _TaskOptions); } /// @@ -4555,13 +4595,13 @@ public: /// An internal version of then that takes additional flags and executes the continuation inline. Used for runtime internal continuations only. /// template - auto _Then(const _Function& _Func, details::_CancellationTokenState *_PTokenState, + auto _Then(_Function&& _Func, details::_CancellationTokenState *_PTokenState, details::_TaskInliningMode_t _InliningMode = details::_ForceInline) const -> typename details::_ContinuationTypeTraits<_Function, void>::_TaskOfType { // inherit from antecedent auto _Scheduler = _GetImpl()->_GetScheduler(); - return _M_unitTask._ThenImpl(_Func, _PTokenState, task_continuation_context::use_default(), _Scheduler, _CAPTURE_CALLSTACK(), _InliningMode); + return _M_unitTask._ThenImpl(std::forward<_Function>(_Func), _PTokenState, task_continuation_context::use_default(), _Scheduler, _CAPTURE_CALLSTACK(), _InliningMode); } private: diff --git a/Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp b/Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp index e0d3596e1..a6ea601ed 100644 --- a/Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp +++ b/Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp @@ -304,6 +304,21 @@ TEST(TestTasks_void_tasks_default_construction) } } +TEST(TestTasks_movable_then) +{ + struct A + { + A() = default; + A(A&&) = default; + A& operator=(A&&) = default; + } a; + + task task = create_task([]{ return 2; }); + auto f = task.then([a = std::move(a)](int) { return 'c'; }); + + IsTrue(f.get() == 'c', L".then should be able to work with movable functors"); +} + TEST(TestTasks_constant_this) { #ifdef _MSC_VER From 2320e0c94b75474cf6d1af8e4bdf639ac13d8486 Mon Sep 17 00:00:00 2001 From: AntonBikineev Date: Sat, 27 Feb 2016 22:08:26 +0300 Subject: [PATCH 2/5] Changing use of C++14 capture initialization to usual functor in pplxtask_tests.hpp --- .../functional/pplx/pplx_test/pplxtask_tests.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp b/Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp index a6ea601ed..c6f39e815 100644 --- a/Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp +++ b/Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp @@ -306,15 +306,25 @@ TEST(TestTasks_void_tasks_default_construction) TEST(TestTasks_movable_then) { + // create movable only type struct A { A() = default; A(A&&) = default; A& operator=(A&&) = default; + + // explicitly delete copy functions + A(const A&) = delete; + A& operator=(const A&) = delete; + + char operator()(int) + { + return 'c'; + } } a; task task = create_task([]{ return 2; }); - auto f = task.then([a = std::move(a)](int) { return 'c'; }); + auto f = task.then(std::move(a)); IsTrue(f.get() == 'c', L".then should be able to work with movable functors"); } From ff4f31157deac23173150f5ab48243b810f336bf Mon Sep 17 00:00:00 2001 From: AntonBikineev Date: Sun, 28 Feb 2016 03:04:12 +0300 Subject: [PATCH 3/5] Disabling movable_then test on Windows --- Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp b/Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp index c6f39e815..e38398f85 100644 --- a/Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp +++ b/Release/tests/functional/pplx/pplx_test/pplxtask_tests.cpp @@ -306,6 +306,7 @@ TEST(TestTasks_void_tasks_default_construction) TEST(TestTasks_movable_then) { +#ifndef _MSC_VER // create movable only type struct A { @@ -327,6 +328,7 @@ TEST(TestTasks_movable_then) auto f = task.then(std::move(a)); IsTrue(f.get() == 'c', L".then should be able to work with movable functors"); +#endif // _MSC_VER } TEST(TestTasks_constant_this) From cd9714182ed4b928553e51272713c391babc5f78 Mon Sep 17 00:00:00 2001 From: AntonBikineev Date: Tue, 1 Mar 2016 08:29:20 -0600 Subject: [PATCH 4/5] Fixing noncopyablewrapper ctor --- Release/include/pplx/pplxtasks.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Release/include/pplx/pplxtasks.h b/Release/include/pplx/pplxtasks.h index 6145fe08f..bda98e46d 100644 --- a/Release/include/pplx/pplxtasks.h +++ b/Release/include/pplx/pplxtasks.h @@ -3100,8 +3100,10 @@ namespace details template struct _NonCopyableFunctorWrapper { - template - _NonCopyableFunctorWrapper(_Tx&& f) + template, + typename std::decay<_Tx>::type>::value>::type> + explicit _NonCopyableFunctorWrapper(_Tx&& f) : _M_functor{std::make_shared<_Ty>(std::forward<_Tx>(f))} {} From 2e23b2adf1aad9508bba14b6e48647d41690d971 Mon Sep 17 00:00:00 2001 From: AntonBikineev Date: Wed, 2 Mar 2016 03:56:16 -0600 Subject: [PATCH 5/5] Adding decay to noncopyablefunctorwrapper --- Release/include/pplx/pplxtasks.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Release/include/pplx/pplxtasks.h b/Release/include/pplx/pplxtasks.h index bda98e46d..760a61453 100644 --- a/Release/include/pplx/pplxtasks.h +++ b/Release/include/pplx/pplxtasks.h @@ -3842,7 +3842,7 @@ private: typedef typename details::_NormalizeVoidToUnitType<_ContinuationReturnType>::_Type _NormalizedContinuationReturnType; typename details::_Task_ptr<_ReturnType>::_Type _M_ancestorTaskImpl; - typename details::_CopyableFunctor<_Function>::_Type _M_function; + typename details::_CopyableFunctor::type >::_Type _M_function; template _ContinuationTaskHandle(const typename details::_Task_ptr<_ReturnType>::_Type & _AncestorImpl,