debugger: start tightening the breakpoint state machinery

src/plugins/debugger/breakhandler.cpp

@@ -341,6 +341,16 @@ BreakpointIds BreakHandler::findBreakpointsByIndex(const QList<QModelIndex> &lis
     return ids;
 }
 
+Qt::ItemFlags BreakHandler::flags(const QModelIndex &index) const
+{
+//    switch (index.column()) {
+//        //case 0:
+//        //    return Qt::ItemIsUserCheckable | Qt::ItemIsEnabled;
+//        default:
+            return QAbstractTableModel::flags(index);
+//    }
+}
+
 QVariant BreakHandler::data(const QModelIndex &mi, int role) const
 {
     static const QString empty = QString(QLatin1Char('-'));
@@ -531,19 +541,6 @@ BreakpointState BreakHandler::state(BreakpointId id) const
     return it->state;
 }
 
-void BreakHandler::setState(BreakpointId id, BreakpointState state)
-{
-    Iterator it = m_storage.find(id);
-    QTC_ASSERT(it != m_storage.end(), return);
-    if (it->state == state) {
-        qDebug() << "STATE UNCHANGED: " << id << state;
-        return;
-    }
-    it->state = state;
-    updateMarker(id);
-    layoutChanged();
-}
-
 DebuggerEngine *BreakHandler::engine(BreakpointId id) const
 {
     ConstIterator it = m_storage.find(id);
@@ -563,6 +560,124 @@ void BreakHandler::setEngine(BreakpointId id, DebuggerEngine *value)
     scheduleSynchronization();
 }
 
+static bool isAllowedTransition(BreakpointState from, BreakpointState to)
+{
+    switch (from) {
+    case BreakpointNew:
+        return to == BreakpointInsertRequested;
+    case BreakpointInsertRequested:
+        return to == BreakpointInsertProceeding;
+    case BreakpointInsertProceeding:
+        return to == BreakpointInserted
+            || to == BreakpointPending
+            || to == BreakpointDead;
+    case BreakpointChangeRequested:
+        return to == BreakpointChangeProceeding;
+    case BreakpointChangeProceeding:
+        return to == BreakpointInserted
+            || to == BreakpointPending
+            || to == BreakpointDead;
+    case BreakpointPending:
+        return false;
+    case BreakpointInserted:
+        return false;
+    case BreakpointRemoveRequested:
+        return false;
+    case BreakpointRemoveProceeding:
+        return false;
+    case BreakpointDead:
+        return false;
+    }
+    qDebug() << "UNKNOWN BREAKPOINT STATE:" << from;
+    return false;
+}
+
+void BreakHandler::setState(BreakpointId id, BreakpointState state)
+{
+    Iterator it = m_storage.find(id);
+    QTC_ASSERT(it != m_storage.end(), return);
+    QTC_ASSERT(isAllowedTransition(it->state, state),
+        qDebug() << "UNEXPECTED BREAKPOINT STATE TRANSITION"
+            << it->state << state);
+
+    if (it->state == state) {
+        qDebug() << "STATE UNCHANGED: " << id << state;
+        return;
+    }
+
+    it->state = state;
+}
+
+void BreakHandler::notifyBreakpointInsertProceeding(BreakpointId id)
+{
+    QTC_ASSERT(state(id)== BreakpointInsertRequested, /**/);
+    setState(id, BreakpointInsertProceeding);
+}
+
+void BreakHandler::notifyBreakpointInsertOk(BreakpointId id)
+{
+    QTC_ASSERT(state(id)== BreakpointInsertProceeding, /**/);
+    setState(id, BreakpointInserted);
+}
+
+void BreakHandler::notifyBreakpointInsertFailed(BreakpointId id)
+{
+    QTC_ASSERT(state(id)== BreakpointInsertProceeding, /**/);
+    setState(id, BreakpointDead);
+}
+
+void BreakHandler::notifyBreakpointRemoveProceeding(BreakpointId id)
+{
+    QTC_ASSERT(state(id)== BreakpointRemoveRequested, /**/);
+    setState(id, BreakpointInsertProceeding);
+}
+
+void BreakHandler::notifyBreakpointRemoveOk(BreakpointId id)
+{
+    QTC_ASSERT(state(id) == BreakpointRemoveProceeding, /**/);
+    setState(id, BreakpointDead);
+    cleanupBreakpoint(id);
+}
+
+void BreakHandler::notifyBreakpointRemoveFailed(BreakpointId id)
+{
+    QTC_ASSERT(state(id) == BreakpointRemoveProceeding, /**/);
+    setState(id, BreakpointDead);
+    cleanupBreakpoint(id);
+}
+
+void BreakHandler::notifyBreakpointChangeOk(BreakpointId id)
+{
+    QTC_ASSERT(state(id) == BreakpointChangeProceeding, /**/);
+    setState(id, BreakpointInserted);
+}
+
+void BreakHandler::notifyBreakpointChangeFailed(BreakpointId id)
+{
+    QTC_ASSERT(state(id) == BreakpointChangeProceeding, /**/);
+    setState(id, BreakpointDead);
+}
+
+void BreakHandler::notifyBreakpointPending(BreakpointId id)
+{
+    //QTC_ASSERT(state(id)== BreakpointInsertProceeding, /**/);
+    setState(id, BreakpointPending);
+}
+
+void BreakHandler::notifyBreakpointReleased(BreakpointId id)
+{
+    //QTC_ASSERT(state(id) == BreakpointChangeProceeding, /**/);
+    Iterator it = m_storage.find(id);
+    QTC_ASSERT(it != m_storage.end(), return);
+    it->state = BreakpointNew;
+    it->engine = 0;
+    it->response = BreakpointResponse();
+    delete it->marker;
+    it->marker = 0;
+    updateMarker(id);
+    layoutChanged();
+}
+
 void BreakHandler::ackCondition(BreakpointId id)
 {
     Iterator it = m_storage.find(id);
@@ -587,16 +702,6 @@ void BreakHandler::ackEnabled(BreakpointId id)
     updateMarker(id);
 }
 
