From 0c9706ada92d838e2c0ef0c300e27567d5ef183f Mon Sep 17 00:00:00 2001
From: Jarek Kobus <jaroslaw.kobus@qt.io>
Date: Tue, 6 Dec 2022 22:41:17 +0100
Subject: [PATCH] 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          |  2 +-
 src/plugins/cppeditor/cppelementevaluator.cpp | 76 ++++++++-----------
 src/plugins/cppeditor/cppelementevaluator.h   | 15 +++-
 3 files changed, 45 insertions(+), 48 deletions(-)

diff --git a/src/libs/cplusplus/LookupContext.cpp b/src/libs/cplusplus/LookupContext.cpp
index dee9e65d21a..14d7f922efa 100644
--- a/src/libs/cplusplus/LookupContext.cpp
+++ b/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()) {
diff --git a/src/plugins/cppeditor/cppelementevaluator.cpp b/src/plugins/cppeditor/cppelementevaluator.cpp
index 04fd19a0105..56677c7b29f 100644
--- a/src/plugins/cppeditor/cppelementevaluator.cpp
+++ b/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*>;
+    ClassOrNamespace *hierarchy = context.lookupType(declaration);
+    if (!hierarchy)
+        return;
+    QSet<ClassOrNamespace *> visited;
+    addBaseHierarchy(futureInterface, context, hierarchy, &visited);
+}
 
-    if (ClassOrNamespace *clazz = context.lookupType(declaration)) {
-        QSet<ClassOrNamespace *> visited;
-
-        QQueue<Data> q;
-        q.enqueue({clazz, this});
-        while (!q.isEmpty()) {
-            if (futureInterface.isCanceled())
-                return;
-            Data current = q.dequeue();
-            clazz = current.first;
-            visited.insert(clazz);
-            const QList<ClassOrNamespace *> &bases = clazz->usings();
-            for (ClassOrNamespace *baseClass : bases) {
-                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()});
-                    }
-                }
+void CppClass::addBaseHierarchy(QFutureInterfaceBase &futureInterface, const LookupContext &context,
+                                ClassOrNamespace *hierarchy, QSet<ClassOrNamespace *> *visited)
+{
+    if (futureInterface.isCanceled())
+        return;
+    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())
+                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
diff --git a/src/plugins/cppeditor/cppelementevaluator.h b/src/plugins/cppeditor/cppelementevaluator.h
index 5e710ea5f85..14f2659b864 100644
--- a/src/plugins/cppeditor/cppelementevaluator.h
+++ b/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