From cc587133618775ca9a0d45e71f0074b2f54fcafc Mon Sep 17 00:00:00 2001
From: Friedemann Kleint <Friedemann.Kleint@nokia.com>
Date: Tue, 23 Jun 2009 13:43:23 +0200
Subject: [PATCH] Add a fake dereferencing item '*' to CDB for dumping
 QString*, etc.

Simplify the symbol group recursion to oblivion and make
it parametrizable with predicates to control recursion and
further processing. Most importantly, insert a parent item
BEFORE its child item (ignoring for now the fact that child recursion
might fail), making it possible to check and do magic on the parent,
disabling further handling by the symbol context. Dumper handling
can then kick in and handle pointed-to dumpeable items by inserting
fake dereferencing items.
Remove logic to detect already expanded items, just re-insert the
children if the handler asks for it.
---
 .../debugger/cdb/cdbstackframecontext.cpp     | 148 ++++++++++++++----
 .../debugger/cdb/cdbsymbolgroupcontext.cpp    |  21 ++-
 .../debugger/cdb/cdbsymbolgroupcontext.h      |  33 +++-
 .../debugger/cdb/cdbsymbolgroupcontext_tpl.h  | 137 +++++-----------
 src/plugins/debugger/watchhandler.cpp         |  12 +-
 5 files changed, 218 insertions(+), 133 deletions(-)

diff --git a/src/plugins/debugger/cdb/cdbstackframecontext.cpp b/src/plugins/debugger/cdb/cdbstackframecontext.cpp
index 258f81d9949..0a4cbc1280b 100644
--- a/src/plugins/debugger/cdb/cdbstackframecontext.cpp
+++ b/src/plugins/debugger/cdb/cdbstackframecontext.cpp
@@ -36,8 +36,6 @@
 
 #include <QtCore/QDebug>
 
