Debugger: Always sort local variables alphabetically.

It was there for GDB at a time, but never for the other backends.
Fix the GDB regression and make the sorting consistent across
backends.

Task-number: QTCREATORBUG-15296
Change-Id: If728c65f0c8ca4a8378c7cf5e53f1dadbfb72b29
Reviewed-by: Christian Stenger <christian.stenger@theqtcompany.com>

share/qtcreator/debugger/gdbbridge.py

@@ -435,7 +435,6 @@ class Dumper(DumperBase):
                 # Don't bother. It's only supplementary information anyway.
                 pass
 
-        locals.sort(key = lambda item: item.name)
         for item in locals:
             value = self.downcast(item.value) if self.useDynamicType else item.value
             with OutputSafer(self):

src/plugins/debugger/debuggerengine.cpp

@@ -168,12 +168,6 @@ enum RemoteSetupState { RemoteSetupNone, RemoteSetupRequested,
                         RemoteSetupSucceeded, RemoteSetupFailed,
                         RemoteSetupCancelled };
 
-struct TypeInfo
-{
-    TypeInfo(uint s = 0) : size(s) {}
-    uint size;
-};
-
 class DebuggerEnginePrivate : public QObject
 {
     Q_OBJECT
@@ -342,7 +336,6 @@ public:
     bool m_isStateDebugging;
 
     Utils::FileInProjectFinder m_fileFinder;
-    QHash<QByteArray, TypeInfo> m_typeInfoCache;
     QByteArray m_qtNamespace;
 };
 
