From 0f6e6cebf712673c09b50390ffe6b4bc448a9bae Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Mon, 6 Nov 2023 18:52:19 +0100 Subject: [PATCH] TaskTree: Use DoneResult in Task done handler Instead of using ambiguous bool. Change-Id: I7eb8e97947f23161c5c2bf5e22575e49bea35d61 Reviewed-by: hjk --- src/libs/solutions/tasking/tasktree.cpp | 17 ++-- src/libs/solutions/tasking/tasktree.h | 36 ++++----- tests/auto/solutions/tasking/tst_tasking.cpp | 85 ++++++++++---------- 3 files changed, 72 insertions(+), 66 deletions(-) diff --git a/src/libs/solutions/tasking/tasktree.cpp b/src/libs/solutions/tasking/tasktree.cpp index ba90500bccb..0452a047709 100644 --- a/src/libs/solutions/tasking/tasktree.cpp +++ b/src/libs/solutions/tasking/tasktree.cpp @@ -837,6 +837,11 @@ static SetupResult toSetupResult(bool success) return success ? SetupResult::StopWithSuccess : SetupResult::StopWithError; } +static DoneResult toDoneResult(DoneWith doneWith) +{ + return doneWith == DoneWith::Success ? DoneResult::Success : DoneResult::Error; +} + bool TreeStorageBase::isValid() const { return m_storageData && m_storageData->m_constructor && m_storageData->m_destructor; @@ -1136,7 +1141,7 @@ public: // in order to unwind properly. SetupResult start(); void stop(); - bool invokeDoneHandler(DoneWith result); + bool invokeDoneHandler(DoneWith doneWith); bool isRunning() const { return m_task || m_container.isRunning(); } bool isTask() const { return bool(m_taskHandler.m_createHandler); } int taskCount() const { return isTask() ? 1 : m_container.m_constData.m_taskCount; } @@ -1507,13 +1512,13 @@ void TaskNode::stop() m_task.reset(); } -bool TaskNode::invokeDoneHandler(DoneWith result) +bool TaskNode::invokeDoneHandler(DoneWith doneWith) { - bool success = result == DoneWith::Success; - if (m_taskHandler.m_doneHandler && shouldCall(m_taskHandler.m_callDoneIf, result)) - success = invokeHandler(parentContainer(), m_taskHandler.m_doneHandler, *m_task.get(), result); + DoneResult result = toDoneResult(doneWith); + if (m_taskHandler.m_doneHandler && shouldCall(m_taskHandler.m_callDoneIf, doneWith)) + result = invokeHandler(parentContainer(), m_taskHandler.m_doneHandler, *m_task.get(), doneWith); m_container.m_constData.m_taskTreePrivate->advanceProgress(1); - return success; + return result == DoneResult::Success; } /*! diff --git a/src/libs/solutions/tasking/tasktree.h b/src/libs/solutions/tasking/tasktree.h index 02ca981d3ba..3690fcf291e 100644 --- a/src/libs/solutions/tasking/tasktree.h +++ b/src/libs/solutions/tasking/tasktree.h @@ -190,7 +190,7 @@ public: // Called prior to task start, just after createHandler using TaskSetupHandler = std::function; // Called on task done, just before deleteLater - using TaskDoneHandler = std::function; + using TaskDoneHandler = std::function; // Called when group entered, after group's storages are created using GroupSetupHandler = std::function; // Called when group done, before group's storages are deleted @@ -423,7 +423,7 @@ public: "The Adapter type for the CustomTask needs to be derived from " "TaskAdapter."); using SetupFunction = std::function; - using DoneFunction = std::function; + using DoneFunction = std::function; static Adapter *createAdapter() { return new Adapter; } template @@ -463,38 +463,38 @@ private: static GroupItem::TaskDoneHandler wrapDone(Handler &&handler) { if constexpr (std::is_same_v) return {}; // When user passed {} for the done handler. - // B, V, T, D stands for: [B]ool, [V]oid, [T]ask, [D]oneWith - static constexpr bool isBTD = isInvocable(); - static constexpr bool isBT = isInvocable(); - static constexpr bool isBD = isInvocable(); - static constexpr bool isB = isInvocable(); - static constexpr bool isVTD = isInvocable(); + // D, V, T, W stands for: [D]oneResult, [V]oid, [T]ask, done[W]ith + static constexpr bool isDTW = isInvocable(); + static constexpr bool isDT = isInvocable(); + static constexpr bool isDW = isInvocable(); + static constexpr bool isD = isInvocable(); + static constexpr bool isVTW = isInvocable(); static constexpr bool isVT = isInvocable(); - static constexpr bool isVD = isInvocable(); + static constexpr bool isVW = isInvocable(); static constexpr bool isV = isInvocable(); - static_assert(isBTD || isBT || isBD || isB || isVTD || isVT || isVD || isV, + static_assert(isDTW || isDT || isDW || isD || isVTW || isVT || isVW || isV, "Task done handler needs to take (const Task &, DoneWith), (const Task &), " - "(DoneWith) or (void) as arguments and has to return void or bool. " + "(DoneWith) or (void) as arguments and has to return void or DoneResult. " "The passed handler doesn't fulfill these requirements."); return [=](const TaskInterface &taskInterface, DoneWith result) { const Adapter &adapter = static_cast(taskInterface); - if constexpr (isBTD) + if constexpr (isDTW) return std::invoke(handler, *adapter.task(), result); - if constexpr (isBT) + if constexpr (isDT) return std::invoke(handler, *adapter.task()); - if constexpr (isBD) + if constexpr (isDW) return std::invoke(handler, result); - if constexpr (isB) + if constexpr (isD) return std::invoke(handler); - if constexpr (isVTD) + if constexpr (isVTW) std::invoke(handler, *adapter.task(), result); else if constexpr (isVT) std::invoke(handler, *adapter.task()); - else if constexpr (isVD) + else if constexpr (isVW) std::invoke(handler, result); else if constexpr (isV) std::invoke(handler); - return result == DoneWith::Success; + return result == DoneWith::Success ? DoneResult::Success : DoneResult::Error; }; }; }; diff --git a/tests/auto/solutions/tasking/tst_tasking.cpp b/tests/auto/solutions/tasking/tst_tasking.cpp index c46072a78d3..9dc0bcdaf88 100644 --- a/tests/auto/solutions/tasking/tst_tasking.cpp +++ b/tests/auto/solutions/tasking/tst_tasking.cpp @@ -372,32 +372,32 @@ GroupItem createBarrierAdvance(const TreeStorage &storage, }); } -static Handler resultToGroupHandler(DoneWith result) +static Handler resultToGroupHandler(DoneWith doneWith) { - switch (result) { - case DoneWith::Success : return Handler::GroupSuccess; - case DoneWith::Error : return Handler::GroupError; - case DoneWith::Cancel : return Handler::GroupCanceled; + switch (doneWith) { + case DoneWith::Success: return Handler::GroupSuccess; + case DoneWith::Error: return Handler::GroupError; + case DoneWith::Cancel: return Handler::GroupCanceled; } return Handler::GroupCanceled; } -static Handler setupToTweakHandler(SetupResult result) +static Handler toTweakSetupHandler(SetupResult result) { switch (result) { - case SetupResult::Continue : return Handler::TweakSetupToContinue; - case SetupResult::StopWithSuccess : return Handler::TweakSetupToSuccess; - case SetupResult::StopWithError : return Handler::TweakSetupToError; + case SetupResult::Continue: return Handler::TweakSetupToContinue; + case SetupResult::StopWithSuccess: return Handler::TweakSetupToSuccess; + case SetupResult::StopWithError: return Handler::TweakSetupToError; } return Handler::TweakSetupToContinue; } -static Handler doneToTweakHandler(bool result) +static Handler toTweakDoneHandler(bool success) { - return result ? Handler::TweakDoneToSuccess : Handler::TweakDoneToError; + return success ? Handler::TweakDoneToSuccess : Handler::TweakDoneToError; } -static Handler doneToTweakHandler(DoneResult result) +static Handler toTweakDoneHandler(DoneResult result) { return result == DoneResult::Success ? Handler::TweakDoneToSuccess : Handler::TweakDoneToError; } @@ -492,20 +492,21 @@ void tst_Tasking::testTree_data() }; }; - const auto setupTaskWithTweak = [storage](int taskId, SetupResult desiredResult) { - return [storage, taskId, desiredResult](TaskObject &) { + const auto setupTaskWithTweak = [storage](int taskId, SetupResult result) { + return [storage, taskId, result](TaskObject &) { storage->m_log.append({taskId, Handler::Setup}); - storage->m_log.append({taskId, setupToTweakHandler(desiredResult)}); - return desiredResult; + storage->m_log.append({taskId, toTweakSetupHandler(result)}); + return result; }; }; - const auto setupDone = [storage](int taskId, bool desiredResult = true) { - return [storage, taskId, desiredResult](DoneWith result) { - const Handler handler = result == DoneWith::Cancel ? Handler::Canceled - : desiredResult ? Handler::Success : Handler::Error; + const auto setupDone = [storage](int taskId, DoneResult result = DoneResult::Success) { + return [storage, taskId, result](DoneWith doneWith) { + const Handler handler = doneWith == DoneWith::Cancel ? Handler::Canceled + : result == DoneResult::Success ? Handler::Success : Handler::Error; storage->m_log.append({taskId, handler}); - return desiredResult && result != DoneWith::Cancel; + return doneWith == DoneWith::Cancel ? DoneResult::Error + : result == DoneResult::Success ? DoneResult::Success : DoneResult::Error; }; }; @@ -516,16 +517,16 @@ void tst_Tasking::testTree_data() }; const auto createTask = [storage, setupTask, setupDone]( - int taskId, bool successTask, milliseconds timeout = 0ms) { - return TestTask(setupTask(taskId, timeout), setupDone(taskId, successTask)); + int taskId, DoneResult result, milliseconds timeout = 0ms) { + return TestTask(setupTask(taskId, timeout), setupDone(taskId, result)); }; const auto createSuccessTask = [createTask](int taskId, milliseconds timeout = 0ms) { - return createTask(taskId, true, timeout); + return createTask(taskId, DoneResult::Success, timeout); }; const auto createFailingTask = [createTask](int taskId, milliseconds timeout = 0ms) { - return createTask(taskId, false, timeout); + return createTask(taskId, DoneResult::Error, timeout); }; const auto createTaskWithSetupTweak = [storage, setupTaskWithTweak, setupDone]( @@ -546,14 +547,14 @@ void tst_Tasking::testTree_data() const auto groupSetupWithTweak = [storage](int taskId, SetupResult desiredResult) { return onGroupSetup([storage, taskId, desiredResult] { storage->m_log.append({taskId, Handler::GroupSetup}); - storage->m_log.append({taskId, setupToTweakHandler(desiredResult)}); + storage->m_log.append({taskId, toTweakSetupHandler(desiredResult)}); return desiredResult; }); }; const auto groupDoneWithTweak = [storage](int taskId, bool desiredResult) { return onGroupDone([storage, taskId, desiredResult](DoneWith result) { storage->m_log.append({taskId, resultToGroupHandler(result)}); - storage->m_log.append({taskId, doneToTweakHandler(desiredResult)}); + storage->m_log.append({taskId, toTweakDoneHandler(desiredResult)}); return desiredResult; }); }; @@ -563,7 +564,7 @@ void tst_Tasking::testTree_data() const auto createSyncWithTweak = [storage](int taskId, DoneResult result) { return Sync([storage, taskId, result] { storage->m_log.append({taskId, Handler::Sync}); - storage->m_log.append({taskId, doneToTweakHandler(result)}); + storage->m_log.append({taskId, toTweakDoneHandler(result)}); return result; }); }; @@ -1525,22 +1526,22 @@ void tst_Tasking::testTree_data() } { - const auto createRoot = [storage, createTask, groupDone]( - bool firstSuccess, bool secondSuccess) { + const auto createRoot = [storage, createTask, groupDone](DoneResult firstResult, + DoneResult secondResult) { return Group { parallel, stopOnFinished, Storage(storage), - createTask(1, firstSuccess, 1000ms), - createTask(2, secondSuccess, 1ms), + createTask(1, firstResult, 1000ms), + createTask(2, secondResult, 1ms), groupDone(0) }; }; - const Group root1 = createRoot(true, true); - const Group root2 = createRoot(true, false); - const Group root3 = createRoot(false, true); - const Group root4 = createRoot(false, false); + const Group root1 = createRoot(DoneResult::Success, DoneResult::Success); + const Group root2 = createRoot(DoneResult::Success, DoneResult::Error); + const Group root3 = createRoot(DoneResult::Error, DoneResult::Success); + const Group root4 = createRoot(DoneResult::Error, DoneResult::Error); const Log success { {1, Handler::Setup}, @@ -1610,18 +1611,18 @@ void tst_Tasking::testTree_data() { // This test checks whether group done handler's result is properly dispatched. const auto createRoot = [storage, createTask, groupDone, groupDoneWithTweak]( - bool successTask, bool desiredResult) { + DoneResult firstResult, bool desiredResult) { return Group { Storage(storage), Group { - createTask(1, successTask), + createTask(1, firstResult), groupDoneWithTweak(1, desiredResult) }, groupDone(0) }; }; - const Group root1 = createRoot(true, true); + const Group root1 = createRoot(DoneResult::Success, true); const Log log1 { {1, Handler::Setup}, {1, Handler::Success}, @@ -1632,7 +1633,7 @@ void tst_Tasking::testTree_data() QTest::newRow("GroupDoneWithSuccessTweakToSuccess") << TestData{storage, root1, log1, 1, OnDone::Success}; - const Group root2 = createRoot(true, false); + const Group root2 = createRoot(DoneResult::Success, false); const Log log2 { {1, Handler::Setup}, {1, Handler::Success}, @@ -1643,7 +1644,7 @@ void tst_Tasking::testTree_data() QTest::newRow("GroupDoneWithSuccessTweakToError") << TestData{storage, root2, log2, 1, OnDone::Failure}; - const Group root3 = createRoot(false, true); + const Group root3 = createRoot(DoneResult::Error, true); const Log log3 { {1, Handler::Setup}, {1, Handler::Error}, @@ -1654,7 +1655,7 @@ void tst_Tasking::testTree_data() QTest::newRow("GroupDoneWithErrorTweakToSuccess") << TestData{storage, root3, log3, 1, OnDone::Success}; - const Group root4 = createRoot(false, false); + const Group root4 = createRoot(DoneResult::Error, false); const Log log4 { {1, Handler::Setup}, {1, Handler::Error},