-Qt::ItemFlags BreakHandler::flags(const QModelIndex &index) const
-{
-//    switch (index.column()) {
-//        //case 0:
-//        //    return Qt::ItemIsUserCheckable | Qt::ItemIsEnabled;
-//        default:
-            return QAbstractTableModel::flags(index);
-//    }
-}
-
 void BreakHandler::removeBreakpoint(BreakpointId id)
 {
     Iterator it = m_storage.find(id);
@@ -775,58 +880,6 @@ BreakpointIds BreakHandler::engineBreakpointIds(DebuggerEngine *engine) const
     return ids;
 }
 
-void BreakHandler::notifyBreakpointInsertOk(BreakpointId id)
-{
-    QTC_ASSERT(state(id)== BreakpointInsertProceeding, /**/);
-    setState(id, BreakpointInserted);
-}
-
-void BreakHandler::notifyBreakpointInsertFailed(BreakpointId id)
-{
-    QTC_ASSERT(state(id)== BreakpointInsertProceeding, /**/);
-    setState(id, BreakpointDead);
-}
-
-void BreakHandler::notifyBreakpointRemoveOk(BreakpointId id)
-{
-    QTC_ASSERT(state(id) == BreakpointRemoveProceeding, /**/);
-    setState(id, BreakpointDead);
-    cleanupBreakpoint(id);
-}
-
-void BreakHandler::notifyBreakpointRemoveFailed(BreakpointId id)
-{
-    QTC_ASSERT(state(id) == BreakpointRemoveProceeding, /**/);
-    setState(id, BreakpointDead);
-    cleanupBreakpoint(id);
-}
-
-void BreakHandler::notifyBreakpointChangeOk(BreakpointId id)
-{
-    QTC_ASSERT(state(id) == BreakpointChangeProceeding, /**/);
-    setState(id, BreakpointInserted);
-}
-
-void BreakHandler::notifyBreakpointChangeFailed(BreakpointId id)
-{
-    QTC_ASSERT(state(id) == BreakpointChangeProceeding, /**/);
-    setState(id, BreakpointDead);
-}
-
-void BreakHandler::notifyBreakpointReleased(BreakpointId id)
-{
-    //QTC_ASSERT(state(id) == BreakpointChangeProceeding, /**/);
-    Iterator it = m_storage.find(id);
-    QTC_ASSERT(it != m_storage.end(), return);
-    it->state = BreakpointNew;
-    it->engine = 0;
-    it->response = BreakpointResponse();
-    delete it->marker;
-    it->marker = 0;
-    updateMarker(id);
-    layoutChanged();
-}
-
 void BreakHandler::cleanupBreakpoint(BreakpointId id)
 {
     QTC_ASSERT(state(id) == BreakpointDead, /**/);

src/plugins/debugger/breakhandler.h

@@ -105,8 +105,6 @@ public:
         void setter(BreakpointId id, const type &value);
 
     PROPERTY(bool, useFullPath, setUseFullPath)
-    //PROPERTY(QString, markerFileName, setMarkerFileName)
-    //PROPERTY(int, markerLineNumber, setMarkerLineNumber)
     PROPERTY(QByteArray, condition, setCondition)
     PROPERTY(int, ignoreCount, setIgnoreCount)
     PROPERTY(QByteArray, threadSpec, setThreadSpec)
@@ -134,15 +132,19 @@ public:
     void ackIgnoreCount(BreakpointId id);
     void ackEnabled(BreakpointId id);
 
+    // State transitions.
+    void notifyBreakpointInsertProceeding(BreakpointId id);
     void notifyBreakpointInsertOk(BreakpointId id);
     void notifyBreakpointInsertFailed(BreakpointId id);
     void notifyBreakpointChangeOk(BreakpointId id);
     void notifyBreakpointChangeFailed(BreakpointId id);
+    void notifyBreakpointPending(BreakpointId id);
+    void notifyBreakpointRemoveProceeding(BreakpointId id);
     void notifyBreakpointRemoveOk(BreakpointId id);
     void notifyBreakpointRemoveFailed(BreakpointId id);
     void notifyBreakpointReleased(BreakpointId id);
 
-
+private:
 public:
     // FIXME: Make private.
     void setState(BreakpointId id, BreakpointState state);
@@ -150,7 +152,7 @@ public:
 private:
     friend class BreakpointMarker;
 
-    // QAbstractItemModel
+    // QAbstractItemModel implementation.
     int columnCount(const QModelIndex &parent) const;
     int rowCount(const QModelIndex &parent) const;
     QVariant data(const QModelIndex &index, int role) const;

src/plugins/debugger/debuggerengine.cpp

@@ -685,7 +685,7 @@ static bool isAllowedTransition(DebuggerState from, DebuggerState to)
         return to == EngineSetupRequested; // Happens on restart.
     }
 
