ProjectExplorer: Use unique_ptr to hold targets in projects

Change-Id: I8f793f5e552b65939d6c7c5e0eb42b89f9f45c3d
Reviewed-by: Eike Ziller <eike.ziller@qt.io>

src/plugins/projectexplorer/project.cpp

@@ -47,7 +47,7 @@
 #include <projectexplorer/kitmanager.h>
 #include <projectexplorer/projecttree.h>
 
-#include <utils/algorithm.h>
+#include <utils/pointeralgorithm.h>
 #include <utils/macroexpander.h>
 #include <utils/qtcassert.h>
 
@@ -163,7 +163,7 @@ public:
     std::unique_ptr<Core::IDocument> m_document;
     std::unique_ptr<ProjectNode> m_rootProjectNode;
     std::unique_ptr<ContainerNode> m_containerNode;
-    QList<Target *> m_targets;
+    std::vector<std::unique_ptr<Target>> m_targets;
     Target *m_activeTarget = nullptr;
     EditorConfiguration m_editorConfiguration;
     Core::Context m_projectLanguages;
@@ -181,8 +181,6 @@ public:
 
 ProjectPrivate::~ProjectPrivate()
 {
-    qDeleteAll(m_targets);
-
     // Make sure our root node is null when deleting the actual node
     std::unique_ptr<ProjectNode> oldNode = std::move(m_rootProjectNode);
 }
@@ -241,31 +239,32 @@ bool Project::hasActiveBuildSettings() const
     return activeTarget() && IBuildConfigurationFactory::find(activeTarget());
 }
 
-void Project::addTarget(Target *t)
+void Project::addTarget(std::unique_ptr<Target> &&t)
 {
-    QTC_ASSERT(t && !d->m_targets.contains(t), return);
+    auto pointer = t.get();
+    QTC_ASSERT(t && !Utils::contains(d->m_targets, pointer), return);
     QTC_ASSERT(!target(t->kit()), return);
     Q_ASSERT(t->project() == this);
 
     t->setDefaultDisplayName(t->displayName());
 
     // add it
-    d->m_targets.push_back(t);
+    d->m_targets.emplace_back(std::move(t));
-    connect(t, &Target::addedProjectConfiguration, this, &Project::addedProjectConfiguration);
+    connect(pointer, &Target::addedProjectConfiguration, this, &Project::addedProjectConfiguration);
-    connect(t, &Target::aboutToRemoveProjectConfiguration, this, &Project::aboutToRemoveProjectConfiguration);
+    connect(pointer, &Target::aboutToRemoveProjectConfiguration, this, &Project::aboutToRemoveProjectConfiguration);
-    connect(t, &Target::removedProjectConfiguration, this, &Project::removedProjectConfiguration);
+    connect(pointer, &Target::removedProjectConfiguration, this, &Project::removedProjectConfiguration);
-    connect(t, &Target::activeProjectConfigurationChanged, this, &Project::activeProjectConfigurationChanged);
+    connect(pointer, &Target::activeProjectConfigurationChanged, this, &Project::activeProjectConfigurationChanged);
-    emit addedProjectConfiguration(t);
+    emit addedProjectConfiguration(pointer);
-    emit addedTarget(t);
+    emit addedTarget(pointer);
 
     // check activeTarget:
     if (!activeTarget())
-        setActiveTarget(t);
+        setActiveTarget(pointer);
 }
 
 bool Project::removeTarget(Target *target)
 {
-    QTC_ASSERT(target && d->m_targets.contains(target), return false);
+    QTC_ASSERT(target && Utils::contains(d->m_targets, target), return false);
 
     if (BuildManager::isBuilding(target))
         return false;
@@ -273,25 +272,24 @@ bool Project::removeTarget(Target *target)
     if (target == activeTarget()) {
         if (d->m_targets.size() == 1)
             SessionManager::setActiveTarget(this, nullptr, SetActive::Cascade);
-        else if (d->m_targets.first() == target)
+        else if (d->m_targets.at(0).get() == target)
-            SessionManager::setActiveTarget(this, d->m_targets.at(1), SetActive::Cascade);
+            SessionManager::setActiveTarget(this, d->m_targets.at(1).get(), SetActive::Cascade);
         else
-            SessionManager::setActiveTarget(this, d->m_targets.at(0), SetActive::Cascade);
+            SessionManager::setActiveTarget(this, d->m_targets.at(0).get(), SetActive::Cascade);
     }
 
     emit aboutToRemoveProjectConfiguration(target);
     emit aboutToRemoveTarget(target);
-    d->m_targets.removeOne(target);
+    auto keep = Utils::take(d->m_targets, target);
     emit removedTarget(target);
     emit removedProjectConfiguration(target);
 
-    delete target;
     return true;
 }
 
 QList<Target *> Project::targets() const
 {
-    return d->m_targets;
+    return Utils::toRawPointer<QList>(d->m_targets);
 }
 
 Target *Project::activeTarget() const
