CppTypeHierarchy: Don't keep pointers to temp objects

When building type hierarchy the pointers to list
elements were put into queue. Later, the list was
modified and it was possible that list detaches
causing stored pointers to be invalid.

Simplify building type hierarchy by adding helper recursive
methods.

Fixes: QTCREATORBUG-28529
Change-Id: I240513fc097536d2175e2242766127b92aaa6a82
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/libs/cplusplus/LookupContext.cpp

@@ -433,7 +433,7 @@ QList<LookupItem> LookupContext::lookup(const Name *name, Scope *scope) const
 {
     QList<LookupItem> candidates;
 
-    if (! name)
+    if (!name)
         return candidates;
 
     for (; scope; scope = scope->enclosingScope()) {

src/plugins/cppeditor/cppelementevaluator.cpp

@@ -142,32 +142,30 @@ CppClass *CppClass::toCppClass()
 void CppClass::lookupBases(QFutureInterfaceBase &futureInterface,
                            Symbol *declaration, const LookupContext &context)
 {
-    using Data = QPair<ClassOrNamespace*, CppClass*>;
-
-    if (ClassOrNamespace *clazz = context.lookupType(declaration)) {
+    ClassOrNamespace *hierarchy = context.lookupType(declaration);
+    if (!hierarchy)
+        return;
     QSet<ClassOrNamespace *> visited;
+    addBaseHierarchy(futureInterface, context, hierarchy, &visited);
+}
 
-        QQueue<Data> q;
-        q.enqueue({clazz, this});
-        while (!q.isEmpty()) {
+void CppClass::addBaseHierarchy(QFutureInterfaceBase &futureInterface, const LookupContext &context,
+                                ClassOrNamespace *hierarchy, QSet<ClassOrNamespace *> *visited)
+{
     if (futureInterface.isCanceled())
         return;
-            Data current = q.dequeue();
-            clazz = current.first;
-            visited.insert(clazz);
-            const QList<ClassOrNamespace *> &bases = clazz->usings();
-            for (ClassOrNamespace *baseClass : bases) {
+    visited->insert(hierarchy);
+    const QList<ClassOrNamespace *> &baseClasses = hierarchy->usings();
+    for (ClassOrNamespace *baseClass : baseClasses) {
         const QList<Symbol *> &symbols = baseClass->symbols();
         for (Symbol *symbol : symbols) {
-                    if (symbol->asClass() && (
-                        clazz = context.lookupType(symbol)) &&
-                        !visited.contains(clazz)) {
-                        CppClass baseCppClass(symbol);
-                        CppClass *cppClass = current.second;
-                        cppClass->bases.append(baseCppClass);
-                        q.enqueue({clazz, &cppClass->bases.last()});
-                    }
-                }
+            if (!symbol->asClass())
+                continue;
+            ClassOrNamespace *baseHierarchy = context.lookupType(symbol);
+            if (baseHierarchy && !visited->contains(baseHierarchy)) {
+                CppClass classSymbol(symbol);
+                classSymbol.addBaseHierarchy(futureInterface, context, baseHierarchy, visited);
+                bases.append(classSymbol);
             }
         }
     }
@@ -176,28 +174,20 @@ void CppClass::lookupBases(QFutureInterfaceBase &futureInterface,
 void CppClass::lookupDerived(QFutureInterfaceBase &futureInterface,
                              Symbol *declaration, const Snapshot &snapshot)
 {
-    using Data = QPair<CppClass*, TypeHierarchy>;
-
     snapshot.updateDependencyTable(futureInterface);
     if (futureInterface.isCanceled())
         return;
-    const TypeHierarchy &completeHierarchy
-            = TypeHierarchyBuilder::buildDerivedTypeHierarchy(futureInterface, declaration, snapshot);
+    addDerivedHierarchy(TypeHierarchyBuilder::buildDerivedTypeHierarchy(
+                        futureInterface, declaration, snapshot));
+}
 
-    QQueue<Data> q;
-    q.enqueue({this, completeHierarchy});
-    while (!q.isEmpty()) {
-        if (futureInterface.isCanceled())
-            return;
-        const Data &current = q.dequeue();
-        CppClass *clazz = current.first;
-        const TypeHierarchy &classHierarchy = current.second;
-        const QList<TypeHierarchy> hierarchy = classHierarchy.hierarchy();
-        for (const TypeHierarchy &derivedHierarchy : hierarchy) {
-            clazz->derived.append(CppClass(derivedHierarchy.symbol()));
-            q.enqueue({&clazz->derived.last(), derivedHierarchy});
-        }
-    }
+void CppClass::addDerivedHierarchy(const TypeHierarchy &hierarchy)
+{
+    CppClass classSymbol(hierarchy.symbol());
+    const QList<TypeHierarchy> derivedHierarchies = hierarchy.hierarchy();
+    for (const TypeHierarchy &derivedHierarchy : derivedHierarchies)
+        classSymbol.addDerivedHierarchy(derivedHierarchy);
+    derived.append(classSymbol);
 }
 
 class CppFunction : public CppDeclarableElement

src/plugins/cppeditor/cppelementevaluator.h

@@ -3,14 +3,14 @@
 
 #pragma once
 
+#include "typehierarchybuilder.h"
+
 #include <coreplugin/helpitem.h>
+#include <cplusplus/CppDocument.h>
 #include <texteditor/texteditor.h>
 #include <utils/utilsicons.h>
 
-#include <cplusplus/CppDocument.h>
-
 #include <QFuture>
-#include <QIcon>
 #include <QSharedPointer>
 #include <QString>
 #include <QStringList>
@@ -19,6 +19,7 @@
 #include <functional>
 
 namespace CPlusPlus {
+class ClassOrNamespace;
 class LookupItem;
 class LookupContext;
 }
@@ -96,9 +97,15 @@ public:
     void lookupDerived(QFutureInterfaceBase &futureInterface,
                        CPlusPlus::Symbol *declaration, const CPlusPlus::Snapshot &snapshot);
 
-public:
     QList<CppClass> bases;
     QList<CppClass> derived;
+
+private:
+    void addBaseHierarchy(QFutureInterfaceBase &futureInterface,
+                          const CPlusPlus::LookupContext &context,
+                          CPlusPlus::ClassOrNamespace *hierarchy,
+                          QSet<CPlusPlus::ClassOrNamespace *> *visited);
+    void addDerivedHierarchy(const TypeHierarchy &hierarchy);
 };
 
 } // namespace Internal