[lldb-dap] Refactoring breakpoints to not use the g_dap reference.

ashgti · ashgti · commit 125c8976906f · 2024-11-06T12:51:02.000-08:00
Instead, when a breakpoint is constructed it will be passed a DAP reference that it should use for its lifetime.

This is part of a larger refactor to remove the global `g_dap` variable in favor of managing instances.
diff --git a/lldb/tools/lldb-dap/Breakpoint.cpp b/lldb/tools/lldb-dap/Breakpoint.cpp
@@ -7,11 +7,13 @@
 //===----------------------------------------------------------------------===//
 
 #include "Breakpoint.h"
-#include "DAP.h"
-#include "JSONUtils.h"
+
 #include "lldb/API/SBBreakpointLocation.h"
 #include "llvm/ADT/StringExtras.h"
 
+#include "DAP.h"
+#include "JSONUtils.h"
+
 using namespace lldb_dap;
 
 void Breakpoint::SetCondition() { bp.SetCondition(condition.c_str()); }
@@ -51,7 +53,7 @@ void Breakpoint::CreateJsonObject(llvm::json::Object &object) {
 
   if (bp_addr.IsValid()) {
     std::string formatted_addr =
-        "0x" + llvm::utohexstr(bp_addr.GetLoadAddress(g_dap.target));
+        "0x" + llvm::utohexstr(bp_addr.GetLoadAddress(bp.GetTarget()));
     object.try_emplace("instructionReference", formatted_addr);
     auto line_entry = bp_addr.GetLineEntry();
     const auto line = line_entry.GetLine();
diff --git a/lldb/tools/lldb-dap/Breakpoint.h b/lldb/tools/lldb-dap/Breakpoint.h
@@ -9,18 +9,19 @@
 #ifndef LLDB_TOOLS_LLDB_DAP_BREAKPOINT_H
 #define LLDB_TOOLS_LLDB_DAP_BREAKPOINT_H
 
-#include "BreakpointBase.h"
 #include "lldb/API/SBBreakpoint.h"
 
+#include "BreakpointBase.h"
+
 namespace lldb_dap {
 
 struct Breakpoint : public BreakpointBase {
   // The LLDB breakpoint associated wit this source breakpoint
   lldb::SBBreakpoint bp;
 
-  Breakpoint() = default;
-  Breakpoint(const llvm::json::Object &obj) : BreakpointBase(obj){};
-  Breakpoint(lldb::SBBreakpoint bp) : bp(bp) {}
+  Breakpoint(DAP &d) : BreakpointBase(d) {}
+  Breakpoint(DAP &d, const llvm::json::Object &obj) : BreakpointBase(d, obj) {}
+  Breakpoint(DAP &d, lldb::SBBreakpoint bp) : BreakpointBase(d), bp(bp) {}
 
   void SetCondition() override;
   void SetHitCondition() override;
diff --git a/lldb/tools/lldb-dap/BreakpointBase.cpp b/lldb/tools/lldb-dap/BreakpointBase.cpp
@@ -11,8 +11,8 @@
 
 using namespace lldb_dap;
 
-BreakpointBase::BreakpointBase(const llvm::json::Object &obj)
-    : condition(std::string(GetString(obj, "condition"))),
+BreakpointBase::BreakpointBase(DAP &d, const llvm::json::Object &obj)
+    : dap(d), condition(std::string(GetString(obj, "condition"))),
       hitCondition(std::string(GetString(obj, "hitCondition"))) {}
 
 void BreakpointBase::UpdateBreakpoint(const BreakpointBase &request_bp) {
diff --git a/lldb/tools/lldb-dap/BreakpointBase.h b/lldb/tools/lldb-dap/BreakpointBase.h
@@ -9,21 +9,27 @@
 #ifndef LLDB_TOOLS_LLDB_DAP_BREAKPOINTBASE_H
 #define LLDB_TOOLS_LLDB_DAP_BREAKPOINTBASE_H
 
-#include "llvm/Support/JSON.h"
 #include <string>
 
+#include "llvm/Support/JSON.h"
+
+#include "DAPForward.h"
+
 namespace lldb_dap {
 
 struct BreakpointBase {
+  // Associated DAP session.
+  DAP &dap;
 
   // An optional expression for conditional breakpoints.
   std::string condition;
   // An optional expression that controls how many hits of the breakpoint are
   // ignored. The backend is expected to interpret the expression as needed
   std::string hitCondition;
 
-  BreakpointBase() = default;
-  BreakpointBase(const llvm::json::Object &obj);
+  BreakpointBase(DAP &d) : dap(d) {}
+  BreakpointBase(DAP &d, const llvm::json::Object &obj);
+  BreakpointBase(const BreakpointBase &) = default;
   virtual ~BreakpointBase() = default;
 
   virtual void SetCondition() = 0;
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
@@ -74,21 +74,21 @@ void DAP::PopulateExceptionBreakpoints() {
     exception_breakpoints = std::vector<ExceptionBreakpoint>{};
 
     if (lldb::SBDebugger::SupportsLanguage(lldb::eLanguageTypeC_plus_plus)) {
-      exception_breakpoints->emplace_back("cpp_catch", "C++ Catch",
+      exception_breakpoints->emplace_back(*this, "cpp_catch", "C++ Catch",
                                           lldb::eLanguageTypeC_plus_plus);
-      exception_breakpoints->emplace_back("cpp_throw", "C++ Throw",
+      exception_breakpoints->emplace_back(*this, "cpp_throw", "C++ Throw",
                                           lldb::eLanguageTypeC_plus_plus);
     }
     if (lldb::SBDebugger::SupportsLanguage(lldb::eLanguageTypeObjC)) {
-      exception_breakpoints->emplace_back("objc_catch", "Objective-C Catch",
-                                          lldb::eLanguageTypeObjC);
-      exception_breakpoints->emplace_back("objc_throw", "Objective-C Throw",
-                                          lldb::eLanguageTypeObjC);
+      exception_breakpoints->emplace_back(
+          *this, "objc_catch", "Objective-C Catch", lldb::eLanguageTypeObjC);
+      exception_breakpoints->emplace_back(
+          *this, "objc_throw", "Objective-C Throw", lldb::eLanguageTypeObjC);
     }
     if (lldb::SBDebugger::SupportsLanguage(lldb::eLanguageTypeSwift)) {
-      exception_breakpoints->emplace_back("swift_catch", "Swift Catch",
+      exception_breakpoints->emplace_back(*this, "swift_catch", "Swift Catch",
                                           lldb::eLanguageTypeSwift);
-      exception_breakpoints->emplace_back("swift_throw", "Swift Throw",
+      exception_breakpoints->emplace_back(*this, "swift_throw", "Swift Throw",
                                           lldb::eLanguageTypeSwift);
     }
     // Besides handling the hardcoded list of languages from above, we try to
@@ -119,7 +119,7 @@ void DAP::PopulateExceptionBreakpoints() {
             raw_throw_keyword ? raw_throw_keyword : "throw";
 
         exception_breakpoints->emplace_back(
-            raw_lang_name + "_" + throw_keyword,
+            *this, raw_lang_name + "_" + throw_keyword,
             capitalized_lang_name + " " + capitalize(throw_keyword), lang);
       }
 
@@ -130,7 +130,7 @@ void DAP::PopulateExceptionBreakpoints() {
             raw_catch_keyword ? raw_catch_keyword : "catch";
 
         exception_breakpoints->emplace_back(
-            raw_lang_name + "_" + catch_keyword,
+            *this, raw_lang_name + "_" + catch_keyword,
             capitalized_lang_name + " " + capitalize(catch_keyword), lang);
       }
     }
@@ -1060,7 +1060,7 @@ void DAP::SetThreadFormat(llvm::StringRef format) {
 InstructionBreakpoint *
 DAP::GetInstructionBreakpoint(const lldb::break_id_t bp_id) {
   for (auto &bp : instruction_breakpoints) {
-    if (bp.second.id == bp_id)
+    if (bp.second.bp.GetID() == bp_id)
       return &bp.second;
   }
   return nullptr;
diff --git a/lldb/tools/lldb-dap/DAPForward.h b/lldb/tools/lldb-dap/DAPForward.h
@@ -16,6 +16,7 @@ struct FunctionBreakpoint;
 struct SourceBreakpoint;
 struct Watchpoint;
 struct InstructionBreakpoint;
+struct DAP;
 } // namespace lldb_dap
 
 namespace lldb {
diff --git a/lldb/tools/lldb-dap/ExceptionBreakpoint.cpp b/lldb/tools/lldb-dap/ExceptionBreakpoint.cpp
@@ -17,8 +17,8 @@ void ExceptionBreakpoint::SetBreakpoint() {
     return;
   bool catch_value = filter.find("_catch") != std::string::npos;
   bool throw_value = filter.find("_throw") != std::string::npos;
-  bp = g_dap.target.BreakpointCreateForException(language, catch_value,
-                                                 throw_value);
+  bp = dap.target.BreakpointCreateForException(language, catch_value,
+                                               throw_value);
   // See comments in BreakpointBase::GetBreakpointLabel() for details of why
   // we add a label to our breakpoints.
   bp.AddName(BreakpointBase::GetBreakpointLabel());
@@ -27,7 +27,7 @@ void ExceptionBreakpoint::SetBreakpoint() {
 void ExceptionBreakpoint::ClearBreakpoint() {
   if (!bp.IsValid())
     return;
-  g_dap.target.BreakpointDelete(bp.GetID());
+  dap.target.BreakpointDelete(bp.GetID());
   bp = lldb::SBBreakpoint();
 }
 
diff --git a/lldb/tools/lldb-dap/ExceptionBreakpoint.h b/lldb/tools/lldb-dap/ExceptionBreakpoint.h
@@ -13,17 +13,21 @@
 
 #include "lldb/API/SBBreakpoint.h"
 
+#include "DAPForward.h"
+
 namespace lldb_dap {
 
 struct ExceptionBreakpoint {
+  DAP &dap;
   std::string filter;
   std::string label;
   lldb::LanguageType language;
-  bool default_value;
+  bool default_value = false;
   lldb::SBBreakpoint bp;
-  ExceptionBreakpoint(std::string f, std::string l, lldb::LanguageType lang)
-      : filter(std::move(f)), label(std::move(l)), language(lang),
-        default_value(false), bp() {}
+  ExceptionBreakpoint(DAP &d, std::string f, std::string l,
+                      lldb::LanguageType lang)
+      : dap(d), filter(std::move(f)), label(std::move(l)), language(lang),
+        bp() {}
 
   void SetBreakpoint();
   void ClearBreakpoint();
diff --git a/lldb/tools/lldb-dap/FunctionBreakpoint.cpp b/lldb/tools/lldb-dap/FunctionBreakpoint.cpp
@@ -7,18 +7,19 @@
 //===----------------------------------------------------------------------===//
 
 #include "FunctionBreakpoint.h"
+
 #include "DAP.h"
 #include "JSONUtils.h"
 
 namespace lldb_dap {
 
-FunctionBreakpoint::FunctionBreakpoint(const llvm::json::Object &obj)
-    : Breakpoint(obj), functionName(std::string(GetString(obj, "name"))) {}
+FunctionBreakpoint::FunctionBreakpoint(DAP &d, const llvm::json::Object &obj)
+    : Breakpoint(d, obj), functionName(std::string(GetString(obj, "name"))) {}
 
 void FunctionBreakpoint::SetBreakpoint() {
   if (functionName.empty())
     return;
-  bp = g_dap.target.BreakpointCreateByName(functionName.c_str());
+  bp = dap.target.BreakpointCreateByName(functionName.c_str());
   Breakpoint::SetBreakpoint();
 }
 
diff --git a/lldb/tools/lldb-dap/FunctionBreakpoint.h b/lldb/tools/lldb-dap/FunctionBreakpoint.h
@@ -10,14 +10,14 @@
 #define LLDB_TOOLS_LLDB_DAP_FUNCTIONBREAKPOINT_H
 
 #include "Breakpoint.h"
+#include "DAPForward.h"
 
 namespace lldb_dap {
 
 struct FunctionBreakpoint : public Breakpoint {
   std::string functionName;
 
-  FunctionBreakpoint() = default;
-  FunctionBreakpoint(const llvm::json::Object &obj);
+  FunctionBreakpoint(DAP &dap, const llvm::json::Object &obj);
 
   // Set this breakpoint in LLDB as a new breakpoint
   void SetBreakpoint();
diff --git a/lldb/tools/lldb-dap/InstructionBreakpoint.cpp b/lldb/tools/lldb-dap/InstructionBreakpoint.cpp
@@ -8,22 +8,25 @@
 //===----------------------------------------------------------------------===//
 
 #include "InstructionBreakpoint.h"
+
 #include "DAP.h"
 #include "JSONUtils.h"
 
 namespace lldb_dap {
 
 // Instruction Breakpoint
-InstructionBreakpoint::InstructionBreakpoint(const llvm::json::Object &obj)
-    : Breakpoint(obj), instructionAddressReference(LLDB_INVALID_ADDRESS), id(0),
+InstructionBreakpoint::InstructionBreakpoint(DAP &d,
+                                             const llvm::json::Object &obj)
+    : Breakpoint(d, obj), instructionAddressReference(LLDB_INVALID_ADDRESS),
       offset(GetSigned(obj, "offset", 0)) {
   GetString(obj, "instructionReference")
       .getAsInteger(0, instructionAddressReference);
   instructionAddressReference += offset;
 }
 
-void InstructionBreakpoint::SetInstructionBreakpoint() {
-  bp = g_dap.target.BreakpointCreateByAddress(instructionAddressReference);
-  id = bp.GetID();
+void InstructionBreakpoint::SetBreakpoint() {
+  bp = dap.target.BreakpointCreateByAddress(instructionAddressReference);
+  Breakpoint::SetBreakpoint();
 }
+
 } // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/InstructionBreakpoint.h b/lldb/tools/lldb-dap/InstructionBreakpoint.h
@@ -10,24 +10,28 @@
 #ifndef LLDB_TOOLS_LLDB_DAP_INSTRUCTIONBREAKPOINT_H
 #define LLDB_TOOLS_LLDB_DAP_INSTRUCTIONBREAKPOINT_H
 
+#include <cstdint>
+
+#include "lldb/lldb-types.h"
+
 #include "Breakpoint.h"
+#include "DAPForward.h"
 
 namespace lldb_dap {
 
 // Instruction Breakpoint
 struct InstructionBreakpoint : public Breakpoint {
 
   lldb::addr_t instructionAddressReference;
-  int32_t id;
   int32_t offset;
 
-  InstructionBreakpoint()
-      : Breakpoint(), instructionAddressReference(LLDB_INVALID_ADDRESS), id(0),
+  InstructionBreakpoint(DAP &d)
+      : Breakpoint(d), instructionAddressReference(LLDB_INVALID_ADDRESS),
         offset(0) {}
-  InstructionBreakpoint(const llvm::json::Object &obj);
+  InstructionBreakpoint(DAP &d, const llvm::json::Object &obj);
 
   // Set instruction breakpoint in LLDB as a new breakpoint
-  void SetInstructionBreakpoint();
+  void SetBreakpoint();
 };
 
 } // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -831,70 +831,6 @@ llvm::json::Value CreateExtendedStackFrameLabel(lldb::SBThread &thread) {
                                               {"presentationHint", "label"}});
 }
 
-// Response to `setInstructionBreakpoints` request.
-// "Breakpoint": {
-//   "type": "object",
-//   "description": "Response to `setInstructionBreakpoints` request.",
-//   "properties": {
-//     "id": {
-//       "type": "number",
-//       "description": "The identifier for the breakpoint. It is needed if
-//       breakpoint events are used to update or remove breakpoints."
-//     },
-//     "verified": {
-//       "type": "boolean",
-//       "description": "If true, the breakpoint could be set (but not
-//       necessarily at the desired location."
-//     },
-//     "message": {
-//       "type": "string",
-//       "description": "A message about the state of the breakpoint.
-//       This is shown to the user and can be used to explain why a breakpoint
-//       could not be verified."
-//     },
-//     "source": {
-//       "type": "Source",
-//       "description": "The source where the breakpoint is located."
-//     },
-//     "line": {
-//       "type": "number",
-//       "description": "The start line of the actual range covered by the
-//       breakpoint."
-//     },
-//     "column": {
-//       "type": "number",
-//       "description": "The start column of the actual range covered by the
-//       breakpoint."
-//     },
-//     "endLine": {
-//       "type": "number",
-//       "description": "The end line of the actual range covered by the
-//       breakpoint."
-//     },
-//     "endColumn": {
-//       "type": "number",
-//       "description": "The end column of the actual range covered by the
-//       breakpoint. If no end line is given, then the end column is assumed to
-//       be in the start line."
-//     },
-//     "instructionReference": {
-//       "type": "string",
-//       "description": "A memory reference to where the breakpoint is set."
-//     },
-//     "offset": {
-//       "type": "number",
-//       "description": "The offset from the instruction reference.
-//       This can be negative."
-//     },
-//   },
-//   "required": [ "id", "verified", "line"]
-// }
-llvm::json::Value CreateInstructionBreakpoint(BreakpointBase *ibp) {
-  llvm::json::Object object;
-  ibp->CreateJsonObject(object);
-  return llvm::json::Value(std::move(object));
-}
-
 // "Thread": {
 //   "type": "object",
 //   "description": "A Thread",
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
@@ -380,17 +380,6 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame);
 ///     definition outlined by Microsoft.
 llvm::json::Value CreateExtendedStackFrameLabel(lldb::SBThread &thread);
 
-/// Create a "instruction" object for a LLDB disassemble object as described in
-/// the Visual Studio Code debug adaptor definition.
-///
-/// \param[in] bp
-///     The LLDB instruction object used to populate the disassembly
-///     instruction.
-/// \return
-///     A "Scope" JSON object with that follows the formal JSON
-///     definition outlined by Microsoft.
-llvm::json::Value CreateInstructionBreakpoint(BreakpointBase *ibp);
-
 /// Create a "Thread" object for a LLDB thread object.
 ///
 /// This function will fill in the following keys in the returned
diff --git a/lldb/tools/lldb-dap/SourceBreakpoint.cpp b/lldb/tools/lldb-dap/SourceBreakpoint.cpp
diff --git a/lldb/tools/lldb-dap/SourceBreakpoint.h b/lldb/tools/lldb-dap/SourceBreakpoint.h
diff --git a/lldb/tools/lldb-dap/Watchpoint.cpp b/lldb/tools/lldb-dap/Watchpoint.cpp
diff --git a/lldb/tools/lldb-dap/Watchpoint.h b/lldb/tools/lldb-dap/Watchpoint.h
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
