TaskTree: Hide more internals inside StorageBase class

Rename storage internals.

This addresses the 20th point in the master task below.

Task-number: QTCREATORBUG-28741
Change-Id: I1ed42472060de6180d4665d6504598adb57828fe
Reviewed-by: hjk <hjk@qt.io>

src/libs/solutions/tasking/tasktree.cpp

@@ -897,8 +897,8 @@ public:
             m_threadDataMap.find(QThread::currentThread()));
     }
 
-    const TreeStorageBase::StorageConstructor m_constructor = {};
+    const StorageBase::StorageConstructor m_constructor = {};
-    const TreeStorageBase::StorageDestructor m_destructor = {};
+    const StorageBase::StorageDestructor m_destructor = {};
     QMutex m_threadDataMutex = {};
     // Use std::map on purpose, so that it doesn't invalidate references on modifications.
     // Don't optimize it by using std::unordered_map.
@@ -972,16 +972,16 @@ StorageData::~StorageData()
     QT_CHECK(m_threadDataMap.empty());
 }
 
-bool TreeStorageBase::isValid() const
+bool StorageBase::isValid() const
 {
     return m_storageData && m_storageData->m_constructor && m_storageData->m_destructor;
 }
 
-TreeStorageBase::TreeStorageBase(StorageConstructor ctor, StorageDestructor dtor)
+StorageBase::StorageBase(StorageConstructor ctor, StorageDestructor dtor)
     : m_storageData(new StorageData{ctor, dtor})
 {}
 
-void *TreeStorageBase::activeStorageVoid() const
+void *StorageBase::activeStorageVoid() const
 {
     return m_storageData->threadData().activeStorageVoid();
 }
@@ -1033,7 +1033,7 @@ void GroupItem::addChildren(const QList<GroupItem> &children)
             break;
         case Type::Storage:
             // Check for duplicates, as can't have the same storage twice on the same level.
