[lldb] Convert Breakpoint & Watchpoints structs to classes (NFC) (#133780)

Convert Breakpoint & Watchpoints structs to classes to provide proper
access control. This is in preparation for adopting SBMutex to protect
the underlying SBBreakpoint and SBWatchpoint.

Author: JDevlieghere

lldb/tools/lldb-dap/Breakpoint.cpp

@@ -19,21 +19,21 @@
 
 using namespace lldb_dap;
 
-void Breakpoint::SetCondition() { bp.SetCondition(condition.c_str()); }
+void Breakpoint::SetCondition() { m_bp.SetCondition(m_condition.c_str()); }
 
 void Breakpoint::SetHitCondition() {
   uint64_t hitCount = 0;
-  if (llvm::to_integer(hitCondition, hitCount))
-    bp.SetIgnoreCount(hitCount - 1);
+  if (llvm::to_integer(m_hit_condition, hitCount))
+    m_bp.SetIgnoreCount(hitCount - 1);
 }
 
 void Breakpoint::CreateJsonObject(llvm::json::Object &object) {
   // Each breakpoint location is treated as a separate breakpoint for VS code.
   // They don't have the notion of a single breakpoint with multiple locations.
-  if (!bp.IsValid())
+  if (!m_bp.IsValid())
     return;
-  object.try_emplace("verified", bp.GetNumResolvedLocations() > 0);
-  object.try_emplace("id", bp.GetID());
+  object.try_emplace("verified", m_bp.GetNumResolvedLocations() > 0);
+  object.try_emplace("id", m_bp.GetID());
   // VS Code DAP doesn't currently allow one breakpoint to have multiple
   // locations so we just report the first one. If we report all locations
   // then the IDE starts showing the wrong line numbers and locations for
@@ -43,20 +43,20 @@ void Breakpoint::CreateJsonObject(llvm::json::Object &object) {
   // this as the breakpoint location since it will have a complete location
   // that is at least loaded in the current process.
   lldb::SBBreakpointLocation bp_loc;
-  const auto num_locs = bp.GetNumLocations();
+  const auto num_locs = m_bp.GetNumLocations();
   for (size_t i = 0; i < num_locs; ++i) {
-    bp_loc = bp.GetLocationAtIndex(i);
+    bp_loc = m_bp.GetLocationAtIndex(i);
     if (bp_loc.IsResolved())
       break;
   }
   // If not locations are resolved, use the first location.
   if (!bp_loc.IsResolved())
-    bp_loc = bp.GetLocationAtIndex(0);
+    bp_loc = m_bp.GetLocationAtIndex(0);
   auto bp_addr = bp_loc.GetAddress();
 
   if (bp_addr.IsValid()) {
     std::string formatted_addr =
-        "0x" + llvm::utohexstr(bp_addr.GetLoadAddress(bp.GetTarget()));
+        "0x" + llvm::utohexstr(bp_addr.GetLoadAddress(m_bp.GetTarget()));
     object.try_emplace("instructionReference", formatted_addr);
     auto line_entry = bp_addr.GetLineEntry();
     const auto line = line_entry.GetLine();
@@ -69,12 +69,14 @@ void Breakpoint::CreateJsonObject(llvm::json::Object &object) {
   }
 }
 
-bool Breakpoint::MatchesName(const char *name) { return bp.MatchesName(name); }
+bool Breakpoint::MatchesName(const char *name) {
+  return m_bp.MatchesName(name);
+}
 
 void Breakpoint::SetBreakpoint() {
-  bp.AddName(kDAPBreakpointLabel);
-  if (!condition.empty())
+  m_bp.AddName(kDAPBreakpointLabel);
+  if (!m_condition.empty())
     SetCondition();
-  if (!hitCondition.empty())
+  if (!m_hit_condition.empty())
     SetHitCondition();
 }

lldb/tools/lldb-dap/Breakpoint.h

@@ -15,19 +15,23 @@
 
 namespace lldb_dap {
 
-struct Breakpoint : public BreakpointBase {
-  // The LLDB breakpoint associated wit this source breakpoint
-  lldb::SBBreakpoint bp;
-
+class Breakpoint : public BreakpointBase {
+public:
   Breakpoint(DAP &d, const llvm::json::Object &obj) : BreakpointBase(d, obj) {}
-  Breakpoint(DAP &d, lldb::SBBreakpoint bp) : BreakpointBase(d), bp(bp) {}
+  Breakpoint(DAP &d, lldb::SBBreakpoint bp) : BreakpointBase(d), m_bp(bp) {}
+
+  lldb::break_id_t GetID() const { return m_bp.GetID(); }
 
   void SetCondition() override;
   void SetHitCondition() override;
   void CreateJsonObject(llvm::json::Object &object) override;
 
   bool MatchesName(const char *name);
   void SetBreakpoint();
+
+protected:
+  /// The LLDB breakpoint associated wit this source breakpoint.
+  lldb::SBBreakpoint m_bp;
 };
 } // namespace lldb_dap
 

lldb/tools/lldb-dap/BreakpointBase.cpp

@@ -13,16 +13,18 @@
 using namespace lldb_dap;
 
 BreakpointBase::BreakpointBase(DAP &d, const llvm::json::Object &obj)
-    : dap(d), condition(std::string(GetString(obj, "condition").value_or(""))),
-      hitCondition(std::string(GetString(obj, "hitCondition").value_or(""))) {}
+    : m_dap(d),
+      m_condition(std::string(GetString(obj, "condition").value_or(""))),
+      m_hit_condition(
+          std::string(GetString(obj, "hitCondition").value_or(""))) {}
 
 void BreakpointBase::UpdateBreakpoint(const BreakpointBase &request_bp) {
-  if (condition != request_bp.condition) {
-    condition = request_bp.condition;
+  if (m_condition != request_bp.m_condition) {
+    m_condition = request_bp.m_condition;
     SetCondition();
   }
-  if (hitCondition != request_bp.hitCondition) {
-    hitCondition = request_bp.hitCondition;
+  if (m_hit_condition != request_bp.m_hit_condition) {
+    m_hit_condition = request_bp.m_hit_condition;
     SetHitCondition();
   }
 }

lldb/tools/lldb-dap/BreakpointBase.h

@@ -15,17 +15,9 @@
 
 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;
-
-  explicit BreakpointBase(DAP &d) : dap(d) {}
+class BreakpointBase {
+public:
+  explicit BreakpointBase(DAP &d) : m_dap(d) {}
   BreakpointBase(DAP &d, const llvm::json::Object &obj);
   virtual ~BreakpointBase() = default;
 
@@ -49,6 +41,17 @@ struct BreakpointBase {
   /// breakpoint in one of the DAP breakpoints that we should report changes
   /// for.
   static constexpr const char *kDAPBreakpointLabel = "dap";
+
+protected:
+  /// Associated DAP session.
+  DAP &m_dap;
+
+  /// An optional expression for conditional breakpoints.
+  std::string m_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 m_hit_condition;
 };
 
 } // namespace lldb_dap

lldb/tools/lldb-dap/DAP.cpp

@@ -162,7 +162,7 @@ void DAP::PopulateExceptionBreakpoints() {
   });
 }
 
-ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const std::string &filter) {
+ExceptionBreakpoint *DAP::GetExceptionBreakpoint(llvm::StringRef filter) {
   // PopulateExceptionBreakpoints() is called after g_dap.debugger is created
   // in a request-initialize.
   //
@@ -181,7 +181,7 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const std::string &filter) {
   PopulateExceptionBreakpoints();
 
   for (auto &bp : *exception_breakpoints) {
-    if (bp.filter == filter)
+    if (bp.GetFilter() == filter)
       return &bp;
   }
   return nullptr;
@@ -192,7 +192,7 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const lldb::break_id_t bp_id) {
   PopulateExceptionBreakpoints();
 
   for (auto &bp : *exception_breakpoints) {
-    if (bp.bp.GetID() == bp_id)
+    if (bp.GetID() == bp_id)
       return &bp;
   }
   return nullptr;
@@ -1066,7 +1066,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.bp.GetID() == bp_id)
+    if (bp.second.GetID() == bp_id)
       return &bp.second;
   }
   return nullptr;

lldb/tools/lldb-dap/DAP.h

@@ -236,7 +236,7 @@ struct DAP {
   void operator=(const DAP &rhs) = delete;
   /// @}
 
-  ExceptionBreakpoint *GetExceptionBreakpoint(const std::string &filter);
+  ExceptionBreakpoint *GetExceptionBreakpoint(llvm::StringRef filter);
   ExceptionBreakpoint *GetExceptionBreakpoint(const lldb::break_id_t bp_id);
 
   /// Redirect stdout and stderr fo the IDE's console output.

lldb/tools/lldb-dap/DAPForward.h

@@ -12,16 +12,16 @@
 // IWYU pragma: begin_exports
 
 namespace lldb_dap {
-struct BreakpointBase;
-struct ExceptionBreakpoint;
-struct FunctionBreakpoint;
-struct SourceBreakpoint;
-struct Watchpoint;
-struct InstructionBreakpoint;
-struct DAP;
-class Log;
 class BaseRequestHandler;
+class BreakpointBase;
+class ExceptionBreakpoint;
+class FunctionBreakpoint;
+class InstructionBreakpoint;
+class Log;
 class ResponseHandler;
+class SourceBreakpoint;
+class Watchpoint;
+struct DAP;
 } // namespace lldb_dap
 
 namespace lldb {

lldb/tools/lldb-dap/ExceptionBreakpoint.cpp

@@ -14,20 +14,20 @@
 namespace lldb_dap {
 
 void ExceptionBreakpoint::SetBreakpoint() {
-  if (bp.IsValid())
+  if (m_bp.IsValid())
     return;
-  bool catch_value = filter.find("_catch") != std::string::npos;
-  bool throw_value = filter.find("_throw") != std::string::npos;
-  bp = dap.target.BreakpointCreateForException(language, catch_value,
-                                               throw_value);
-  bp.AddName(BreakpointBase::kDAPBreakpointLabel);
+  bool catch_value = m_filter.find("_catch") != std::string::npos;
+  bool throw_value = m_filter.find("_throw") != std::string::npos;
+  m_bp = m_dap.target.BreakpointCreateForException(m_language, catch_value,
+                                                   throw_value);
+  m_bp.AddName(BreakpointBase::kDAPBreakpointLabel);
 }
 
 void ExceptionBreakpoint::ClearBreakpoint() {
-  if (!bp.IsValid())
+  if (!m_bp.IsValid())
     return;
-  dap.target.BreakpointDelete(bp.GetID());
-  bp = lldb::SBBreakpoint();
+  m_dap.target.BreakpointDelete(m_bp.GetID());
+  m_bp = lldb::SBBreakpoint();
 }
 
 } // namespace lldb_dap

lldb/tools/lldb-dap/ExceptionBreakpoint.h

@@ -12,25 +12,34 @@
 #include "DAPForward.h"
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/lldb-enumerations.h"
+#include "llvm/ADT/StringRef.h"
 #include <string>
 #include <utility>
 
 namespace lldb_dap {
 
-struct ExceptionBreakpoint {
-  DAP &dap;
-  std::string filter;
-  std::string label;
-  lldb::LanguageType language;
-  bool default_value = false;
-  lldb::SBBreakpoint bp;
+class ExceptionBreakpoint {
+public:
   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() {}
+      : m_dap(d), m_filter(std::move(f)), m_label(std::move(l)),
+        m_language(lang), m_bp() {}
 
   void SetBreakpoint();
   void ClearBreakpoint();
+
+  lldb::break_id_t GetID() const { return m_bp.GetID(); }
+  llvm::StringRef GetFilter() const { return m_filter; }
+  llvm::StringRef GetLabel() const { return m_label; }
+
+  static constexpr bool kDefaultValue = false;
+
+protected:
+  DAP &m_dap;
+  std::string m_filter;
+  std::string m_label;
+  lldb::LanguageType m_language;
+  lldb::SBBreakpoint m_bp;
 };
 
 } // namespace lldb_dap

lldb/tools/lldb-dap/FunctionBreakpoint.cpp

@@ -14,12 +14,12 @@ namespace lldb_dap {
 
 FunctionBreakpoint::FunctionBreakpoint(DAP &d, const llvm::json::Object &obj)
     : Breakpoint(d, obj),
-      functionName(std::string(GetString(obj, "name").value_or(""))) {}
+      m_function_name(std::string(GetString(obj, "name").value_or(""))) {}
 
 void FunctionBreakpoint::SetBreakpoint() {
-  if (functionName.empty())
+  if (m_function_name.empty())
     return;
-  bp = dap.target.BreakpointCreateByName(functionName.c_str());
+  m_bp = m_dap.target.BreakpointCreateByName(m_function_name.c_str());
   Breakpoint::SetBreakpoint();
 }
 

lldb/tools/lldb-dap/FunctionBreakpoint.h

@@ -14,13 +14,17 @@
 
 namespace lldb_dap {
 
-struct FunctionBreakpoint : public Breakpoint {
-  std::string functionName;
-
+class FunctionBreakpoint : public Breakpoint {
+public:
   FunctionBreakpoint(DAP &dap, const llvm::json::Object &obj);
 
-  // Set this breakpoint in LLDB as a new breakpoint
+  /// Set this breakpoint in LLDB as a new breakpoint.
   void SetBreakpoint();
+
+  llvm::StringRef GetFunctionName() const { return m_function_name; }
+
+protected:
+  std::string m_function_name;
 };
 
 } // namespace lldb_dap

lldb/tools/lldb-dap/Handler/ExceptionInfoRequestHandler.cpp

@@ -125,8 +125,8 @@ void ExceptionInfoRequestHandler::operator()(
     else if (stopReason == lldb::eStopReasonBreakpoint) {
       ExceptionBreakpoint *exc_bp = dap.GetExceptionBPFromStopReason(thread);
       if (exc_bp) {
-        EmplaceSafeString(body, "exceptionId", exc_bp->filter);
-        EmplaceSafeString(body, "description", exc_bp->label);
+        EmplaceSafeString(body, "exceptionId", exc_bp->GetFilter());
+        EmplaceSafeString(body, "description", exc_bp->GetLabel());
       } else {
         body.try_emplace("exceptionId", "exception");
       }

lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp

@@ -143,7 +143,8 @@ void SetBreakpointsRequestHandler::operator()(
       const auto *bp_obj = bp.getAsObject();
       if (bp_obj) {
         SourceBreakpoint src_bp(dap, *bp_obj);
-        std::pair<uint32_t, uint32_t> bp_pos(src_bp.line, src_bp.column);
+        std::pair<uint32_t, uint32_t> bp_pos(src_bp.GetLine(),
+                                             src_bp.GetColumn());
         request_bps.try_emplace(bp_pos, src_bp);
         const auto [iv, inserted] =
             dap.source_breakpoints[path].try_emplace(bp_pos, src_bp);
@@ -153,7 +154,7 @@ void SetBreakpointsRequestHandler::operator()(
         else
           iv->getSecond().UpdateBreakpoint(src_bp);
         AppendBreakpoint(&iv->getSecond(), response_breakpoints, path,
-                         src_bp.line);
+                         src_bp.GetLine());
       }
     }
   }
@@ -167,7 +168,7 @@ void SetBreakpointsRequestHandler::operator()(
       auto request_pos = request_bps.find(old_bp.first);
       if (request_pos == request_bps.end()) {
         // This breakpoint no longer exists in this source file, delete it
-        dap.target.BreakpointDelete(old_bp.second.bp.GetID());
+        dap.target.BreakpointDelete(old_bp.second.GetID());
         old_src_bp_pos->second.erase(old_bp.first);
       }
     }

lldb/tools/lldb-dap/Handler/SetDataBreakpointsRequestHandler.cpp

@@ -97,9 +97,9 @@ void SetDataBreakpointsRequestHandler::operator()(
   // backward.
   std::set<lldb::addr_t> addresses;
   for (auto iter = watchpoints.rbegin(); iter != watchpoints.rend(); ++iter) {
-    if (addresses.count(iter->addr) == 0) {
+    if (addresses.count(iter->GetAddress()) == 0) {
       iter->SetWatchpoint();
-      addresses.insert(iter->addr);
+      addresses.insert(iter->GetAddress());
     }
   }
   for (auto wp : watchpoints)