@@ -1987,33 +1980,13 @@ void DebuggerEngine::updateLocalsView(const GdbMi &all)
 {
     WatchHandler *handler = watchHandler();
 
-    const bool partial = all["partial"].toInt();
-
     const GdbMi typeInfo = all["typeinfo"];
-    if (typeInfo.type() == GdbMi::List) {
-        foreach (const GdbMi &s, typeInfo.children()) {
-            const GdbMi name = s["name"];
-            const GdbMi size = s["size"];
-            if (name.isValid() && size.isValid())
-                d->m_typeInfoCache.insert(QByteArray::fromHex(name.data()),
-                                       TypeInfo(size.data().toUInt()));
-        }
-    }
+    handler->recordTypeInfo(typeInfo);
 
-    const bool sortStructMembers = boolSetting(SortStructMembers);
+    const GdbMi data = all["data"];
+    handler->insertItems(data);
 
-    GdbMi data = all["data"];
-    foreach (const GdbMi &child, data.children()) {
-        WatchItem *item = new WatchItem;
-        item->parse(child, sortStructMembers);
-        const TypeInfo ti = d->m_typeInfoCache.value(item->type);
-        if (ti.size && !item->size)
-            item->size = ti.size;
-
-        handler->insertItem(item);
-    }
-
-    GdbMi ns = all["qtnamespace"];
+    const GdbMi ns = all["qtnamespace"];
     if (ns.isValid()) {
         setQtNamespace(ns.data());
         showMessage(_("FOUND NAMESPACED QT: " + ns.data()));
@@ -2026,6 +1999,7 @@ void DebuggerEngine::updateLocalsView(const GdbMi &all)
 
     DebuggerToolTipManager::updateEngine(this);
 
+    const bool partial = all["partial"].toInt();
     if (!partial)
         emit stackFrameCompleted();
 }

src/plugins/debugger/pdb/pdbengine.cpp

@@ -513,14 +513,7 @@ void PdbEngine::refreshLocals(const GdbMi &vars)
 {
     WatchHandler *handler = watchHandler();
     handler->resetValueCache();
-
-    const bool sortStructMembers = boolSetting(SortStructMembers);
-    foreach (const GdbMi &child, vars.children()) {
-        WatchItem *item = new WatchItem;
-        item->parse(child, sortStructMembers);
-        handler->insertItem(item);
-    }
-
+    handler->insertItems(vars);
     handler->notifyUpdateFinished();
 
     DebuggerToolTipManager::updateEngine(this);

src/plugins/debugger/watchhandler.cpp

@@ -80,6 +80,12 @@ static int theUnprintableBase = -1;
 const char INameProperty[] = "INameProperty";
 const char KeyProperty[] = "KeyProperty";
 
+struct TypeInfo
+{
+    TypeInfo(uint s = 0) : size(s) {}
+    uint size;
+};
+
 static const WatchModel *watchModel(const WatchItem *item)
 {
     return reinterpret_cast<const WatchModel *>(item->model());
@@ -345,7 +351,6 @@ public:
     WatchItem *findItem(const QByteArray &iname) const;
     const WatchItem *watchItem(const QModelIndex &idx) const;
 
-    void insertItem(WatchItem *item);
     void reexpandItems();
 
     void showEditValue(const WatchItem *item);
@@ -373,6 +378,7 @@ public:
     QSet<QByteArray> m_expandedINames;
     QTimer m_requestUpdateTimer;
 
+    QHash<QByteArray, TypeInfo> m_reportedTypeInfo;
     QHash<QString, DisplayFormats> m_reportedTypeFormats; // Type name -> Dumper Formats
     QHash<QByteArray, QString> m_valueCache;
 };
@@ -1249,23 +1255,46 @@ void WatchHandler::cleanup()
     m_model->m_separatedView->hide();
 }
 
-void WatchHandler::insertItem(WatchItem *item)
+static bool sortByName(const TreeItem *a, const TreeItem *b)
 {
-    m_model->insertItem(item);
+    auto aa = static_cast<const WatchItem *>(a);
+    auto bb = static_cast<const WatchItem *>(b);
+    return aa->name < bb->name;
 }
 
-void WatchModel::insertItem(WatchItem *item)
+void WatchHandler::insertItems(const GdbMi &data)
 {
-    QTC_ASSERT(!item->iname.isEmpty(), return);
+    QSet<TreeItem *> itemsToSort;
 
-    WatchItem *parent = findItem(parentName(item->iname));
-    QTC_ASSERT(parent, return);
+    const bool sortStructMembers = boolSetting(SortStructMembers);
+    foreach (const GdbMi &child, data.children()) {
+        auto item = new WatchItem;
+        item->parse(child, sortStructMembers);
+        const TypeInfo ti = m_model->m_reportedTypeInfo.value(item->type);
+        if (ti.size && !item->size)
+            item->size = ti.size;
+
+        const bool added = insertItem(item);
+        if (added && item->level() == 2)
+            itemsToSort.insert(item->parent());
+    }
+
+    foreach (TreeItem *toplevel, itemsToSort)
+        toplevel->sortChildren(&sortByName);
+}
+
+bool WatchHandler::insertItem(WatchItem *item)
+{
+    QTC_ASSERT(!item->iname.isEmpty(), return false);
+
+    WatchItem *parent = m_model->findItem(parentName(item->iname));
+    QTC_ASSERT(parent, return false);
 
     bool found = false;
     const QVector<TreeItem *> siblings = parent->children();
     for (int row = 0, n = siblings.size(); row < n; ++row) {
         if (static_cast<WatchItem *>(siblings.at(row))->iname == item->iname) {
-            delete takeItem(parent->children().at(row));
+            delete m_model->takeItem(parent->children().at(row));
             parent->insertChild(row, item);
             found = true;
             break;
@@ -1276,7 +1305,9 @@ void WatchModel::insertItem(WatchItem *item)
 
     item->update();
 
-    item->walkTree([this](TreeItem *sub) { showEditValue(static_cast<WatchItem *>(sub)); });
+    item->walkTree([this](TreeItem *sub) { m_model->showEditValue(static_cast<WatchItem *>(sub)); });
+
+    return !found;
 }
 
 void WatchModel::reexpandItems()
@@ -1403,7 +1434,7 @@ void WatchHandler::watchExpression(const QString &exp0, const QString &name)
     item->exp = exp;
     item->name = name.isEmpty() ? exp0 : name;
     item->iname = watcherName(exp);
-    m_model->insertItem(item);
+    insertItem(item);
     saveWatchers();
 
     if (m_model->m_engine->state() == DebuggerNotReady) {
@@ -1856,5 +1887,16 @@ QSet<QByteArray> WatchHandler::expandedINames() const
     return m_model->m_expandedINames;
 }
 
+void WatchHandler::recordTypeInfo(const GdbMi &typeInfo)
+{
+    if (typeInfo.type() == GdbMi::List) {
+        foreach (const GdbMi &s, typeInfo.children()) {
+            QByteArray typeName = QByteArray::fromHex(s["name"].data());
+            TypeInfo ti(s["size"].data().toUInt());
+            m_model->m_reportedTypeInfo.insert(typeName, ti);
+        }
+    }
+}
+
 } // namespace Internal
 } // namespace Debugger

src/plugins/debugger/watchhandler.h

@@ -109,7 +109,9 @@ public:
     void setCurrentItem(const QByteArray &iname);
     void updateWatchersWindow();
 
-    void insertItem(WatchItem *item); // Takes ownership.
+    bool insertItem(WatchItem *item); // Takes ownership, returns whether item was added, not overwritten.
+    void insertItems(const GdbMi &data);
+
     void removeItemByIName(const QByteArray &iname);
     void removeAllData(bool includeInspectData = false);
     void resetValueCache();
@@ -119,6 +121,7 @@ public:
     void notifyUpdateFinished();
 
     void reexpandItems();
+    void recordTypeInfo(const GdbMi &typeInfo);
 
 private:
     WatchModel *m_model; // Owned.