Get rid of some raw pointer members ...

... where they actually point to objects held in
shared pointers elsewhere. In such cases, use
shared pointers when possible and weak pointers
when necessary. Beside better self-documentatiom,
this also enables us to get rid of some ugly loops
that were needed to get from the raw pointer value
back to the shared equivalent.
(We ignore members held in the Artifact class for now
as to not complicate the patch.)

Change-Id: Icc47c790835c4d144ddab5c1de65fc2c5de5e4c7
Reviewed-by: Joerg Bornemann <joerg.bornemann@digia.com>

8 files changed, 84 insertions, 59 deletions

diff --git a/src/lib/buildgraph/buildgraph.cpp b/src/lib/buildgraph/buildgraph.cpp
index 02d8561fe..a4862133e 100644
--- a/src/lib/buildgraph/buildgraph.cpp
+++ b/src/lib/buildgraph/buildgraph.cpp
@@ -59,7 +59,6 @@ namespace qbs {
 namespace Internal {
 
 BuildProduct::BuildProduct()
-    : project(0)
 {
 }
 
@@ -157,12 +156,12 @@ void BuildGraph::insert(BuildProduct *product, Artifact *n) const
     Q_ASSERT(!n->filePath().isEmpty());
     Q_ASSERT(!product->artifacts.contains(n));
 #ifdef QT_DEBUG
-    foreach (BuildProduct::Ptr otherProduct, product->project->buildProducts()) {
+    foreach (const BuildProduct::Ptr &otherProduct, product->project->buildProducts()) {
         if (otherProduct->lookupArtifact(n->filePath())) {
             if (n->artifactType == Artifact::Generated) {
                 QString pl;
                 pl.append(QString("  - %1 \n").arg(product->rProduct->name));
-                foreach (BuildProduct::Ptr p, product->project->buildProducts()) {
+                foreach (const BuildProduct::Ptr &p, product->project->buildProducts()) {
                     if (p->lookupArtifact(n->filePath())) {
                         pl.append(QString("  - %1 \n").arg(p->rProduct->name));
                     }
@@ -195,7 +194,7 @@ void BuildGraph::setupScriptEngineForProduct(ScriptEngine *engine,
 
     if (lastSetupProject != product->project) {
         engine->setProperty("lastSetupProject",
-                QVariant(reinterpret_cast<qulonglong>(product->project)));
+                QVariant(reinterpret_cast<qulonglong>(product->project.data())));
         QScriptValue projectScriptValue;
         projectScriptValue = engine->newObject();
         projectScriptValue.setProperty("filePath", product->project->qbsFile);
@@ -487,14 +486,14 @@ BuildProject::Ptr BuildGraph::resolveProject(ResolvedProject::Ptr rProject)
     project->setResolvedProject(rProject);
     if (m_progressObserver)
         m_progressObserver->initialize(Tr::tr("Resolving project"), rProject->products.count());
-    foreach (ResolvedProduct::Ptr rProduct, rProject->products) {
-        resolveProduct(project.data(), rProduct);
-    }
+    foreach (ResolvedProduct::Ptr rProduct, rProject->products)
+        resolveProduct(project, rProduct);
     CycleDetector().visitProject(project);
     return project;
 }
 
-BuildProduct::Ptr BuildGraph::resolveProduct(BuildProject *project, ResolvedProduct::Ptr rProduct)
+BuildProduct::Ptr BuildGraph::resolveProduct(const BuildProject::Ptr &project,
+                                             const ResolvedProduct::Ptr &rProduct)
 {
     BuildProduct::Ptr product = m_productCache.value(rProduct);
     if (product)
@@ -517,13 +516,13 @@ BuildProduct::Ptr BuildGraph::resolveProduct(BuildProject *project, ResolvedProd
             throw Error(Tr::tr("circular using"));
         }
         BuildProduct::Ptr referencedProduct = resolveProduct(project, t2);
-        product->dependencies.append(referencedProduct.data());
+        product->dependencies.append(referencedProduct);
     }
 
     //add qbsFile artifact
     Artifact *qbsFileArtifact = product->lookupArtifact(rProduct->qbsFile);
     if (!qbsFileArtifact) {
-        qbsFileArtifact = new Artifact(project);
+        qbsFileArtifact = new Artifact(project.data());
         qbsFileArtifact->artifactType = Artifact::SourceFile;
         qbsFileArtifact->setFilePath(rProduct->qbsFile);
         qbsFileArtifact->properties = rProduct->properties;
@@ -819,7 +818,7 @@ void BuildProduct::load(PersistentPool &pool)
     dependencies.clear();
     dependencies.reserve(i);
     for (; --i >= 0;)
-        dependencies += pool.idLoadS<BuildProduct>().data();
+        dependencies += pool.idLoadS<BuildProduct>();
 }
 
 void BuildProduct::store(PersistentPool &pool) const
@@ -907,6 +906,8 @@ void BuildProject::restoreBuildGraph(const QString &projectFilePath, BuildGraph
     project = BuildProject::Ptr(new BuildProject(bg));
     TimedActivityLogger loadLogger(QLatin1String("Loading build graph"), QLatin1String("[BG] "));
     project->load(pool);
+    foreach (const BuildProduct::Ptr &bp, project->buildProducts())
+        bp->project = project;
     project->resolvedProject()->qbsFile = projectFilePath;
     project->resolvedProject()->setBuildConfiguration(pool.headData().projectConfig);
     loadResult->loadedProject = project;
@@ -1019,12 +1020,13 @@ void BuildProject::store() const
 void BuildProject::load(PersistentPool &pool)
 {
     m_resolvedProject = pool.idLoadS<ResolvedProject>();
+    foreach (const ResolvedProduct::Ptr &product, m_resolvedProject->products)
+        product->project = m_resolvedProject;
 
     int count;
     pool.stream() >> count;
     for (; --count >= 0;) {
         BuildProduct::Ptr product = pool.idLoadS<BuildProduct>();
-        product->project = this;
         foreach (Artifact *artifact, product->artifacts) {
             artifact->project = this;
             insertIntoArtifactLookupTable(artifact);
@@ -1261,7 +1263,7 @@ void RulesApplicator::doApply(const QSet<Artifact *> &inputArtifacts)
     ArtifactList usingArtifacts;
     if (!m_rule->usings.isEmpty()) {
         const QSet<QString> usingsFileTags = m_rule->usings.toSet();
-        foreach (BuildProduct * const dep, m_buildProduct->dependencies) {
+        foreach (const BuildProduct::Ptr &dep, m_buildProduct->dependencies) {
             QList<Artifact *> artifactsToCheck;
             foreach (Artifact *targetArtifact, dep->targetArtifacts)
                 artifactsToCheck += targetArtifact->transformer->outputs.toList();
diff --git a/src/lib/buildgraph/buildgraph.h b/src/lib/buildgraph/buildgraph.h
index 104e48c11..998c48153 100644
--- a/src/lib/buildgraph/buildgraph.h
+++ b/src/lib/buildgraph/buildgraph.h
@@ -35,6 +35,7 @@
 #include <language/language.h>
 #include <tools/error.h>
 #include <tools/persistentobject.h>
+#include <tools/weakpointer.h>
 
 #include <QDir>
 #include <QScriptValue>
@@ -69,10 +70,10 @@ public:
     Artifact *lookupArtifact(const QString &dirPath, const QString &fileName) const;
     Artifact *lookupArtifact(const QString &filePath) const;
 
-    BuildProject *project;
+    WeakPointer<BuildProject> project;
     ResolvedProduct::Ptr rProduct;
     QSet<Artifact *> targetArtifacts;
-    QList<BuildProduct *> dependencies;
+    QList<BuildProduct::Ptr> dependencies;
     ArtifactList artifacts;
 
 private:
@@ -201,7 +202,8 @@ public:
 private:
     void initEngine();
     void cleanupEngine();
-    BuildProduct::Ptr resolveProduct(BuildProject *, ResolvedProduct::Ptr);
+    BuildProduct::Ptr resolveProduct(const BuildProject::Ptr &project,
+                                     const ResolvedProduct::Ptr &rProduct);
     Artifact *createArtifact(BuildProduct::Ptr product, SourceArtifact::ConstPtr sourceArtifact);
     void updateNodeThatMustGetNewTransformer(Artifact *artifact);
 
diff --git a/src/lib/language/language.cpp b/src/lib/language/language.cpp
index caacd0f20..60d25d670 100644
--- a/src/lib/language/language.cpp
+++ b/src/lib/language/language.cpp
@@ -347,7 +347,6 @@ void Rule::store(PersistentPool &pool) const
 }
 
 ResolvedProduct::ResolvedProduct()
-    : project(0)
 {
 }
 
@@ -586,7 +585,6 @@ void ResolvedProject::load(PersistentPool &pool)
     products.reserve(count);
     for (; --count >= 0;) {
         ResolvedProduct::Ptr rProduct = pool.idLoadS<ResolvedProduct>();
-        rProduct->project = this;
         products.append(rProduct);
     }
 }
diff --git a/src/lib/language/language.h b/src/lib/language/language.h
index 6b67da833..8fa3efad8 100644
--- a/src/lib/language/language.h
+++ b/src/lib/language/language.h
@@ -32,9 +32,10 @@
 
 #include "jsimports.h"
 #include <tools/codelocation.h>
+#include <tools/fileinfo.h>
 #include <tools/persistentobject.h>
 #include <tools/settings.h>
-#include <tools/fileinfo.h>
+#include <tools/weakpointer.h>
 
 #include <QDataStream>
 #include <QMutex>
@@ -321,7 +322,7 @@ public:
     QString sourceDirectory;
     QString destinationDirectory;
     QString qbsFile;
-    ResolvedProject *project;
+    WeakPointer<ResolvedProject> project;
     PropertyMap::Ptr properties;
     QSet<Rule::Ptr> rules;
     QSet<ResolvedProduct::Ptr> dependencies;
diff --git a/src/lib/language/loader.cpp b/src/lib/language/loader.cpp
index 87bf1deb5..d457719c2 100644
--- a/src/lib/language/loader.cpp
+++ b/src/lib/language/loader.cpp
@@ -2925,7 +2925,7 @@ void Loader::resolveTopLevel(const ResolvedProject::Ptr &rproject,
     const ResolvedProduct::Ptr rproduct = ResolvedProduct::create();
     rproduct->qbsFile = projectFileName;
     rproduct->sourceDirectory = QFileInfo(projectFileName).absolutePath();
-    rproduct->project = rproject.data();
+    rproduct->project = rproject;
 
     ProductData productData;
     productData.product = evaluationObject;
diff --git a/src/lib/language/qbsengine.cpp b/src/lib/language/qbsengine.cpp
index 0d75cfc29..13d54364b 100644
--- a/src/lib/language/qbsengine.cpp
+++ b/src/lib/language/qbsengine.cpp
@@ -249,17 +249,7 @@ void QbsEngine::cleanProducts(const QList<Product> &products, const BuildOptions
         const ResolvedProduct::ConstPtr rProduct = d->publicObjectsMap.product(product.id());
         if (!rProduct)
             throw Error(Tr::tr("Cleaning up failed: Product not found."));
-
-        // TODO: Use of weak pointers will eliminate this loop.
-        ResolvedProject::ConstPtr rProject;
-        foreach (const ResolvedProject::ConstPtr &p, d->resolvedProjects) {
-            if (p == rProduct->project) {
-                rProject = p;
-                break;
-            }
-        }
-
-        Q_ASSERT(rProject);
+        const ResolvedProject::ConstPtr rProject = rProduct->project.toStrongRef();
         const BuildProject::ConstPtr bProject = d->setupBuildProject(rProject);
         foreach (const BuildProduct::ConstPtr &bProduct, bProject->buildProducts()) {
             if (bProduct->rProduct == rProduct) {
@@ -431,19 +421,11 @@ void QbsEngine::QbsEnginePrivate::buildProducts(const QList<ResolvedProduct::Con
         const BuildOptions &buildOptions, bool needsDepencencyResolving)
 {
     // Make sure all products are set up first.
-    QSet<const ResolvedProject *> rProjects;
+    QSet<ResolvedProject::ConstPtr> rProjects;
     foreach (const ResolvedProduct::ConstPtr &product, products)
-        rProjects << product->project;
-    foreach (const ResolvedProject *rproject, rProjects) {
-
-        // TODO: This awful loop is there because we don't use weak pointers. Change that.
-        foreach (const ResolvedProject::ConstPtr &rProjectSp, resolvedProjects) {
-            if (rproject == rProjectSp) {
-                setupBuildProject(rProjectSp);
-                break;
-            }
-        }
-    }
+        rProjects << product->project.toStrongRef();
+    foreach (const ResolvedProject::ConstPtr &rProject, rProjects)
+        setupBuildProject(rProject);
 
     // Gather build products.
     QList<BuildProduct::Ptr> productsToBuild;
@@ -459,20 +441,8 @@ void QbsEngine::QbsEnginePrivate::buildProducts(const QList<ResolvedProduct::Con
     if (needsDepencencyResolving) {
         for (int i = 0; i < productsToBuild.count(); ++i) {
             const BuildProduct::ConstPtr &product = productsToBuild.at(i);
-            foreach (BuildProduct * const dependency, product->dependencies) {
-
-                // TODO: This awful loop is there because we don't use weak pointers. Change that.
-                BuildProduct::Ptr dependencySP;
-                foreach (const BuildProduct::Ptr &bp, productsToBuild) {
-                    if (bp == dependency) {
-                        dependencySP = bp;
-                        break;
-                    }
-                }
-
-                if (dependencySP)
-                    productsToBuild << dependencySP;
-            }
+            foreach (const BuildProduct::Ptr &dependency, product->dependencies)
+                productsToBuild << dependency;
         }
     }
 
diff --git a/src/lib/tools/tools.pri b/src/lib/tools/tools.pri
index 4ce2e0add..8508f4ae7 100644
--- a/src/lib/tools/tools.pri
+++ b/src/lib/tools/tools.pri
@@ -14,7 +14,8 @@ HEADERS += \
     $$PWD/hostosinfo.h \
     $$PWD/buildoptions.h \
     $$PWD/runenvironment.h \
-    $$PWD/persistentobject.h
+    $$PWD/persistentobject.h \
+    $$PWD/weakpointer.h
 
 SOURCES += \
     $$PWD/error.cpp \
diff --git a/src/lib/tools/weakpointer.h b/src/lib/tools/weakpointer.h
new file mode 100644
index 000000000..f50f3d78f
--- /dev/null
+++ b/src/lib/tools/weakpointer.h
@@ -0,0 +1,51 @@
+/****************************************************************************
+**
+** Copyright (C) 2012 Digia Plc and/or its subsidiary(-ies).
+** Contact: http://www.qt-project.org/legal
+**
+** This file is part of the Qt Build Suite.
+**
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and Digia.  For licensing terms and
+** conditions see http://qt.digia.com/licensing.  For further information
+** use the contact form at http://qt.digia.com/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU Lesser General Public License version 2.1 requirements
+** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** In addition, as a special exception, Digia gives you certain additional
+** rights.  These rights are described in the Digia Qt LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+****************************************************************************/
+#ifndef WEAKPOINTER_H
+#define WEAKPOINTER_H
+
+#include <QWeakPointer>
+
+namespace qbs {
+namespace Internal {
+
+template<typename T> class WeakPointer : public QWeakPointer<T>
+{
+public:
+    WeakPointer() : QWeakPointer<T>() {}
+    WeakPointer(const QSharedPointer<T> &sharedPointer) : QWeakPointer<T>(sharedPointer) {}
+
+    operator T*() const { return QWeakPointer<T>::data(); }
+    T *operator->() const { return QWeakPointer<T>::data(); }
+    T operator*() const { return *QWeakPointer<T>::data(); }
+};
+
+} // namespace Internal
+} // namespace qbs
+
+#endif // WEAKPOINTER_H