DAP: Move to a unified way to handle breakpoints

Rely on SetBreakpoints response instead of breakpoint event.

Change-Id: Iff052a13c442fb1fcd945cf1a80f1354c43c15c5
Reviewed-by: hjk <hjk@qt.io>

src/plugins/debugger/breakhandler.cpp

@@ -1241,7 +1241,7 @@ static bool isAllowedTransition(BreakpointState from, BreakpointState to)
 
 void BreakpointItem::gotoState(BreakpointState target, BreakpointState assumedCurrent)
 {
-    QTC_ASSERT(m_state == assumedCurrent, qDebug() << m_state);
+    QTC_ASSERT(m_state == assumedCurrent, qDebug() << target << m_state);
     setState(target);
 }
 

src/plugins/debugger/dap/cmakedapengine.cpp

@@ -142,6 +142,11 @@ void CMakeDapEngine::setupEngine()
     });
 }
 
+bool CMakeDapEngine::acceptsBreakpoint(const BreakpointParameters &bp) const
+{
+    return bp.fileName.endsWith(".txt") || bp.fileName.endsWith(".cmake");
+}
+
 bool CMakeDapEngine::hasCapability(unsigned cap) const
 {
     return cap & (ReloadModuleCapability
@@ -151,32 +156,6 @@ bool CMakeDapEngine::hasCapability(unsigned cap) const
                   /*| RunToLineCapability*/); // disable while the #25176 bug is not fixed
 }
 
-void CMakeDapEngine::insertBreakpoint(const Breakpoint &bp)
-{
-    DapEngine::insertBreakpoint(bp);
-    notifyBreakpointInsertOk(bp); // Needed for CMake support issue:25176
-}
-
-void CMakeDapEngine::removeBreakpoint(const Breakpoint &bp)
-{
-    DapEngine::removeBreakpoint(bp);
-    notifyBreakpointRemoveOk(bp); // Needed for CMake support issue:25176
-}
-
-void CMakeDapEngine::updateBreakpoint(const Breakpoint &bp)
-{
-    DapEngine::updateBreakpoint(bp);
-
-    /* Needed for CMake support issue:25176 */
-    BreakpointParameters parameters = bp->requestedParameters();
-    if (parameters.enabled != bp->isEnabled()) {
-        parameters.pending = false;
-        bp->setParameters(parameters);
-    }
-    notifyBreakpointChangeOk(bp);
-    /* Needed for CMake support issue:25176 */
-}
-
 const QLoggingCategory &CMakeDapEngine::logCategory()
 {
     static const QLoggingCategory logCategory = QLoggingCategory("qtc.dbg.dapengine.cmake",

src/plugins/debugger/dap/cmakedapengine.h

@@ -15,14 +15,8 @@ public:
 private:
     void setupEngine() override;
 
-    /* Needed for CMake support issue:25176 */
+    bool acceptsBreakpoint(const BreakpointParameters &bp) const override;
-    void insertBreakpoint(const Breakpoint &bp) override;
-    void updateBreakpoint(const Breakpoint &bp) override;
-    void removeBreakpoint(const Breakpoint &bp) override;
-    /* Needed for CMake support issue:25176 */
-
     bool hasCapability(unsigned cap) const override;
-
     const QLoggingCategory &logCategory() override;
 };
 

src/plugins/debugger/dap/dapclient.cpp

@@ -224,6 +224,8 @@ void DapClient::emitSignals(const QJsonDocument &doc)
             type = DapResponseType::Pause;
         } else if (command == "evaluate") {
             type = DapResponseType::Evaluate;
+        } else if (command == "setBreakpoints") {
+            type = DapResponseType::SetBreakpoints;
         }
         emit responseReady(type, ob);
         return;

src/plugins/debugger/dap/dapclient.h

@@ -52,6 +52,7 @@ enum class DapResponseType
     StepOver,
     Pause,
     Evaluate,
+    SetBreakpoints,
     Unknown
 };
 

src/plugins/debugger/dap/dapengine.cpp