-    qDebug() << "UNKNOWN STATE:" << from;
+    qDebug() << "UNKNOWN DEBUGGER STATE:" << from;
     return false;
 }
 

src/plugins/debugger/gdb/gdbengine.cpp

@@ -427,7 +427,7 @@ void GdbEngine::handleResponse(const QByteArray &buff)
                 const GdbMi bkpt = result.findChild("bkpt");
                 const int number = bkpt.findChild("number").data().toInt();
                 BreakpointId id = breakHandler()->findBreakpointByNumber(number);
-                setBreakpointDataFromOutput(id, bkpt);
+                updateBreakpointDataFromOutput(id, bkpt);
             } else {
                 qDebug() << "IGNORED ASYNC OUTPUT"
                     << asyncClass << result.toString();
@@ -2021,13 +2021,12 @@ void GdbEngine::setTokenBarrier()
 //
 //////////////////////////////////////////////////////////////////////
 
-void GdbEngine::setBreakpointDataFromOutput(BreakpointId id, const GdbMi &bkpt)
+void GdbEngine::updateBreakpointDataFromOutput(BreakpointId id, const GdbMi &bkpt)
 {
-    if (!bkpt.isValid())
-        return;
+    QTC_ASSERT(bkpt.isValid(), return);
+    BreakpointResponse response = breakHandler()->response(id);
 
-    BreakpointResponse response;
-    //data->pending = false;
+    bool pending = false;
     response.multiple = false;
     response.enabled = true;
     response.condition.clear();
@@ -2067,7 +2066,7 @@ void GdbEngine::setBreakpointDataFromOutput(BreakpointId id, const GdbMi &bkpt)
             response.enabled = (child.data() == "y");
         } else if (child.hasName("pending")) {
             //data->setState(BreakpointPending);
-            breakHandler()->setState(id, BreakpointPending);
+            pending = true;
             // Any content here would be interesting only if we did accept
             // spontaneously appearing breakpoints (user using gdb commands).
         } else if (child.hasName("at")) {
@@ -2105,6 +2104,11 @@ void GdbEngine::setBreakpointDataFromOutput(BreakpointId id, const GdbMi &bkpt)
     if (!name.isEmpty())
         response.fileName = name;
 
+    // FIXME: Should this honour current state?
+    if (pending)
+        breakHandler()->notifyBreakpointPending(id);
+    else
+        breakHandler()->notifyBreakpointInsertOk(id);
     breakHandler()->setResponse(id, response);
 }
 
@@ -2178,8 +2182,7 @@ void GdbEngine::handleBreakInsert1(const GdbResponse &response)
     if (response.resultClass == GdbResultDone) {
         // Interesting only on Mac?
         GdbMi bkpt = response.data.findChild("bkpt");
-        setBreakpointDataFromOutput(id, bkpt);
-        notifyBreakpointInsertOk(id);
+        updateBreakpointDataFromOutput(id, bkpt);
     } else {
         // Some versions of gdb like "GNU gdb (GDB) SUSE (6.8.91.20090930-2.4)"
         // know how to do pending breakpoints using CLI but not MI. So try
@@ -2255,10 +2258,10 @@ void GdbEngine::handleBreakList(const GdbMi &table)
         needle.fileName = _("xx");
         BreakpointId id = breakHandler()->findSimilarBreakpoint(needle);
         //qDebug() << "\n\nGOT: " << bkpt.toString() << '\n' << temp.toString();
-        // FIXME: use setBreakpointDataFromOutput(BreakpointId id, const GdbMi &bkpt)
+        // FIXME: use updateBreakpointDataFromOutput()
         if (id != BreakpointId(-1)) {
             //qDebug() << "  FROM: " << data->toString();
-            setBreakpointDataFromOutput(id, bkpt);
+            updateBreakpointDataFromOutput(id, bkpt);
             //qDebug() << "  TO: " << data->toString();
         } else {
             qDebug() << "  NOTHING SUITABLE FOUND";
@@ -2555,7 +2558,7 @@ void GdbEngine::insertBreakpoint(BreakpointId id)
     // by the MI interface.
     BreakHandler *handler = breakHandler();
     QTC_ASSERT(handler->state(id) == BreakpointInsertRequested, /**/);
-    handler->setState(id, BreakpointInsertProceeding);
+    handler->notifyBreakpointInsertProceeding(id);
     if (handler->type(id) == Watchpoint) {
         postCommand("watch " + addressSpec(handler->address(id)),
             NeedsStop | RebuildBreakpointModel,
@@ -2649,13 +2652,14 @@ void GdbEngine::removeBreakpoint(BreakpointId id)
 {
     BreakHandler *handler = breakHandler();
     QTC_ASSERT(handler->state(id) == BreakpointRemoveRequested, /**/);
-    handler->setState(id, BreakpointRemoveProceeding);
+    handler->notifyBreakpointRemoveProceeding(id);
     BreakpointResponse br = handler->response(id);
     showMessage(_("DELETING BP %1 IN ").arg(br.number)
         + handler->fileName(id));
     postCommand("-break-delete " + QByteArray::number(br.number),
         NeedsStop | RebuildBreakpointModel);
     // Pretend it succeeds without waiting for response. Feels better.
+    // FIXME: Really?
     handler->notifyBreakpointRemoveOk(id);
 }
 

src/plugins/debugger/gdb/gdbengine.h

@@ -357,7 +357,7 @@ private: ////////// View & Data Stuff //////////
     void handleWatchInsert(const GdbResponse &response);
     void handleInfoLine(const GdbResponse &response);
     void extractDataFromInfoBreak(const QString &output, BreakpointId);
-    void setBreakpointDataFromOutput(BreakpointId id, const GdbMi &bkpt);
+    void updateBreakpointDataFromOutput(BreakpointId id, const GdbMi &bkpt);
     QByteArray breakpointLocation(BreakpointId id);
     QString breakLocation(const QString &file) const;
     void reloadBreakListInternal();

src/plugins/debugger/pdb/pdbengine.cpp

@@ -333,7 +333,7 @@ void PdbEngine::attemptBreakpointSynchronization()
     bool updateNeeded = false;
     foreach (BreakpointId id, handler->engineBreakpointIds(this)) {
         if (handler->state(id) == BreakpointInsertRequested) {
-            handler->setState(id, BreakpointInserted); // FIXME
+            handler->notifyBreakpointInsertOk(id);
             updateNeeded = true;
 
             QByteArray loc;

src/plugins/debugger/qml/qmlengine.cpp

@@ -702,7 +702,7 @@ void QmlEngine::messageReceived(const QByteArray &message)
             foreach (BreakpointId id, handler->engineBreakpointIds(this)) {
                 QString processedFilename = handler->fileName(id);
                 if (processedFilename == file && handler->lineNumber(id) == line) {
-                    handler->setState(id, BreakpointInserted);
+                    handler->notifyBreakpointInsertOk(id);
                     BreakpointResponse br = handler->response(id);
                     br.fileName = file;
                     br.lineNumber = line;

src/plugins/debugger/script/scriptengine.cpp

@@ -630,7 +630,7 @@ bool ScriptEngine::checkForBreakCondition(bool byFunction)
         br.lineNumber = lineNumber;
         br.fileName = fileName;
         br.functionName = functionName;
-        handler->setState(id, BreakpointInserted);
+        handler->notifyBreakpointInsertOk(id);
         handler->setResponse(id, br);
     }
     notifyInferiorSpontaneousStop();