@@ -301,8 +299,8 @@ Target *Project::activeTarget() const
 
 void Project::setActiveTarget(Target *target)
 {
-    if ((!target && !d->m_targets.isEmpty()) ||
+    if ((!target && d->m_targets.size() > 0) ||
-        (target && d->m_targets.contains(target) && d->m_activeTarget != target)) {
+        (target && Utils::contains(d->m_targets, target) && d->m_activeTarget != target)) {
         d->m_activeTarget = target;
         emit activeProjectConfigurationChanged(d->m_activeTarget);
         emit activeTargetChanged(d->m_activeTarget);
@@ -327,16 +325,14 @@ QList<Task> Project::projectIssues(const Kit *k) const
     return {};
 }
 
-Target *Project::createTarget(Kit *k)
+std::unique_ptr<Target> Project::createTarget(Kit *k)
 {
     if (!k || target(k))
         return nullptr;
 
-    auto t = new Target(this, k);
+    auto t = std::make_unique<Target>(this, k, Target::_constructor_tag{});
-    if (!setupTarget(t)) {
+    if (!setupTarget(t.get()))
-        delete t;
+        return {};
-        return nullptr;
-    }
     return t;
 }
 
@@ -529,7 +525,7 @@ void Project::handleSubTreeChanged(FolderNode *node)
     emit fileListChanged();
 }
 
-Target *Project::restoreTarget(const QVariantMap &data)
+std::unique_ptr<Target> Project::restoreTarget(const QVariantMap &data)
 {
     Core::Id id = idFromMap(data);
     if (target(id)) {
@@ -544,11 +540,9 @@ Target *Project::restoreTarget(const QVariantMap &data)
         return nullptr;
     }
 
-    auto t = new Target(this, k);
+    auto t = std::make_unique<Target>(this, k, Target::_constructor_tag{});
-    if (!t->fromMap(data)) {
+    if (!t->fromMap(data))
-        delete t;
+        return {};
-        return nullptr;
-    }
 
     return t;
 }
@@ -698,16 +692,11 @@ void Project::createTargetFromMap(const QVariantMap &map, int index)
         return;
     QVariantMap targetMap = map.value(key).toMap();
 
-    Target *t = restoreTarget(targetMap);
+    std::unique_ptr<Target> t = restoreTarget(targetMap);
-    if (!t)
+    if (!t || (t->runConfigurations().isEmpty() && t->buildConfigurations().isEmpty()))
         return;
-    if (t->runConfigurations().isEmpty() && t->buildConfigurations().isEmpty()) {
-        delete t;
-        return;
-    }
 
-
+    addTarget(std::move(t));
-    addTarget(t);
 }
 
 EditorConfiguration *Project::editorConfiguration() const
@@ -799,7 +788,7 @@ void Project::setNamedSettings(const QString &name, const QVariant &value)
 
 bool Project::needsConfiguration() const
 {
-    return d->m_targets.isEmpty();
+    return d->m_targets.size() == 0;
 }
 
 bool Project::needsBuildConfigurations() const
@@ -822,20 +811,20 @@ bool Project::knowsAllBuildExecutables() const
     return true;
 }
 