@@ -325,19 +325,27 @@ void DapEngine::insertBreakpoint(const Breakpoint &bp)
     QTC_CHECK(bp->state() == BreakpointInsertionRequested);
     notifyBreakpointInsertProceeding(bp);
 
+    BreakpointParameters parameters = bp->requestedParameters();
+    if (!parameters.enabled) { // hack for disabling breakpoints
+        parameters.pending = false;
+        bp->setParameters(parameters);
+        notifyBreakpointInsertOk(bp);
+        return;
+    }
+
     dapInsertBreakpoint(bp);
 }
 
 void DapEngine::dapInsertBreakpoint(const Breakpoint &bp)
 {
-    bp->setResponseId(QString::number(m_nextBreakpointId++));
     const BreakpointParameters &params = bp->requestedParameters();
 
     QJsonArray breakpoints;
     for (const auto &breakpoint : breakHandler()->breakpoints()) {
         const BreakpointParameters &bpParams = breakpoint->requestedParameters();
         QJsonObject jsonBp = createBreakpoint(bpParams);
-        if (!jsonBp.isEmpty() && params.fileName.path() == bpParams.fileName.path()) {
+        if (!jsonBp.isEmpty() && params.fileName.path() == bpParams.fileName.path()
+            && bpParams.enabled) {
             breakpoints.append(jsonBp);
         }
     }
@@ -376,7 +384,8 @@ void DapEngine::dapRemoveBreakpoint(const Breakpoint &bp)
     QJsonArray breakpoints;
     for (const auto &breakpoint : breakHandler()->breakpoints()) {
         const BreakpointParameters &bpParams = breakpoint->requestedParameters();
-        if (breakpoint->responseId() != bp->responseId() && params.fileName == bpParams.fileName) {
+        if (breakpoint->responseId() != bp->responseId() && params.fileName == bpParams.fileName
+            && bpParams.enabled) {
             QJsonObject jsonBp = createBreakpoint(bpParams);
             breakpoints.append(jsonBp);
         }
@@ -599,6 +608,9 @@ void DapEngine::handleResponse(DapResponseType type, const QJsonObject &response
     case DapResponseType::Evaluate:
         handleEvaluateResponse(response);
         break;
+    case DapResponseType::SetBreakpoints:
+        handleBreakpointResponse(response);
+        break;
     default:
         showMessage("UNKNOWN RESPONSE:" + command);
     };
@@ -693,6 +705,90 @@ void DapEngine::handleEvaluateResponse(const QJsonObject &response)
     m_variablesHandler->handleNext();
 }
 
+void DapEngine::handleBreakpointResponse(const QJsonObject &response)
+{
+    const QJsonObject body = response.value("body").toObject();
+    QJsonArray breakpoints = body.value("breakpoints").toArray();
+
+    QHash<QString, QJsonObject> map;
+    for (QJsonValueRef jsonbp : breakpoints) {
+        QJsonObject breakpoint = jsonbp.toObject();
+        QString fileName = breakpoint.value("source").toObject().value("path").toString();
+        int line = breakpoint.value("line").toInt();
+
+        map.insert(fileName + ":" + QString::number(line), breakpoint);
+    }
+
+    const Breakpoints bps = breakHandler()->breakpoints();
+    for (const Breakpoint &bp : bps) {
+        BreakpointParameters parameters = bp->requestedParameters();
+        QString mapKey = parameters.fileName.toString() + ":"
+                         + QString::number(parameters.textPosition.line);
+        if (map.find(mapKey) != map.end()) {
+            if (bp->state() == BreakpointRemoveProceeding) {
+                notifyBreakpointRemoveFailed(bp);
+            } else if (bp->state() == BreakpointInsertionProceeding
+                       && !map.value(mapKey).value("verified").toBool()) {
+                notifyBreakpointInsertFailed(bp);
+            } else if (bp->state() == BreakpointInsertionProceeding) {
+                parameters.pending = false;
+                bp->setParameters(parameters);
+                notifyBreakpointInsertOk(bp);
+            }
+            if (!bp.isNull())
+                bp->setResponseId(QString::number(map.value(mapKey).value("id").toInt()));
+            map.remove(mapKey);
+        } else {
+            if (bp->state() == BreakpointRemoveProceeding) {
+                notifyBreakpointRemoveOk(bp);
+            }
+        }
+
+        if (!bp.isNull() && bp->state() == BreakpointUpdateProceeding) {
+            BreakpointParameters parameters = bp->requestedParameters();
+            if (parameters.enabled != bp->isEnabled()) {
+                parameters.pending = false;
+                bp->setParameters(parameters);
+                notifyBreakpointChangeOk(bp);
+                continue;
+            }
+        }
+    }
+
+    for (const Breakpoint &bp : breakHandler()->breakpoints()) {
+        if (bp->state() == BreakpointInsertionProceeding) {
+            if (!bp->isEnabled())
+                continue;
+
+            QString path = bp->requestedParameters().fileName.toString();
+            int line = bp->requestedParameters().textPosition.line;
+
+            QJsonObject jsonBreakpoint;
+            QString key;
+            for (QString bpKey : map.keys()) {
+                QJsonObject breakpoint = map.value(bpKey);
+                if (path == bp->requestedParameters().fileName.toString()
+                    && abs(breakpoint.value("line").toInt() - line)
+                           < abs(jsonBreakpoint.value("line").toInt() - line)) {
+                    jsonBreakpoint = breakpoint;
+                    key = bpKey;
+                }
+            }
+
+            if (!jsonBreakpoint.isEmpty() && jsonBreakpoint.value("verified").toBool()) {
+                BreakpointParameters parameters = bp->requestedParameters();
+                parameters.pending = false;
+                parameters.textPosition.line = jsonBreakpoint.value("line").toInt();
+                parameters.textPosition.column = jsonBreakpoint.value("column").toInt();
+                bp->setParameters(parameters);
+                bp->setResponseId(QString::number(jsonBreakpoint.value("id").toInt()));
+                notifyBreakpointInsertOk(bp);
+                map.remove(key);
+            }
+        }
+    }
+}
+
 void DapEngine::handleEvent(DapEventType type, const QJsonObject &event)
 {
     const QString eventType = event.value("event").toString();
@@ -716,9 +812,6 @@ void DapEngine::handleEvent(DapEventType type, const QJsonObject &event)
         if (body.value("reason").toString() == "started" && body.value("threadId").toInt() == 1)
             claimInitialBreakpoints();
         break;
-    case DapEventType::DapBreakpoint:
-        handleBreakpointEvent(event);
-        break;
     case DapEventType::Output: {
         const QString category = body.value("category").toString();
         const QString output = body.value("output").toString();
@@ -761,51 +854,6 @@ void DapEngine::handleStoppedEvent(const QJsonObject &event)
     m_dapClient->threads();
 }
 
-void DapEngine::handleBreakpointEvent(const QJsonObject &event)
-{
-    const QJsonObject body = event.value("body").toObject();
-    QJsonObject breakpoint = body.value("breakpoint").toObject();
-
-    Breakpoint bp = breakHandler()->findBreakpointByResponseId(
-        QString::number(breakpoint.value("id").toInt()));
-    qCDebug(logCategory()) << "breakpoint id :" << breakpoint.value("id").toInt();
-
-    if (bp) {
-        BreakpointParameters parameters = bp->requestedParameters();
-        if (parameters.enabled != bp->isEnabled()) {
-            parameters.pending = false;
-            bp->setParameters(parameters);
-            notifyBreakpointChangeOk(bp);
-            return;
-        }
-    }
-
-    if (body.value("reason").toString() == "new") {
-        if (breakpoint.value("verified").toBool()) {
-            notifyBreakpointInsertOk(bp);
-            const BreakpointParameters &params = bp->requestedParameters();
-            if (params.oneShot)
-                continueInferior();
-            qCDebug(logCategory()) << "breakpoint inserted";
-        } else {
-            notifyBreakpointInsertFailed(bp);
-            qCDebug(logCategory()) << "breakpoint insertion failed";
-        }
-        return;
-    }
-
-    if (body.value("reason").toString() == "removed") {
-        if (breakpoint.value("verified").toBool()) {
-            notifyBreakpointRemoveOk(bp);
-            qCDebug(logCategory()) << "breakpoint removed";
-        } else {
-            notifyBreakpointRemoveFailed(bp);
-            qCDebug(logCategory()) << "breakpoint remove failed";
-        }
-        return;
-    }
-}
-
 void DapEngine::refreshLocals(const QJsonArray &variables)
 {
     WatchItem *currentItem = watchHandler()->findItem(m_variablesHandler->currentItem().iname);

src/plugins/debugger/dap/dapengine.h

@@ -128,9 +128,9 @@ protected:
     void handleScopesResponse(const QJsonObject &response);
     void handleThreadsResponse(const QJsonObject &response);
     void handleEvaluateResponse(const QJsonObject &response);
+    void handleBreakpointResponse(const QJsonObject &response);
 
     void handleEvent(DapEventType type, const QJsonObject &event);
-    void handleBreakpointEvent(const QJsonObject &event);
     void handleStoppedEvent(const QJsonObject &event);
 
     void updateAll() override;

src/plugins/debugger/dap/gdbdapengine.cpp

@@ -161,6 +161,11 @@ void GdbDapEngine::setupEngine()
     m_dapClient->dataProvider()->start();
 }
 
+bool GdbDapEngine::acceptsBreakpoint(const BreakpointParameters &bp) const
+{
+    return bp.fileName.endsWith(".cpp") || bp.fileName.endsWith(".h");
+}
+
 const QLoggingCategory &GdbDapEngine::logCategory()
 {
     static const QLoggingCategory logCategory = QLoggingCategory("qtc.dbg.dapengine.gdb",

src/plugins/debugger/dap/gdbdapengine.h

@@ -19,7 +19,7 @@ private:
     void handleDapConfigurationDone() override;
 
     bool isLocalAttachEngine() const;
-
+    bool acceptsBreakpoint(const BreakpointParameters &bp) const override;
     const QLoggingCategory &logCategory() override;
 };
 

src/plugins/debugger/dap/pydapengine.cpp

@@ -204,32 +204,6 @@ bool PyDapEngine::acceptsBreakpoint(const BreakpointParameters &bp) const
     return bp.fileName.endsWith(".py");
 }
 
-void PyDapEngine::insertBreakpoint(const Breakpoint &bp)
-{
-    DapEngine::insertBreakpoint(bp);
-    notifyBreakpointInsertOk(bp); // Needed for Python support issue:1386
-}
-
-void PyDapEngine::removeBreakpoint(const Breakpoint &bp)
-{
-    DapEngine::removeBreakpoint(bp);
-    notifyBreakpointRemoveOk(bp); // Needed for Python support issue:1386
-}
-
-void PyDapEngine::updateBreakpoint(const Breakpoint &bp)
-{
-    DapEngine::updateBreakpoint(bp);
-
-    /* Needed for Python support issue:1386 */
-    BreakpointParameters parameters = bp->requestedParameters();
-    if (parameters.enabled != bp->isEnabled()) {
-        parameters.pending = false;
-        bp->setParameters(parameters);
-    }
-    notifyBreakpointChangeOk(bp);
-    /* Needed for Python support issue:1386 */
-}
-
 bool PyDapEngine::isLocalAttachEngine() const
 {
     return runParameters().startMode == AttachToLocalProcess;

src/plugins/debugger/dap/pydapengine.h

@@ -20,11 +20,6 @@ private:
     bool isLocalAttachEngine() const;
 
     bool acceptsBreakpoint(const BreakpointParameters &bp) const override;
-    /* Needed for Python support issue:1386 */
-    void insertBreakpoint(const Breakpoint &bp) override;
-    void updateBreakpoint(const Breakpoint &bp) override;
-    void removeBreakpoint(const Breakpoint &bp) override;
-    /* Needed for Python support issue:1386 */
 
     const QLoggingCategory &logCategory() override;