-enum { debug = 0 };
-
 namespace Debugger {
 namespace Internal {
 
@@ -46,7 +44,31 @@ enum { OwnerNewItem, OwnerSymbolGroup, OwnerDumper };
 typedef QSharedPointer<CdbDumperHelper> SharedPointerCdbDumperHelper;
 typedef QList<WatchData> WatchDataList;
 
-// Put a sequence of  WatchData into the model
+// Predicates for parametrizing the symbol group
+inline bool truePredicate(const WatchData & /* whatever */) { return true; }
+inline bool falsePredicate(const WatchData & /* whatever */) { return false; }
+inline bool isDumperPredicate(const WatchData &wd)
+{ return wd.source == OwnerDumper; }
+
+// Match an item that is expanded in the watchhandler.
+class WatchHandlerExpandedPredicate {
+public:
+    explicit inline WatchHandlerExpandedPredicate(const WatchHandler *wh) : m_wh(wh) {}
+    inline bool operator()(const WatchData &wd) { return m_wh->isExpandedIName(wd.iname); }
+private:
+    const WatchHandler *m_wh;
+};
+
+// Match an item by iname
+class MatchINamePredicate {
+public:
+    explicit inline MatchINamePredicate(const QString &iname) : m_iname(iname) {}
+    inline bool operator()(const WatchData &wd) { return wd.iname == m_iname; }
+private:
+    const QString &m_iname;
+};
+
+// Put a sequence of  WatchData into the model for the non-dumper case
 class WatchHandlerModelInserter {
 public:
     explicit WatchHandlerModelInserter(WatchHandler *wh) : m_wh(wh) {}
@@ -62,49 +84,106 @@ private:
     WatchHandler *m_wh;
 };
 
-// Helper to sort apart a sequence of  WatchData using the Dumpers.
+// Put a sequence of  WatchData into the model for the dumper case.
+// Sorts apart a sequence of  WatchData using the Dumpers.
 // Puts the stuff for which there is no dumper in the model
 // as is and sets ownership to symbol group. The rest goes
-// to the dumpers.
-class WatchHandlerSorterInserter {
+// to the dumpers. Additionally, checks for items pointing to a
+// dumpeable type and inserts a fake dereferenced item and value.
+class WatchHandleDumperInserter {
 public:
-    explicit WatchHandlerSorterInserter(WatchHandler *wh,
+    explicit WatchHandleDumperInserter(WatchHandler *wh,
                                         const SharedPointerCdbDumperHelper &dumper);
 
-    inline WatchHandlerSorterInserter & operator*() { return *this; }
-    inline WatchHandlerSorterInserter &operator=(WatchData wd);
-    inline WatchHandlerSorterInserter &operator++() { return *this; }
+    inline WatchHandleDumperInserter & operator*() { return *this; }
+    inline WatchHandleDumperInserter &operator=(WatchData &wd);
+    inline WatchHandleDumperInserter &operator++() { return *this; }
 
 private:
+    bool expandPointerToDumpable(const WatchData &wd, QString *errorMessage);
+
+    const QRegExp m_hexNullPattern;
     WatchHandler *m_wh;
     const SharedPointerCdbDumperHelper m_dumper;
     QList<WatchData> m_dumperResult;
-    QStringList m_dumperINames;
 };
 
-WatchHandlerSorterInserter::WatchHandlerSorterInserter(WatchHandler *wh,
+WatchHandleDumperInserter::WatchHandleDumperInserter(WatchHandler *wh,
                                                        const SharedPointerCdbDumperHelper &dumper) :
+    m_hexNullPattern(QLatin1String("0x0+")),
     m_wh(wh),
-    m_dumper(dumper)
+    m_dumper(dumper)    
 {
+    Q_ASSERT(m_hexNullPattern.isValid());
 }
 
-WatchHandlerSorterInserter &WatchHandlerSorterInserter::operator=(WatchData wd)
+// Is this a non-null pointer to a dumpeable item with a value
+// "0x4343 class QString *" ? - Insert a fake '*' dereferenced item
+// and run dumpers on it. If that succeeds, insert the fake items owned by dumpers,
+// which will trigger the ignore predicate.
+// Note that the symbol context does not create '*' dereferenced items for
+// classes (see note in its header documentation).
+bool WatchHandleDumperInserter::expandPointerToDumpable(const WatchData &wd, QString *errorMessage)
 {
-    // Is this a child belonging to some item handled by dumpers,
-    // such as d-elements of QStrings -> just ignore it.
-    if (const int dumperINamesCount = m_dumperINames.size()) {
-        for (int i = 0; i < dumperINamesCount; i++)
-            if (wd.iname.startsWith(m_dumperINames.at(i)))
-                return *this;
-    }
+    if (debugCDBWatchHandling)
+        qDebug() << ">expandPointerToDumpable" << wd.iname;
+
+    bool handled = false;
+    do {
+        if (!isPointerType(wd.type))
+            break;
+        const int classPos = wd.value.indexOf(" class ");
+        if (classPos == -1)
+            break;
+        const QString hexAddrS = wd.value.mid(0, classPos);
+        if (m_hexNullPattern.exactMatch(hexAddrS))
+            break;
+        const QString type = stripPointerType(wd.value.mid(classPos + 7));
+        WatchData derefedWd;
+        derefedWd.setType(type);
+        derefedWd.setAddress(hexAddrS);
+        derefedWd.name = QString(QLatin1Char('*'));
+        derefedWd.iname = wd.iname + QLatin1String(".*");
+        derefedWd.source = OwnerDumper;
+        const CdbDumperHelper::DumpResult dr = m_dumper->dumpType(derefedWd, true, OwnerDumper, &m_dumperResult, errorMessage);
+        if (dr != CdbDumperHelper::DumpOk)
+            break;
+        // Insert the pointer item with 1 additional child + its dumper results
+        // Note: formal arguments might already be expanded in the symbol group.
+        WatchData ptrWd = wd;
+        ptrWd.source = OwnerDumper;
+        ptrWd.setHasChildren(true);
+        ptrWd.setChildrenUnneeded();
+        m_wh->insertData(ptrWd);
+        foreach(const WatchData &dwd, m_dumperResult)
+            m_wh->insertData(dwd);
+        handled = true;
+    } while (false);
+    if (debugCDBWatchHandling)
+        qDebug() << "<expandPointerToDumpable returns " << handled << *errorMessage;
+    return handled;
+}
+
+WatchHandleDumperInserter &WatchHandleDumperInserter::operator=(WatchData &wd)
+{
+    if (debugCDBWatchHandling)
+        qDebug() << "WatchHandleDumperInserter::operator=" << wd.toString();
+    // Check pointer to dumpeable, dumpeable, insert accordingly.
     QString errorMessage;
+    if (expandPointerToDumpable(wd, &errorMessage)) {
+        // Nasty side effect: Modify owner for the ignore predicate
+        wd.source = OwnerDumper;
+        return *this;
+    }
     switch (m_dumper->dumpType(wd, true, OwnerDumper, &m_dumperResult, &errorMessage)) {
     case CdbDumperHelper::DumpOk:
+        if (debugCDBWatchHandling)
+            qDebug() << "dumper triggered";
         // Discard the original item and insert the dumper results
-        m_dumperINames.push_back(wd.iname + QLatin1Char('.'));
         foreach(const WatchData &dwd, m_dumperResult)
             m_wh->insertData(dwd);
+        // Nasty side effect: Modify owner for the ignore predicate
+        wd.source = OwnerDumper;
         break;
     case CdbDumperHelper::DumpNotHandled:
     case CdbDumperHelper::DumpError:
@@ -133,15 +212,19 @@ bool CdbStackFrameContext::assignValue(const QString &iname, const QString &valu
 bool CdbStackFrameContext::populateModelInitially(WatchHandler *wh, QString *errorMessage)
 {
     if (debugCDBWatchHandling)
-        qDebug() << "populateModelInitially";
+        qDebug() << "populateModelInitially dumpers=" << m_useDumpers;
+    // Recurse down items that are initially expanded in the view, stop processing for
+    // dumper items.
     const bool rc = m_useDumpers ?
         CdbSymbolGroupContext::populateModelInitially(m_symbolContext,
-                                                      wh->expandedINames(),
-                                                      WatchHandlerSorterInserter(wh, m_dumper),                                                      
+                                                      WatchHandleDumperInserter(wh, m_dumper),
+                                                      WatchHandlerExpandedPredicate(wh),
+                                                      isDumperPredicate,
                                                       errorMessage) :
         CdbSymbolGroupContext::populateModelInitially(m_symbolContext,
-                                                      wh->expandedINames(),
                                                       WatchHandlerModelInserter(wh),
+                                                      WatchHandlerExpandedPredicate(wh),
+                                                      falsePredicate,
                                                       errorMessage);
     return rc;
 }
@@ -151,25 +234,32 @@ bool CdbStackFrameContext::completeData(const WatchData &incompleteLocal,
                                         QString *errorMessage)
 {
     if (debugCDBWatchHandling)
-        qDebug() << ">completeData " << incompleteLocal.iname << " src=" << incompleteLocal.source;
+        qDebug() << ">completeData src=" << incompleteLocal.source << incompleteLocal.toString();
 
+    // Expand symbol group items, recurse one level from desired item
     if (!m_useDumpers) {
         return CdbSymbolGroupContext::completeData(m_symbolContext, incompleteLocal,
                                                    WatchHandlerModelInserter(wh),
+                                                   MatchINamePredicate(incompleteLocal.iname),
+                                                   falsePredicate,
                                                    errorMessage);
     }
 
     // Expand dumper items (not implemented)
     if (incompleteLocal.source == OwnerDumper) {
+        if (debugCDBWatchHandling)
+            qDebug() << "ignored dumper item";
         WatchData wd = incompleteLocal;
         wd.setAllUnneeded();
         wh->insertData(wd);
         return true;
     }
 
-    // Expand symbol group items
+    // Expand symbol group items, recurse one level from desired item
     return CdbSymbolGroupContext::completeData(m_symbolContext, incompleteLocal,
-                                               WatchHandlerSorterInserter(wh, m_dumper),
+                                               WatchHandleDumperInserter(wh, m_dumper),
+                                               MatchINamePredicate(incompleteLocal.iname),
+                                               isDumperPredicate,
                                                errorMessage);
 }
 
diff --git a/src/plugins/debugger/cdb/cdbsymbolgroupcontext.cpp b/src/plugins/debugger/cdb/cdbsymbolgroupcontext.cpp
index 14fb16c736a..12d78c0fc1c 100644
--- a/src/plugins/debugger/cdb/cdbsymbolgroupcontext.cpp
+++ b/src/plugins/debugger/cdb/cdbsymbolgroupcontext.cpp
@@ -33,6 +33,7 @@
 #include "watchutils.h"
 
 #include <QtCore/QTextStream>
+#include <QtCore/QRegExp>
 
 enum { debug = 0 };
 
@@ -353,6 +354,20 @@ static inline QString hexSymbolOffset(CIDebugSymbolGroup *sg, unsigned long inde
     return QLatin1String("0x") + QString::number(rc, 16);
 }
 
+// check for "0x000", "0x000 class X"
+static inline bool isNullPointer(const WatchData &wd)
+{
+    if (!isPointerType(wd.type))
+        return false;
+    static const QRegExp hexNullPattern(QLatin1String("0x0+"));
+    Q_ASSERT(hexNullPattern.isValid());
+    const int blankPos = wd.value.indexOf(QLatin1Char(' '));
+    if (blankPos == -1)
+        return hexNullPattern.exactMatch(wd.value);
+    const QString addr = wd.value.mid(0, blankPos);
+    return hexNullPattern.exactMatch(addr);
+}
+
 WatchData CdbSymbolGroupContext::symbolAt(unsigned long index) const
 {
     WatchData wd;
@@ -367,8 +382,10 @@ WatchData CdbSymbolGroupContext::symbolAt(unsigned long index) const
     // Figure out children. The SubElement is only a guess unless the symbol,
     // is expanded, so, we leave this as a guess for later updates.
     // If the symbol has children (expanded or not), we leave the 'Children' flag
-    // in 'needed' state.
-    wd.setHasChildren(m_symbolParameters.at(index).SubElements);
+    // in 'needed' state. Suppress 0-pointers right ("0x000 class X")
+    // here as they only lead to children with memory access errors.
+    const bool hasChildren = m_symbolParameters.at(index).SubElements && !isNullPointer(wd);
+    wd.setHasChildren(hasChildren);
     if (debug > 1)
         qDebug() << Q_FUNC_INFO << index << '\n' << wd.toString();
     return wd;
diff --git a/src/plugins/debugger/cdb/cdbsymbolgroupcontext.h b/src/plugins/debugger/cdb/cdbsymbolgroupcontext.h
index 2c5b6b3e365..91ec5fd9240 100644
--- a/src/plugins/debugger/cdb/cdbsymbolgroupcontext.h
+++ b/src/plugins/debugger/cdb/cdbsymbolgroupcontext.h
@@ -55,7 +55,14 @@ class WatchHandler;
  * "local.string.data" (class member)
  * and maintains a mapping iname -> index.
  * IDebugSymbolGroup2 can "expand" expandable symbols, inserting them into the
- * flat list after their parent. */
+ * flat list after their parent.
+ *
+ * Note the pecularity of IDebugSymbolGroup2 with regard to pointed to items:
+ * 1) A pointer to a POD (say int *) will expand to a pointed-to integer named '*'.
+ * 2) A pointer to a class (QString *), will expand to the class members right away,
+ *    omitting the '*' derefenced item. That is a problem since the dumpers trigger
+ *    only on the derefenced item, so, additional handling is required.
+ */
 
 class CdbSymbolGroupContext
 {
@@ -74,15 +81,29 @@ public:
     bool assignValue(const QString &iname, const QString &value,
                      QString *newValue /* = 0 */, QString *errorMessage);
 
-    template <class OutputIterator>
+    // Initially populate the locals model for a new stackframe.
+    // Write a sequence of WatchData to it, recurse down if the
+    // recursionPredicate agrees. The ignorePredicate can be used
+    // to terminate processing after insertion of an item (if the calling
+    // routine wants to insert another subtree).
+    template <class OutputIterator, class RecursionPredicate, class IgnorePredicate>
     static bool populateModelInitially(CdbSymbolGroupContext *sg,
-                                       QSet<QString> expandedINames,
-                                       OutputIterator it, QString *errorMessage);
-
-    template <class OutputIterator>
+                                       OutputIterator it,
+                                       RecursionPredicate recursionPredicate,
+                                       IgnorePredicate ignorePredicate,
+                                       QString *errorMessage);
+
+    // Complete children of a WatchData item.
+    // Write a sequence of WatchData to it, recurse if the
+    // recursionPredicate agrees. The ignorePredicate can be used
+    // to terminate processing after insertion of an item (if the calling
+    // routine wants to insert another subtree).
+    template <class OutputIterator, class RecursionPredicate, class IgnorePredicate>
     static bool completeData (CdbSymbolGroupContext *sg,
                               WatchData incompleteLocal,
                               OutputIterator it,
+                              RecursionPredicate recursionPredicate,
+                              IgnorePredicate ignorePredicate,
                               QString *errorMessage);
 
     // Retrieve child symbols of prefix as a sequence of WatchData.
diff --git a/src/plugins/debugger/cdb/cdbsymbolgroupcontext_tpl.h b/src/plugins/debugger/cdb/cdbsymbolgroupcontext_tpl.h
index 73e84001f84..ccaba97ee89 100644
--- a/src/plugins/debugger/cdb/cdbsymbolgroupcontext_tpl.h
+++ b/src/plugins/debugger/cdb/cdbsymbolgroupcontext_tpl.h
@@ -66,97 +66,72 @@ bool CdbSymbolGroupContext::getChildSymbols(const QString &prefix, OutputIterato
     return true;
 }
 
-template <class OutputIterator>
-        bool insertChildrenRecursion(const QString &iname,
-                                     CdbSymbolGroupContext *sg,
-                                     OutputIterator it,
-                                     int maxRecursionLevel,
-                                     int level,
-                                     QString *errorMessage,
-                                     int *childCount = 0);
-
 // Insert a symbol (and its first level children depending on forceRecursion)
-template <class OutputIterator>
+// The parent symbol is inserted before the children to make dumper handling
+// simpler. In theory, it can happen that the symbol context indicates
+// children but can expand none, which would lead to invalid parent information
+// (expand icon), though (ignore for simplicity).
+template <class OutputIterator, class RecursionPredicate, class IgnorePredicate>
 bool insertSymbolRecursion(WatchData wd,
-                                  CdbSymbolGroupContext *sg,
-                                  OutputIterator it,
-                                  int maxRecursionLevel,
-                                  int level,
-                                  QString *errorMessage)
+                           CdbSymbolGroupContext *sg,
+                           OutputIterator it,
+                           RecursionPredicate recursionPredicate,
+                           IgnorePredicate ignorePredicate,
+                           QString *errorMessage)
 {
-    // Find out whether to recurse (has children or at least knows it has children)
-    // Open next level if specified by recursion depth or child is already expanded
-    // (Sometimes, some root children are already expanded after creating the context).
+    // Find out whether to recurse (has children and predicate agrees)
     const bool hasChildren = wd.hasChildren || wd.isChildrenNeeded();
-    const bool recurse = hasChildren && (level < maxRecursionLevel || sg->isExpanded(wd.iname));
+    const bool recurse = hasChildren && recursionPredicate(wd);
     if (debugSgRecursion)
-        qDebug() << "insertSymbolRecursion" << '\n' << wd.iname << "level=" << level <<  "recurse=" << recurse;
-    bool rc = true;
-    if (recurse) { // Determine number of children and indicate in model
-        int childCount;
-        rc = insertChildrenRecursion(wd.iname, sg, it, maxRecursionLevel, level, errorMessage, &childCount);
-        if (rc) {
-            wd.setHasChildren(childCount > 0);
-            wd.setChildrenUnneeded();
-        }
-    } else {
+        qDebug() << "insertSymbolRecursion" << '\n' << wd.iname << "recurse=" << recurse;
+    if (!recurse) {
         // No further recursion at this level, pretend entry is complete
-        // to the watchmodel
+        // to the watchmodel for the parent to show (only remaining watchhandler-specific
+        // part here).
         if (wd.isChildrenNeeded()) {
             wd.setHasChildren(true);
             wd.setChildrenUnneeded();
         }
+        if (debugSgRecursion)
+            qDebug() << " INSERTING non-recursive: " << wd.toString();
+        *it = wd;
+        ++it;
+        return true;
     }
+    // Recursion: Indicate children unneeded
+    wd.setHasChildren(true);
+    wd.setChildrenUnneeded();
     if (debugSgRecursion)
-        qDebug() << " INSERTING: at " << level << wd.toString();
+        qDebug() << " INSERTING recursive: " << wd.toString();
     *it = wd;
     ++it;
-    return rc;
-}
-
-// Insert the children of prefix.
-template <class OutputIterator>
-        bool insertChildrenRecursion(const QString &iname,
-                                     CdbSymbolGroupContext *sg,
-                                     OutputIterator it,
-                                     int maxRecursionLevel,
-                                     int level,
-                                     QString *errorMessage,
-                                     int *childCountPtr)
-{
-    if (debugSgRecursion > 1)
-        qDebug() << "insertChildrenRecursion" << '\n' << iname << level;
-
+    // Recurse unless the predicate disagrees
+    if (ignorePredicate(wd))
+        return true;
     QList<WatchData> watchList;
     // This implicitly enforces expansion
-    if (!sg->getChildSymbols(iname, WatchDataBackInserter(watchList), errorMessage))
+    if (!sg->getChildSymbols(wd.iname, WatchDataBackInserter(watchList), errorMessage))
         return false;
-
     const int childCount = watchList.size();
-    if (childCountPtr)
-        *childCountPtr = childCount;
-    int succeededChildCount = 0;
     for (int c = 0; c < childCount; c++) {
-        const WatchData &wd = watchList.at(c);
+        const WatchData &cwd = watchList.at(c);
         if (wd.isValid()) { // We sometimes get empty names for deeply nested data
-            if (!insertSymbolRecursion(wd, sg, it, maxRecursionLevel, level + 1, errorMessage))
+            if (!insertSymbolRecursion(cwd, sg, it, recursionPredicate, ignorePredicate, errorMessage))
                 return false;
-            succeededChildCount++;
         }  else {
             const QString msg = QString::fromLatin1("WARNING: Skipping invalid child symbol #%2 (type %3) of '%4'.").
-                                arg(QLatin1String(Q_FUNC_INFO)).arg(c).arg(wd.type, iname);
+                                arg(QLatin1String(Q_FUNC_INFO)).arg(c).arg(cwd.type, wd.iname);
             qWarning("%s\n", qPrintable(msg));
         }
     }
-    if (childCountPtr)
-        *childCountPtr = succeededChildCount;
     return true;
 }
 
-template <class OutputIterator>
+template <class OutputIterator, class RecursionPredicate, class IgnorePredicate>
 bool CdbSymbolGroupContext::populateModelInitially(CdbSymbolGroupContext *sg,
-                                                   QSet<QString> expandedINames,
                                                    OutputIterator it,
+                                                   RecursionPredicate recursionPredicate,
+                                                   IgnorePredicate ignorePredicate,
                                                    QString *errorMessage)
 {
     if (debugSgRecursion)
@@ -166,53 +141,27 @@ bool CdbSymbolGroupContext::populateModelInitially(CdbSymbolGroupContext *sg,
     QList<WatchData> watchList;
     if (!sg->getChildSymbols(sg->prefix(), WatchDataBackInserter(watchList), errorMessage))
         return false;
-    // (Recursively) expand symbols stored as expanded in the history until no more matches
-    // are found.
-    while (!expandedINames.empty()) {
-        unsigned matchCount = 0;
-        for (QSet<QString>::iterator it = expandedINames.begin(); it != expandedINames.end(); ) {
-            // Try to expand. We might hit on a leaf due to name mismatches, ignore errors.
-            unsigned long index;
-            if (sg->lookupPrefix(*it, &index)) {
-                if (!sg->expandSymbol(*it, index, errorMessage))
-                    qWarning("%s\n", qPrintable(*errorMessage));
-                matchCount++;
-                it = expandedINames.erase(it);
-            } else {
-                ++it;
-            }
-        } // loop set
-        if (matchCount == 0)
-            break;
-    }
     // Insert data
     foreach(const WatchData &wd, watchList)
-        if (!insertSymbolRecursion(wd, sg, it, 0, 0, errorMessage))
+        if (!insertSymbolRecursion(wd, sg, it, recursionPredicate, ignorePredicate, errorMessage))
             return false;
     return true;
 }
 
-template <class OutputIterator>
+template <class OutputIterator, class RecursionPredicate, class IgnorePredicate>
 bool CdbSymbolGroupContext::completeData(CdbSymbolGroupContext *sg,
                                          WatchData incompleteLocal,
                                          OutputIterator it,
+                                         RecursionPredicate recursionPredicate,
+                                         IgnorePredicate ignorePredicate,
                                          QString *errorMessage)
 {
     if (debugSgRecursion)
         qDebug().nospace() << "###>CdbSymbolGroupContext::completeData" << ' ' << incompleteLocal.iname << '\n';
-    const bool contextExpanded = sg->isExpanded(incompleteLocal.iname);
-    if (debugSgRecursion)
-        qDebug() << "  " << incompleteLocal.iname << "CE=" << contextExpanded;
-    // The view reinserts any node being expanded with flag 'ChildrenNeeded'.
-    // Recurse down one level in context unless this is already the case.
-    if (contextExpanded) {
-        incompleteLocal.setChildrenUnneeded();
-        *it = incompleteLocal;
-        ++it;
-    } else {
-        if (!insertSymbolRecursion(incompleteLocal, sg, it, 1, 0, errorMessage))
-            return false;
-    }
+    // If the symbols are already expanded in the context, they will be re-inserted,
+    // which is not handled for simplicity.
+    if (!insertSymbolRecursion(incompleteLocal, sg, it, recursionPredicate, ignorePredicate, errorMessage))
+        return false;
     return true;
 }
 
diff --git a/src/plugins/debugger/watchhandler.cpp b/src/plugins/debugger/watchhandler.cpp
index b8f794b4a09..79e67dd8685 100644
--- a/src/plugins/debugger/watchhandler.cpp
+++ b/src/plugins/debugger/watchhandler.cpp
@@ -877,7 +877,7 @@ void WatchHandler::watchExpression(const QString &exp)
     WatchData data;
     data.exp = exp;
     data.name = exp;
-    if (exp == watcherEditPlaceHolder())
+    if (exp.isEmpty() || exp == watcherEditPlaceHolder())
         data.setAllUnneeded();
     data.iname = watcherName(exp);
     insertData(data);
@@ -1012,7 +1012,15 @@ void WatchHandler::loadWatchers()
 void WatchHandler::saveWatchers()
 {
     //qDebug() << "SAVE WATCHERS: " << m_watchers.keys();
-    setSessionValueRequested("Watchers", QVariant(m_watcherNames.keys()));
+    // Filter out valid watchers.
+    QStringList watcherNames;
+    const QHash<QString, int>::const_iterator cend = m_watcherNames.constEnd();
+    for (QHash<QString, int>::const_iterator it = m_watcherNames.constBegin(); it != cend; ++it) {
+        const QString &watcherName = it.key();
+        if (!watcherName.isEmpty() && watcherName != watcherEditPlaceHolder())
+            watcherNames.push_back(watcherName);
+    }
+    setSessionValueRequested("Watchers", QVariant(watcherNames));
 }
 
 void WatchHandler::saveSessionData()
-- 
GitLab