From 7ff13f1eaa0d1207abda40741e9df87c13f43e7e Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Mon, 6 Nov 2023 17:48:56 +0100 Subject: [PATCH] TaskTree: Introduce DoneResult, use it for Sync setup Instead of using ambiguous bool. Change-Id: Iec45b920a839ac3383abc2d0676e5834f282dddf Reviewed-by: hjk --- src/libs/solutions/tasking/tasktree.h | 17 ++++++++---- src/plugins/cppeditor/cppincludesfilter.cpp | 2 +- .../nim/project/nimcompilercleanstep.cpp | 8 +++--- tests/auto/solutions/tasking/tst_tasking.cpp | 27 +++++++++++-------- 4 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/libs/solutions/tasking/tasktree.h b/src/libs/solutions/tasking/tasktree.h index 5743beff518..02ca981d3ba 100644 --- a/src/libs/solutions/tasking/tasktree.h +++ b/src/libs/solutions/tasking/tasktree.h @@ -159,6 +159,13 @@ enum class SetupResult }; Q_ENUM_NS(SetupResult); +enum class DoneResult +{ + Success, + Error +}; +Q_ENUM_NS(DoneResult); + enum class DoneWith { Success, @@ -374,16 +381,16 @@ public: private: template static GroupSetupHandler wrapHandler(Handler &&handler) { - // B, V stands for: [B]ool, [V]oid - static constexpr bool isB = isInvocable(); + // D, V stands for: [D]oneResult, [V]oid + static constexpr bool isB = isInvocable(); static constexpr bool isV = isInvocable(); static_assert(isB || isV, - "Sync handler needs to take no arguments and has to return void or bool. " + "Sync handler needs to take no arguments and has to return void or DoneResult. " "The passed handler doesn't fulfill these requirements."); return [=] { if constexpr (isB) { - return std::invoke(handler) ? SetupResult::StopWithSuccess - : SetupResult::StopWithError; + return std::invoke(handler) == DoneResult::Success ? SetupResult::StopWithSuccess + : SetupResult::StopWithError; } std::invoke(handler); return SetupResult::StopWithSuccess; diff --git a/src/plugins/cppeditor/cppincludesfilter.cpp b/src/plugins/cppeditor/cppincludesfilter.cpp index e1df13f7a2d..d955dbeb43b 100644 --- a/src/plugins/cppeditor/cppincludesfilter.cpp +++ b/src/plugins/cppeditor/cppincludesfilter.cpp @@ -59,7 +59,7 @@ CppIncludesFilter::CppIncludesFilter() setDefaultShortcutString("ai"); setDefaultIncludedByDefault(true); const auto invalidate = [this] { m_cache.invalidate(); }; - setRefreshRecipe(Tasking::Sync([invalidate] { invalidate(); return true; })); + setRefreshRecipe(Tasking::Sync([invalidate] { invalidate(); })); setPriority(ILocatorFilter::Low); connect(ProjectExplorerPlugin::instance(), &ProjectExplorerPlugin::fileListChanged, diff --git a/src/plugins/nim/project/nimcompilercleanstep.cpp b/src/plugins/nim/project/nimcompilercleanstep.cpp index edfe1b370cb..cb497638b57 100644 --- a/src/plugins/nim/project/nimcompilercleanstep.cpp +++ b/src/plugins/nim/project/nimcompilercleanstep.cpp @@ -63,19 +63,19 @@ GroupItem NimCompilerCleanStep::runRecipe() if (!m_buildDir.exists()) { emit addOutput(Tr::tr("Build directory \"%1\" does not exist.") .arg(m_buildDir.toUserOutput()), OutputFormat::ErrorMessage); - return false; + return DoneResult::Error; } if (!removeCacheDirectory()) { emit addOutput(Tr::tr("Failed to delete the cache directory."), OutputFormat::ErrorMessage); - return false; + return DoneResult::Error; } if (!removeOutFilePath()) { emit addOutput(Tr::tr("Failed to delete the out file."), OutputFormat::ErrorMessage); - return false; + return DoneResult::Error; } emit addOutput(Tr::tr("Clean step completed successfully."), OutputFormat::NormalMessage); - return true; + return DoneResult::Success; }; return Sync(onSetup); } diff --git a/tests/auto/solutions/tasking/tst_tasking.cpp b/tests/auto/solutions/tasking/tst_tasking.cpp index aa8068cfbcd..c46072a78d3 100644 --- a/tests/auto/solutions/tasking/tst_tasking.cpp +++ b/tests/auto/solutions/tasking/tst_tasking.cpp @@ -397,6 +397,11 @@ static Handler doneToTweakHandler(bool result) return result ? Handler::TweakDoneToSuccess : Handler::TweakDoneToError; } +static Handler doneToTweakHandler(DoneResult result) +{ + return result == DoneResult::Success ? Handler::TweakDoneToSuccess : Handler::TweakDoneToError; +} + static TestData storageShadowing() { // This test check if storage shadowing works OK. @@ -555,11 +560,11 @@ void tst_Tasking::testTree_data() const auto createSync = [storage](int taskId) { return Sync([storage, taskId] { storage->m_log.append({taskId, Handler::Sync}); }); }; - const auto createSyncWithTweak = [storage](int taskId, bool desiredResult) { - return Sync([storage, taskId, desiredResult] { + 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(desiredResult)}); - return desiredResult; + storage->m_log.append({taskId, doneToTweakHandler(result)}); + return result; }); }; @@ -2083,11 +2088,11 @@ void tst_Tasking::testTree_data() { const Group root { Storage(storage), - createSyncWithTweak(1, true), - createSyncWithTweak(2, true), - createSyncWithTweak(3, true), - createSyncWithTweak(4, true), - createSyncWithTweak(5, true) + createSyncWithTweak(1, DoneResult::Success), + createSyncWithTweak(2, DoneResult::Success), + createSyncWithTweak(3, DoneResult::Success), + createSyncWithTweak(4, DoneResult::Success), + createSyncWithTweak(5, DoneResult::Success) }; const Log log { {1, Handler::Sync}, @@ -2130,7 +2135,7 @@ void tst_Tasking::testTree_data() parallel, createSync(1), createSync(2), - createSyncWithTweak(3, false), + createSyncWithTweak(3, DoneResult::Error), createSync(4), createSync(5) }; @@ -2171,7 +2176,7 @@ void tst_Tasking::testTree_data() Storage(storage), createSync(1), createSuccessTask(2), - createSyncWithTweak(3, false), + createSyncWithTweak(3, DoneResult::Error), createSuccessTask(4), createSync(5), groupDone(0)