-            for (const TreeStorageBase &storage : child.m_storageList) {
+            for (const StorageBase &storage : child.m_storageList) {
                 if (m_storageList.contains(storage)) {
                     QT_ASSERT(false, qWarning("Can't add the same storage into one Group twice, "
                                               "skipping..."));
@@ -1078,7 +1078,7 @@ public:
 
 private:
     void activateContext(TaskRuntimeContainer *container);
-    QList<TreeStorageBase> m_activeStorages;
+    QList<StorageBase> m_activeStorages;
 };
 
 class TaskContainer
@@ -1094,7 +1094,7 @@ public:
     const int m_parallelLimit = 1;
     const WorkflowPolicy m_workflowPolicy = WorkflowPolicy::StopOnError;
     const GroupItem::GroupHandler m_groupHandler;
-    const QList<TreeStorageBase> m_storageList;
+    const QList<StorageBase> m_storageList;
     std::vector<TaskNode> m_children;
     const int m_taskCount = 0;
 };
@@ -1131,18 +1131,18 @@ public:
     void emitStartedAndProgress();
     void emitProgress();
     void emitDone(DoneWith result);
-    void callSetupHandler(TreeStorageBase storage, int storageId) {
+    void callSetupHandler(StorageBase storage, int storageId) {
         callStorageHandler(storage, storageId, &StorageHandler::m_setupHandler);
     }
-    void callDoneHandler(TreeStorageBase storage, int storageId) {
+    void callDoneHandler(StorageBase storage, int storageId) {
         callStorageHandler(storage, storageId, &StorageHandler::m_doneHandler);
     }
     struct StorageHandler {
-        TreeStorageBase::StorageVoidHandler m_setupHandler = {};
+        StorageBase::StorageHandler m_setupHandler = {};
-        TreeStorageBase::StorageVoidHandler m_doneHandler = {};
+        StorageBase::StorageHandler m_doneHandler = {};
     };
-    typedef TreeStorageBase::StorageVoidHandler StorageHandler::*HandlerPtr; // ptr to class member
+    typedef StorageBase::StorageHandler StorageHandler::*HandlerPtr; // ptr to class member
-    void callStorageHandler(TreeStorageBase storage, int storageId, HandlerPtr ptr)
+    void callStorageHandler(StorageBase storage, int storageId, HandlerPtr ptr)
     {
         const auto it = m_storageHandlers.constFind(storage);
         if (it == m_storageHandlers.constEnd())
@@ -1187,8 +1187,8 @@ public:
     TaskTree *q = nullptr;
     Guard m_guard;
     int m_progressValue = 0;
-    QSet<TreeStorageBase> m_storages;
+    QSet<StorageBase> m_storages;
-    QHash<TreeStorageBase, StorageHandler> m_storageHandlers;
+    QHash<StorageBase, StorageHandler> m_storageHandlers;
     std::optional<TaskNode> m_root;
     std::unique_ptr<TaskRuntimeNode> m_runtimeRoot; // Keep me last in order to destruct first
 };
@@ -1225,7 +1225,7 @@ public:
     ~TaskRuntimeContainer()
     {
         for (int i = m_taskContainer.m_storageList.size() - 1; i >= 0; --i) { // iterate in reverse order
-            const TreeStorageBase storage = m_taskContainer.m_storageList[i];
+            const StorageBase storage = m_taskContainer.m_storageList[i];
             const int storageId = m_storageIdList.value(i);
             if (m_callStorageDoneHandlersOnDestruction)
                 m_taskContainer.m_taskTreePrivate->callDoneHandler(storage, storageId);
@@ -1271,7 +1271,7 @@ void ExecutionContextActivator::activateContext(TaskRuntimeContainer *container)
 {
     const TaskContainer &taskContainer = container->m_taskContainer;
     for (int i = 0; i < taskContainer.m_storageList.size(); ++i) {
-        const TreeStorageBase &storage = taskContainer.m_storageList[i];
+        const StorageBase &storage = taskContainer.m_storageList[i];
         auto &threadData = storage.m_storageData->threadData();
         if (threadData.activeStorageId())
             continue; // Storage shadowing: The storage is already active, skipping it...
@@ -1358,14 +1358,14 @@ TaskContainer::TaskContainer(TaskTreePrivate *taskTreePrivate, const GroupItem &
     , m_taskCount(std::accumulate(m_children.cbegin(), m_children.cend(), 0,
                                   [](int r, const TaskNode &n) { return r + n.taskCount(); }))
 {
-    for (const TreeStorageBase &storage : m_storageList)
+    for (const StorageBase &storage : m_storageList)
         m_taskTreePrivate->m_storages << storage;
 }
 
 QList<int> TaskRuntimeContainer::createStorages(const TaskContainer &container)
 {
     QList<int> storageIdList;
-    for (const TreeStorageBase &storage : container.m_storageList) {
+    for (const StorageBase &storage : container.m_storageList) {
         const int storageId = storage.m_storageData->threadData().createStorage();
         storageIdList.append(storageId);
         container.m_taskTreePrivate->callSetupHandler(storage, storageId);
@@ -2631,9 +2631,9 @@ int TaskTree::progressValue() const
     \sa onStorageSetup()
 */
 
-void TaskTree::setupStorageHandler(const TreeStorageBase &storage,
+void TaskTree::setupStorageHandler(const StorageBase &storage,
-                                   TreeStorageBase::StorageVoidHandler setupHandler,
+                                   StorageBase::StorageHandler setupHandler,
-                                   TreeStorageBase::StorageVoidHandler doneHandler)
+                                   StorageBase::StorageHandler doneHandler)
 {
     auto it = d->m_storageHandlers.find(storage);
     if (it == d->m_storageHandlers.end()) {

src/libs/solutions/tasking/tasktree.h

@@ -96,42 +96,44 @@ protected:
     virtual void start() = 0;
 };
 
-class TASKING_EXPORT TreeStorageBase
+class TASKING_EXPORT StorageBase
 {
 public:
-    using StorageConstructor = std::function<void *(void)>;
-    using StorageDestructor = std::function<void(void *)>;
-    using StorageVoidHandler = std::function<void(void *)>;
-
     bool isValid() const;
 
 private:
-    TreeStorageBase(StorageConstructor ctor, StorageDestructor dtor);
+    using StorageConstructor = std::function<void *(void)>;
+    using StorageDestructor = std::function<void(void *)>;
+    using StorageHandler = std::function<void(void *)>;
+
+    StorageBase(StorageConstructor ctor, StorageDestructor dtor);
 
     void *activeStorageVoid() const;
 
-    friend bool operator==(const TreeStorageBase &first, const TreeStorageBase &second)
+    friend bool operator==(const StorageBase &first, const StorageBase &second)
     { return first.m_storageData == second.m_storageData; }
 
-    friend bool operator!=(const TreeStorageBase &first, const TreeStorageBase &second)
+    friend bool operator!=(const StorageBase &first, const StorageBase &second)
     { return first.m_storageData != second.m_storageData; }
 
-    friend size_t qHash(const TreeStorageBase &storage, uint seed = 0)
+    friend size_t qHash(const StorageBase &storage, uint seed = 0)
     { return size_t(storage.m_storageData.get()) ^ seed; }
 
     QSharedPointer<StorageData> m_storageData;
 
     template <typename StorageStruct> friend class TreeStorage;
     friend class ExecutionContextActivator;
+    friend class StorageData;
     friend class TaskRuntimeContainer;
+    friend class TaskTree;
     friend class TaskTreePrivate;
 };
 
 template <typename StorageStruct>
-class TreeStorage final : public TreeStorageBase
+class TreeStorage final : public StorageBase
 {
 public:
-    TreeStorage() : TreeStorageBase(TreeStorage::ctor(), TreeStorage::dtor()) {}
+    TreeStorage() : StorageBase(TreeStorage::ctor(), TreeStorage::dtor()) {}
     StorageStruct &operator*() const noexcept { return *activeStorage(); }
     StorageStruct *operator->() const noexcept { return activeStorage(); }
     StorageStruct *activeStorage() const {
@@ -153,7 +155,7 @@ public:
     // Called when group done, before group's storages are deleted
     using GroupDoneHandler = std::function<DoneResult(DoneWith)>;
 
-    GroupItem(const TreeStorageBase &storage)
+    GroupItem(const StorageBase &storage)
         : m_type(Type::Storage)
         , m_storageList{storage} {}
 
@@ -226,7 +228,7 @@ private:
     Type m_type = Type::Group;
     QList<GroupItem> m_children;
     GroupData m_groupData;
-    QList<TreeStorageBase> m_storageList;
+    QList<StorageBase> m_storageList;
     TaskHandler m_taskHandler;
 };
 
@@ -488,21 +490,21 @@ public:
     int progressMaximum() const { return taskCount(); }
     int progressValue() const; // all finished / skipped / stopped tasks, groups itself excluded
 
-    template <typename StorageStruct, typename StorageHandler>
+    template <typename StorageStruct, typename Handler>
-    void onStorageSetup(const TreeStorage<StorageStruct> &storage, StorageHandler &&handler) {
+    void onStorageSetup(const TreeStorage<StorageStruct> &storage, Handler &&handler) {
-        static_assert(std::is_invocable_v<std::decay_t<StorageHandler>, StorageStruct &>,
+        static_assert(std::is_invocable_v<std::decay_t<Handler>, StorageStruct &>,
                       "Storage setup handler needs to take (Storage &) as an argument. "
                       "The passed handler doesn't fulfill this requirement.");
         setupStorageHandler(storage,
-                            wrapHandler<StorageStruct>(std::forward<StorageHandler>(handler)), {});
+                            wrapHandler<StorageStruct>(std::forward<Handler>(handler)), {});
     }
-    template <typename StorageStruct, typename StorageHandler>
+    template <typename StorageStruct, typename Handler>
-    void onStorageDone(const TreeStorage<StorageStruct> &storage, StorageHandler &&handler) {
+    void onStorageDone(const TreeStorage<StorageStruct> &storage, Handler &&handler) {
-        static_assert(std::is_invocable_v<std::decay_t<StorageHandler>, const StorageStruct &>,
+        static_assert(std::is_invocable_v<std::decay_t<Handler>, const StorageStruct &>,
                       "Storage done handler needs to take (const Storage &) as an argument. "
                       "The passed handler doesn't fulfill this requirement.");
         setupStorageHandler(storage, {},
-                            wrapHandler<const StorageStruct>(std::forward<StorageHandler>(handler)));
+                            wrapHandler<const StorageStruct>(std::forward<Handler>(handler)));
     }
 
 signals:
@@ -511,11 +513,11 @@ signals:
     void progressValueChanged(int value); // updated whenever task finished / skipped / stopped
 
 private:
-    void setupStorageHandler(const TreeStorageBase &storage,
+    void setupStorageHandler(const StorageBase &storage,
-                             TreeStorageBase::StorageVoidHandler setupHandler,
+                             StorageBase::StorageHandler setupHandler,
-                             TreeStorageBase::StorageVoidHandler doneHandler);
+                             StorageBase::StorageHandler doneHandler);
-    template <typename StorageStruct, typename StorageHandler>
+    template <typename StorageStruct, typename Handler>
-    TreeStorageBase::StorageVoidHandler wrapHandler(StorageHandler &&handler) {
+    StorageBase::StorageHandler wrapHandler(Handler &&handler) {
         return [=](void *voidStruct) {
             auto *storageStruct = static_cast<StorageStruct *>(voidStruct);
             std::invoke(handler, *storageStruct);

tests/auto/solutions/tasking/tst_tasking.cpp

@@ -2789,9 +2789,9 @@ void tst_Tasking::storageIO()
 
 void tst_Tasking::storageOperators()
 {
-    TreeStorageBase storage1 = TreeStorage<CustomStorage>();
+    StorageBase storage1 = TreeStorage<CustomStorage>();
-    TreeStorageBase storage2 = TreeStorage<CustomStorage>();
+    StorageBase storage2 = TreeStorage<CustomStorage>();
-    TreeStorageBase storage3 = storage1;
+    StorageBase storage3 = storage1;
 
     QVERIFY(storage1 == storage3);
     QVERIFY(storage1 != storage2);