From a7264d9b8c0a50e5ffbf0ea5e7b908637687d9f4 Mon Sep 17 00:00:00 2001 From: Sona Kurazyan Date: Wed, 15 Apr 2020 11:44:46 +0200 Subject: [PATCH] Make continuations work with move-only types Use the move-only versions of result reporting and getting operations, if the type of QFuture is not copyable. Task-number: QTBUG-81941 Change-Id: Ic9fa978380e2c24e190e68d974051a650b0e5571 Reviewed-by: Qt CI Bot Reviewed-by: Vitaly Fanaskov --- src/corelib/thread/qfuture_impl.h | 104 +++++++++++++----- .../corelib/thread/qfuture/tst_qfuture.cpp | 59 +++++++++- 2 files changed, 136 insertions(+), 27 deletions(-) diff --git a/src/corelib/thread/qfuture_impl.h b/src/corelib/thread/qfuture_impl.h index 4a57ba7846c..5a4ffce1c79 100644 --- a/src/corelib/thread/qfuture_impl.h +++ b/src/corelib/thread/qfuture_impl.h @@ -186,6 +186,14 @@ public: static void create(Function &&func, QFuture *f, QFutureInterface &p, QThreadPool *pool); +private: + void fulfillPromiseWithResult(); + void fulfillVoidPromise(); + void fulfillPromiseWithVoidResult(); + + template + void fulfillPromise(Args &&... args); + protected: virtual void runImpl() = 0; @@ -193,7 +201,7 @@ protected: protected: QFutureInterface promise; - const QFuture parentFuture; + QFuture parentFuture; Function function; }; @@ -269,7 +277,7 @@ private: private: QFutureInterface promise; - const QFuture parentFuture; + QFuture parentFuture; Function handler; }; @@ -286,32 +294,31 @@ void Continuation::runFunction() try { #endif if constexpr (!std::is_void_v) { - if constexpr (std::is_invocable_v, QFuture>) { - promise.reportResult(function(parentFuture)); - } else if constexpr (std::is_void_v) { - promise.reportResult(function()); + if constexpr (std::is_void_v) { + fulfillPromiseWithVoidResult(); + } else if constexpr (std::is_invocable_v) { + fulfillPromiseWithResult(); } else { // This assert normally should never fail, this is to make sure // that nothing unexpected happend. - static_assert( - std::is_invocable_v, std::decay_t>, - "The continuation is not invocable with the provided arguments"); - - promise.reportResult(function(parentFuture.result())); + static_assert(std::is_invocable_v>, + "The continuation is not invocable with the provided arguments"); + fulfillPromise(parentFuture); } } else { - if constexpr (std::is_invocable_v, QFuture>) { - function(parentFuture); - } else if constexpr (std::is_void_v) { - function(); + if constexpr (std::is_void_v) { + if constexpr (std::is_invocable_v>) + function(parentFuture); + else + function(); + } else if constexpr (std::is_invocable_v) { + fulfillVoidPromise(); } else { // This assert normally should never fail, this is to make sure // that nothing unexpected happend. - static_assert( - std::is_invocable_v, std::decay_t>, - "The continuation is not invocable with the provided arguments"); - - function(parentFuture.result()); + static_assert(std::is_invocable_v>, + "The continuation is not invocable with the provided arguments"); + function(parentFuture); } } #ifndef QT_NO_EXCEPTIONS @@ -426,6 +433,43 @@ void Continuation::create(Function &&fun f->d.setContinuation(continuation); } +template +void Continuation::fulfillPromiseWithResult() +{ + if constexpr (std::is_copy_constructible_v) + fulfillPromise(parentFuture.result()); + else + fulfillPromise(parentFuture.takeResult()); +} + +template +void Continuation::fulfillVoidPromise() +{ + if constexpr (std::is_copy_constructible_v) + function(parentFuture.result()); + else + function(parentFuture.takeResult()); +} + +template +void Continuation::fulfillPromiseWithVoidResult() +{ + if constexpr (std::is_invocable_v>) + fulfillPromise(parentFuture); + else + fulfillPromise(); +} + +template +template +void Continuation::fulfillPromise(Args &&... args) +{ + if constexpr (std::is_copy_constructible_v) + promise.reportResult(std::invoke(function, std::forward(args)...)); + else + promise.reportAndMoveResult(std::invoke(function, std::forward(args)...)); +} + #ifndef QT_NO_EXCEPTIONS template @@ -460,8 +504,12 @@ void FailureHandler::run() handleException(); } } else { - if constexpr (!std::is_void_v) - promise.reportResult(parentFuture.result()); + if constexpr (!std::is_void_v) { + if constexpr (std::is_copy_constructible_v) + promise.reportResult(parentFuture.result()); + else + promise.reportAndMoveResult(parentFuture.takeResult()); + } } promise.reportFinished(); } @@ -478,7 +526,10 @@ void FailureHandler::handleException() if constexpr (std::is_void_v) { handler(e); } else { - promise.reportResult(handler(e)); + if constexpr (std::is_copy_constructible_v) + promise.reportResult(handler(e)); + else + promise.reportAndMoveResult(handler(e)); } } catch (...) { promise.reportException(std::current_exception()); @@ -497,11 +548,12 @@ void FailureHandler::handleAllExceptions() parentFuture.d.exceptionStore().throwPossibleException(); } catch (...) { try { - if constexpr (std::is_void_v) { + if constexpr (std::is_void_v) handler(); - } else { + else if constexpr (std::is_copy_constructible_v) promise.reportResult(handler()); - } + else + promise.reportAndMoveResult(handler()); } catch (...) { promise.reportException(std::current_exception()); } diff --git a/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp b/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp index 1caa3866388..4784cf16dbb 100644 --- a/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp +++ b/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp @@ -71,6 +71,8 @@ private: std::function m_fn; }; +using UniquePtr = std::unique_ptr; + class tst_QFuture: public QObject { Q_OBJECT @@ -100,12 +102,14 @@ private slots: void nonGlobalThreadPool(); void then(); + void thenForMoveOnlyTypes(); void thenOnCanceledFuture(); #ifndef QT_NO_EXCEPTIONS void thenOnExceptionFuture(); void thenThrows(); void onFailed(); void onFailedTestCallables(); + void onFailedForMoveOnlyTypes(); #endif void takeResults(); void takeResult(); @@ -114,7 +118,6 @@ private slots: void resultsReadyAt(); private: using size_type = std::vector::size_type; - using UniquePtr = std::unique_ptr; static void testSingleResult(const UniquePtr &p); static void testSingleResult(const std::vector &v); @@ -1874,6 +1877,38 @@ void tst_QFuture::then() } } +template +bool runThenForMoveOnly(Callable &&callable) +{ + QFutureInterface promise; + auto future = promise.future(); + + auto then = future.then(std::forward(callable)); + + promise.reportStarted(); + if constexpr (!std::is_same_v) + promise.reportAndMoveResult(std::make_unique(42)); + promise.reportFinished(); + then.waitForFinished(); + + bool success = true; + if constexpr (!std::is_same_v>) + success &= *then.takeResult() == 42; + + if constexpr (!std::is_same_v) + success &= !future.isValid(); + + return success; +} + +void tst_QFuture::thenForMoveOnlyTypes() +{ + QVERIFY(runThenForMoveOnly([](UniquePtr res) { return res; })); + QVERIFY(runThenForMoveOnly([](UniquePtr res) { Q_UNUSED(res); })); + QVERIFY(runThenForMoveOnly([](QFuture res) { return res.takeResult(); })); + QVERIFY(runThenForMoveOnly([] { return std::make_unique(42); })); +} + void tst_QFuture::thenOnCanceledFuture() { // Continuations on a canceled future @@ -2482,6 +2517,28 @@ void tst_QFuture::onFailedTestCallables() QVERIFY(runForCallable(Functor3())); } +template +bool runOnFailedForMoveOnly(Callable &&callable) +{ + QFutureInterface promise; + auto future = promise.future(); + + auto failedFuture = future.onFailed(std::forward(callable)); + + promise.reportStarted(); + QException e; + promise.reportException(e); + promise.reportFinished(); + + return *failedFuture.takeResult() == -1; +} + +void tst_QFuture::onFailedForMoveOnlyTypes() +{ + QVERIFY(runOnFailedForMoveOnly([](const QException &) { return std::make_unique(-1); })); + QVERIFY(runOnFailedForMoveOnly([] { return std::make_unique(-1); })); +} + #endif // QT_NO_EXCEPTIONS void tst_QFuture::testSingleResult(const UniquePtr &p)