-void Project::setup(QList<const BuildInfo *> infoList)
+void Project::setup(const QList<const BuildInfo *> &infoList)
 {
-    QList<Target *> toRegister;
+    std::vector<std::unique_ptr<Target>> toRegister;
-    foreach (const BuildInfo *info, infoList) {
+    for (const BuildInfo *info : infoList) {
         Kit *k = KitManager::kit(info->kitId);
         if (!k)
             continue;
         Target *t = target(k);
-        if (!t) {
+        if (!t)
             t = Utils::findOrDefault(toRegister, Utils::equal(&Target::kit, k));
-        }
         if (!t) {
-            t = new Target(this, k);
+            auto newTarget = std::make_unique<Target>(this, k, Target::_constructor_tag{});
-            toRegister << t;
+            t = newTarget.get();
+            toRegister.emplace_back(std::move(newTarget));
         }
 
         if (!info->factory())
@@ -846,10 +835,10 @@ void Project::setup(QList<const BuildInfo *> infoList)
             continue;
         t->addBuildConfiguration(bc);
     }
-    foreach (Target *t, toRegister) {
+    for (std::unique_ptr<Target> &t : toRegister) {
         t->updateDefaultDeployConfigurations();
         t->updateDefaultRunConfigurations();
-        addTarget(t);
+        addTarget(std::move(t));
     }
 }
 

src/plugins/projectexplorer/project.h

@@ -114,7 +114,7 @@ public:
     EditorConfiguration *editorConfiguration() const;
 
     // Target:
-    void addTarget(Target *target);
+    void addTarget(std::unique_ptr<Target> &&target);
     bool removeTarget(Target *target);
 
     QList<Target *> targets() const;
@@ -124,9 +124,9 @@ public:
     Target *target(Kit *k) const;
     virtual QList<Task> projectIssues(const Kit *k) const;
 
-    Target *createTarget(Kit *k);
+    std::unique_ptr<Target> createTarget(Kit *k);
     static bool copySteps(Target *sourceTarget, Target *newTarget);
-    Target *restoreTarget(const QVariantMap &data);
+    std::unique_ptr<Target> restoreTarget(const QVariantMap &data);
 
     void saveSettings();
     enum class RestoreResult { Ok, Error, UserAbort };
@@ -163,7 +163,7 @@ public:
     // of configuration.
     virtual bool knowsAllBuildExecutables() const;
 
-    void setup(QList<const BuildInfo *> infoList);
+    void setup(const QList<const BuildInfo *> &infoList);
     Utils::MacroExpander *macroExpander() const;
 
     bool isParsing() const;

src/plugins/projectexplorer/projectwindow.cpp

@@ -554,9 +554,10 @@ public:
         for (BuildInfo *info : toImport) {
             Target *target = project->target(info->kitId);
             if (!target) {
-                target = project->createTarget(KitManager::kit(info->kitId));
+                std::unique_ptr<Target> newTarget = project->createTarget(KitManager::kit(info->kitId));
-                if (target)
+                target = newTarget.get();
-                    project->addTarget(target);
+                if (newTarget)
+                    project->addTarget(std::move(newTarget));
             }
             if (target) {
                 projectImporter->makePersistent(target->kit());

src/plugins/projectexplorer/target.cpp

@@ -92,11 +92,11 @@ public:
     QIcon m_overlayIcon;
 
     QList<BuildConfiguration *> m_buildConfigurations;
-    BuildConfiguration *m_activeBuildConfiguration = 0;
+    BuildConfiguration *m_activeBuildConfiguration = nullptr;
     QList<DeployConfiguration *> m_deployConfigurations;
-    DeployConfiguration *m_activeDeployConfiguration = 0;
+    DeployConfiguration *m_activeDeployConfiguration = nullptr;
     QList<RunConfiguration *> m_runConfigurations;
-    RunConfiguration* m_activeRunConfiguration = 0;
+    RunConfiguration* m_activeRunConfiguration = nullptr;
     DeploymentData m_deploymentData;
     BuildTargetInfoList m_appTargets;
     QVariantMap m_pluginSettings;
@@ -108,7 +108,7 @@ TargetPrivate::TargetPrivate(Kit *k) :
     m_kit(k)
 { }
 
-Target::Target(Project *project, Kit *k) :
+Target::Target(Project *project, Kit *k, _constructor_tag) :
     ProjectConfiguration(project, k->id()),
     d(new TargetPrivate(k))
 {

src/plugins/projectexplorer/target.h

@@ -54,7 +54,10 @@ class PROJECTEXPLORER_EXPORT Target : public ProjectConfiguration
     friend class SessionManager; // for setActiveBuild and setActiveDeployConfiguration
     Q_OBJECT
 
+    struct _constructor_tag { explicit _constructor_tag() = default; };
+
 public:
+    Target(Project *parent, Kit *k, _constructor_tag);
     ~Target() override;
 
     Project *project() const override;
@@ -164,7 +167,6 @@ signals:
     void applicationTargetsChanged();
 
 private:
-    Target(Project *parent, Kit *k);
     void setEnabled(bool);
 
     bool fromMap(const QVariantMap &map) override;

src/plugins/qmlprofiler/tests/qmlprofilerdetailsrewriter_test.cpp

@@ -202,9 +202,9 @@ void QmlProfilerDetailsRewriterTest::seedRewriter()
 
     DummyProject *project = new DummyProject(Utils::FileName::fromString(filename));
     ProjectExplorer::SessionManager::addProject(project);
-    ProjectExplorer::Target *target = project->createTarget(kit);
+    std::unique_ptr<ProjectExplorer::Target> target = project->createTarget(kit);
 
-    m_rewriter.populateFileFinder(target);
+    m_rewriter.populateFileFinder(target.get());
     ProjectExplorer::SessionManager::removeProject(project);
     ProjectExplorer::KitManager::deleteKit(kit);
 }

src/plugins/qmlprojectmanager/qmlprojectrunconfiguration.h

@@ -27,6 +27,7 @@
 
 #include "qmlprojectmanager_global.h"
 
+#include <projectexplorer/runconfiguration.h>
 #include <projectexplorer/runnables.h>
 
 namespace Core { class IEditor; }