Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lldb-dap][NFC] Add Breakpoint struct to share common logic. #80753

Merged
merged 3 commits into from
Feb 13, 2024

Conversation

ZequanWu
Copy link
Contributor

@ZequanWu ZequanWu commented Feb 5, 2024

This adds a layer between SounceBreakpoint/FunctionBreakpoint and BreakpointBase to have better separation and encapsulation so we are not directly operating on SBBreakpoint.

I basically moved the SBBreakpoint and the methods that requires it from BreakpointBase to Breakpoint. This allows adding support for data watchpoint easier by sharing the logic inside BreakpointBase.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 5, 2024

@llvm/pr-subscribers-lldb

Author: Zequan Wu (ZequanWu)

Changes

This adds a layer between SounceBreakpoint/FunctionBreakpoint and BreakpointBase to have better separation and encapsulation so we are not directly operating on SBBreakpoint.

I basically moved the SBBreakpoint and the methods that requires it from BreakpointBase to Breakpoint. This allows adding support for data watchpoint easier by sharing the logic inside BreakpointBase.


Patch is 27.36 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/80753.diff

13 Files Affected:

  • (added) lldb/tools/lldb-dap/Breakpoint.cpp (+182)
  • (added) lldb/tools/lldb-dap/Breakpoint.h (+34)
  • (modified) lldb/tools/lldb-dap/BreakpointBase.cpp (-113)
  • (modified) lldb/tools/lldb-dap/BreakpointBase.h (+6-6)
  • (modified) lldb/tools/lldb-dap/CMakeLists.txt (+1)
  • (modified) lldb/tools/lldb-dap/FunctionBreakpoint.cpp (+2-10)
  • (modified) lldb/tools/lldb-dap/FunctionBreakpoint.h (+2-2)
  • (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+3-43)
  • (modified) lldb/tools/lldb-dap/JSONUtils.h (+3-2)
  • (modified) lldb/tools/lldb-dap/SourceBreakpoint.cpp (+2-10)
  • (modified) lldb/tools/lldb-dap/SourceBreakpoint.h (+3-3)
  • (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+9-8)
  • (modified) llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn (+1)
diff --git a/lldb/tools/lldb-dap/Breakpoint.cpp b/lldb/tools/lldb-dap/Breakpoint.cpp
new file mode 100644
index 0000000000000..4ccf353b06ccc
--- /dev/null
+++ b/lldb/tools/lldb-dap/Breakpoint.cpp
@@ -0,0 +1,182 @@
+//===-- Breakpoint.cpp ------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Breakpoint.h"
+#include "DAP.h"
+#include "JSONUtils.h"
+#include "llvm/ADT/StringExtras.h"
+
+using namespace lldb_dap;
+
+void Breakpoint::SetCondition() { bp.SetCondition(condition.c_str()); }
+
+void Breakpoint::SetHitCondition() {
+  uint64_t hitCount = 0;
+  if (llvm::to_integer(hitCondition, hitCount))
+    bp.SetIgnoreCount(hitCount - 1);
+}
+
+// logMessage will be divided into array of LogMessagePart as two kinds:
+// 1. raw print text message, and
+// 2. interpolated expression for evaluation which is inside matching curly
+//    braces.
+//
+// The function tries to parse logMessage into a list of LogMessageParts
+// for easy later access in BreakpointHitCallback.
+void Breakpoint::SetLogMessage() {
+  logMessageParts.clear();
+
+  // Contains unmatched open curly braces indices.
+  std::vector<int> unmatched_curly_braces;
+
+  // Contains all matched curly braces in logMessage.
+  // Loop invariant: matched_curly_braces_ranges are sorted by start index in
+  // ascending order without any overlap between them.
+  std::vector<std::pair<int, int>> matched_curly_braces_ranges;
+
+  lldb::SBError error;
+  // Part1 - parse matched_curly_braces_ranges.
+  // locating all curly braced expression ranges in logMessage.
+  // The algorithm takes care of nested and imbalanced curly braces.
+  for (size_t i = 0; i < logMessage.size(); ++i) {
+    if (logMessage[i] == '{') {
+      unmatched_curly_braces.push_back(i);
+    } else if (logMessage[i] == '}') {
+      if (unmatched_curly_braces.empty())
+        // Nothing to match.
+        continue;
+
+      int last_unmatched_index = unmatched_curly_braces.back();
+      unmatched_curly_braces.pop_back();
+
+      // Erase any matched ranges included in the new match.
+      while (!matched_curly_braces_ranges.empty()) {
+        assert(matched_curly_braces_ranges.back().first !=
+                   last_unmatched_index &&
+               "How can a curley brace be matched twice?");
+        if (matched_curly_braces_ranges.back().first < last_unmatched_index)
+          break;
+
+        // This is a nested range let's earse it.
+        assert((size_t)matched_curly_braces_ranges.back().second < i);
+        matched_curly_braces_ranges.pop_back();
+      }
+
+      // Assert invariant.
+      assert(matched_curly_braces_ranges.empty() ||
+             matched_curly_braces_ranges.back().first < last_unmatched_index);
+      matched_curly_braces_ranges.emplace_back(last_unmatched_index, i);
+    }
+  }
+
+  // Part2 - parse raw text and expresions parts.
+  // All expression ranges have been parsed in matched_curly_braces_ranges.
+  // The code below uses matched_curly_braces_ranges to divide logMessage
+  // into raw text parts and expression parts.
+  int last_raw_text_start = 0;
+  for (const std::pair<int, int> &curly_braces_range :
+       matched_curly_braces_ranges) {
+    // Raw text before open curly brace.
+    assert(curly_braces_range.first >= last_raw_text_start);
+    size_t raw_text_len = curly_braces_range.first - last_raw_text_start;
+    if (raw_text_len > 0) {
+      error = AppendLogMessagePart(
+          llvm::StringRef(logMessage.c_str() + last_raw_text_start,
+                          raw_text_len),
+          /*is_expr=*/false);
+      if (error.Fail()) {
+        NotifyLogMessageError(error.GetCString());
+        return;
+      }
+    }
+
+    // Expression between curly braces.
+    assert(curly_braces_range.second > curly_braces_range.first);
+    size_t expr_len = curly_braces_range.second - curly_braces_range.first - 1;
+    error = AppendLogMessagePart(
+        llvm::StringRef(logMessage.c_str() + curly_braces_range.first + 1,
+                        expr_len),
+        /*is_expr=*/true);
+    if (error.Fail()) {
+      NotifyLogMessageError(error.GetCString());
+      return;
+    }
+
+    last_raw_text_start = curly_braces_range.second + 1;
+  }
+  // Trailing raw text after close curly brace.
+  assert(last_raw_text_start >= 0);
+  if (logMessage.size() > (size_t)last_raw_text_start) {
+    error = AppendLogMessagePart(
+        llvm::StringRef(logMessage.c_str() + last_raw_text_start,
+                        logMessage.size() - last_raw_text_start),
+        /*is_expr=*/false);
+    if (error.Fail()) {
+      NotifyLogMessageError(error.GetCString());
+      return;
+    }
+  }
+
+  bp.SetCallback(BreakpointBase::BreakpointHitCallback, this);
+}
+
+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())
+    return;
+  object.try_emplace("verified", bp.GetNumResolvedLocations() > 0);
+  object.try_emplace("id", 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
+  // other source file and line breakpoints in the same file.
+
+  // Below we search for the first resolved location in a breakpoint and report
+  // 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();
+  for (size_t i = 0; i < num_locs; ++i) {
+    bp_loc = 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);
+  auto bp_addr = bp_loc.GetAddress();
+
+  if (bp_addr.IsValid()) {
+    std::string formatted_addr =
+        "0x" + llvm::utohexstr(bp_addr.GetLoadAddress(g_dap.target));
+    object.try_emplace("instructionReference", formatted_addr);
+    auto line_entry = bp_addr.GetLineEntry();
+    const auto line = line_entry.GetLine();
+    if (line != UINT32_MAX)
+      object.try_emplace("line", line);
+    const auto column = line_entry.GetColumn();
+    if (column != 0)
+      object.try_emplace("column", column);
+    object.try_emplace("source", CreateSource(line_entry));
+  }
+}
+
+bool Breakpoint::MatchesName(const char *name) { return bp.MatchesName(name); }
+
+void Breakpoint::SetBreakpoint() {
+  // See comments in BreakpointBase::GetBreakpointLabel() for details of why
+  // we add a label to our breakpoints.
+  bp.AddName(GetBreakpointLabel());
+  if (!condition.empty())
+    SetCondition();
+  if (!hitCondition.empty())
+    SetHitCondition();
+  if (!logMessage.empty())
+    SetLogMessage();
+}
diff --git a/lldb/tools/lldb-dap/Breakpoint.h b/lldb/tools/lldb-dap/Breakpoint.h
new file mode 100644
index 0000000000000..a668e29f3d1e8
--- /dev/null
+++ b/lldb/tools/lldb-dap/Breakpoint.h
@@ -0,0 +1,34 @@
+//===-- Breakpoint.h --------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_TOOLS_LLDB_DAP_BREAKPOINT_H
+#define LLDB_TOOLS_LLDB_DAP_BREAKPOINT_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) {}
+
+  void SetCondition() override;
+  void SetHitCondition() override;
+  void SetLogMessage() override;
+  void CreateJsonObject(llvm::json::Object &object) override;
+
+  bool MatchesName(const char *name);
+  void SetBreakpoint();
+};
+} // namespace lldb_dap
+
+#endif
diff --git a/lldb/tools/lldb-dap/BreakpointBase.cpp b/lldb/tools/lldb-dap/BreakpointBase.cpp
index fb4b27fbe315f..088ccc6b27f8e 100644
--- a/lldb/tools/lldb-dap/BreakpointBase.cpp
+++ b/lldb/tools/lldb-dap/BreakpointBase.cpp
@@ -8,7 +8,6 @@
 
 #include "BreakpointBase.h"
 #include "DAP.h"
-#include "JSONUtils.h"
 #include "llvm/ADT/StringExtras.h"
 
 using namespace lldb_dap;
@@ -18,14 +17,6 @@ BreakpointBase::BreakpointBase(const llvm::json::Object &obj)
       hitCondition(std::string(GetString(obj, "hitCondition"))),
       logMessage(std::string(GetString(obj, "logMessage"))) {}
 
-void BreakpointBase::SetCondition() { bp.SetCondition(condition.c_str()); }
-
-void BreakpointBase::SetHitCondition() {
-  uint64_t hitCount = 0;
-  if (llvm::to_integer(hitCondition, hitCount))
-    bp.SetIgnoreCount(hitCount - 1);
-}
-
 lldb::SBError BreakpointBase::AppendLogMessagePart(llvm::StringRef part,
                                                    bool is_expr) {
   if (is_expr) {
@@ -164,110 +155,6 @@ lldb::SBError BreakpointBase::FormatLogText(llvm::StringRef text,
   return error;
 }
 
-// logMessage will be divided into array of LogMessagePart as two kinds:
-// 1. raw print text message, and
-// 2. interpolated expression for evaluation which is inside matching curly
-//    braces.
-//
-// The function tries to parse logMessage into a list of LogMessageParts
-// for easy later access in BreakpointHitCallback.
-void BreakpointBase::SetLogMessage() {
-  logMessageParts.clear();
-
-  // Contains unmatched open curly braces indices.
-  std::vector<int> unmatched_curly_braces;
-
-  // Contains all matched curly braces in logMessage.
-  // Loop invariant: matched_curly_braces_ranges are sorted by start index in
-  // ascending order without any overlap between them.
-  std::vector<std::pair<int, int>> matched_curly_braces_ranges;
-
-  lldb::SBError error;
-  // Part1 - parse matched_curly_braces_ranges.
-  // locating all curly braced expression ranges in logMessage.
-  // The algorithm takes care of nested and imbalanced curly braces.
-  for (size_t i = 0; i < logMessage.size(); ++i) {
-    if (logMessage[i] == '{') {
-      unmatched_curly_braces.push_back(i);
-    } else if (logMessage[i] == '}') {
-      if (unmatched_curly_braces.empty())
-        // Nothing to match.
-        continue;
-
-      int last_unmatched_index = unmatched_curly_braces.back();
-      unmatched_curly_braces.pop_back();
-
-      // Erase any matched ranges included in the new match.
-      while (!matched_curly_braces_ranges.empty()) {
-        assert(matched_curly_braces_ranges.back().first !=
-                   last_unmatched_index &&
-               "How can a curley brace be matched twice?");
-        if (matched_curly_braces_ranges.back().first < last_unmatched_index)
-          break;
-
-        // This is a nested range let's earse it.
-        assert((size_t)matched_curly_braces_ranges.back().second < i);
-        matched_curly_braces_ranges.pop_back();
-      }
-
-      // Assert invariant.
-      assert(matched_curly_braces_ranges.empty() ||
-             matched_curly_braces_ranges.back().first < last_unmatched_index);
-      matched_curly_braces_ranges.emplace_back(last_unmatched_index, i);
-    }
-  }
-
-  // Part2 - parse raw text and expresions parts.
-  // All expression ranges have been parsed in matched_curly_braces_ranges.
-  // The code below uses matched_curly_braces_ranges to divide logMessage
-  // into raw text parts and expression parts.
-  int last_raw_text_start = 0;
-  for (const std::pair<int, int> &curly_braces_range :
-       matched_curly_braces_ranges) {
-    // Raw text before open curly brace.
-    assert(curly_braces_range.first >= last_raw_text_start);
-    size_t raw_text_len = curly_braces_range.first - last_raw_text_start;
-    if (raw_text_len > 0) {
-      error = AppendLogMessagePart(
-          llvm::StringRef(logMessage.c_str() + last_raw_text_start,
-                          raw_text_len),
-          /*is_expr=*/false);
-      if (error.Fail()) {
-        NotifyLogMessageError(error.GetCString());
-        return;
-      }
-    }
-
-    // Expression between curly braces.
-    assert(curly_braces_range.second > curly_braces_range.first);
-    size_t expr_len = curly_braces_range.second - curly_braces_range.first - 1;
-    error = AppendLogMessagePart(
-        llvm::StringRef(logMessage.c_str() + curly_braces_range.first + 1,
-                        expr_len),
-        /*is_expr=*/true);
-    if (error.Fail()) {
-      NotifyLogMessageError(error.GetCString());
-      return;
-    }
-
-    last_raw_text_start = curly_braces_range.second + 1;
-  }
-  // Trailing raw text after close curly brace.
-  assert(last_raw_text_start >= 0);
-  if (logMessage.size() > (size_t)last_raw_text_start) {
-    error = AppendLogMessagePart(
-        llvm::StringRef(logMessage.c_str() + last_raw_text_start,
-                        logMessage.size() - last_raw_text_start),
-        /*is_expr=*/false);
-    if (error.Fail()) {
-      NotifyLogMessageError(error.GetCString());
-      return;
-    }
-  }
-
-  bp.SetCallback(BreakpointBase::BreakpointHitCallback, this);
-}
-
 void BreakpointBase::NotifyLogMessageError(llvm::StringRef error) {
   std::string message = "Log message has error: ";
   message += error;
diff --git a/lldb/tools/lldb-dap/BreakpointBase.h b/lldb/tools/lldb-dap/BreakpointBase.h
index 41787f7861021..a4184bfe65d27 100644
--- a/lldb/tools/lldb-dap/BreakpointBase.h
+++ b/lldb/tools/lldb-dap/BreakpointBase.h
@@ -9,7 +9,6 @@
 #ifndef LLDB_TOOLS_LLDB_DAP_BREAKPOINTBASE_H
 #define LLDB_TOOLS_LLDB_DAP_BREAKPOINTBASE_H
 
-#include "JSONUtils.h"
 #include "lldb/API/SBBreakpoint.h"
 #include "llvm/Support/JSON.h"
 #include <string>
@@ -35,15 +34,16 @@ struct BreakpointBase {
   // interpolated.
   std::string logMessage;
   std::vector<LogMessagePart> logMessageParts;
-  // The LLDB breakpoint associated wit this source breakpoint
-  lldb::SBBreakpoint bp;
 
   BreakpointBase() = default;
   BreakpointBase(const llvm::json::Object &obj);
+  virtual ~BreakpointBase() = default;
+
+  virtual void SetCondition() = 0;
+  virtual void SetHitCondition() = 0;
+  virtual void SetLogMessage() = 0;
+  virtual void CreateJsonObject(llvm::json::Object &object) = 0;
 
-  void SetCondition();
-  void SetHitCondition();
-  void SetLogMessage();
   void UpdateBreakpoint(const BreakpointBase &request_bp);
 
   // Format \param text and return formatted text in \param formatted.
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt
index 554567eb3b0e2..f8c0e4ecf36c2 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -24,6 +24,7 @@ tablegen(LLVM Options.inc -gen-opt-parser-defs)
 add_public_tablegen_target(LLDBDAPOptionsTableGen)
 add_lldb_tool(lldb-dap
   lldb-dap.cpp
+  Breakpoint.cpp
   BreakpointBase.cpp
   ExceptionBreakpoint.cpp
   FifoFiles.cpp
diff --git a/lldb/tools/lldb-dap/FunctionBreakpoint.cpp b/lldb/tools/lldb-dap/FunctionBreakpoint.cpp
index d4bdb976500ec..21743bf908706 100644
--- a/lldb/tools/lldb-dap/FunctionBreakpoint.cpp
+++ b/lldb/tools/lldb-dap/FunctionBreakpoint.cpp
@@ -12,21 +12,13 @@
 namespace lldb_dap {
 
 FunctionBreakpoint::FunctionBreakpoint(const llvm::json::Object &obj)
-    : BreakpointBase(obj), functionName(std::string(GetString(obj, "name"))) {}
+    : Breakpoint(obj), functionName(std::string(GetString(obj, "name"))) {}
 
 void FunctionBreakpoint::SetBreakpoint() {
   if (functionName.empty())
     return;
   bp = g_dap.target.BreakpointCreateByName(functionName.c_str());
-  // See comments in BreakpointBase::GetBreakpointLabel() for details of why
-  // we add a label to our breakpoints.
-  bp.AddName(GetBreakpointLabel());
-  if (!condition.empty())
-    SetCondition();
-  if (!hitCondition.empty())
-    SetHitCondition();
-  if (!logMessage.empty())
-    SetLogMessage();
+  Breakpoint::SetBreakpoint();
 }
 
 } // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/FunctionBreakpoint.h b/lldb/tools/lldb-dap/FunctionBreakpoint.h
index fc23e94e12876..b15ff1931a6b2 100644
--- a/lldb/tools/lldb-dap/FunctionBreakpoint.h
+++ b/lldb/tools/lldb-dap/FunctionBreakpoint.h
@@ -9,11 +9,11 @@
 #ifndef LLDB_TOOLS_LLDB_DAP_FUNCTIONBREAKPOINT_H
 #define LLDB_TOOLS_LLDB_DAP_FUNCTIONBREAKPOINT_H
 
-#include "BreakpointBase.h"
+#include "Breakpoint.h"
 
 namespace lldb_dap {
 
-struct FunctionBreakpoint : public BreakpointBase {
+struct FunctionBreakpoint : public Breakpoint {
   std::string functionName;
 
   FunctionBreakpoint() = default;
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index a8b438d9d6df3..878449a91aa66 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -364,54 +364,14 @@ llvm::json::Value CreateScope(const llvm::StringRef name,
 //   },
 //   "required": [ "verified" ]
 // }
-llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint &bp,
+llvm::json::Value CreateBreakpoint(BreakpointBase *bp,
                                    std::optional<llvm::StringRef> request_path,
                                    std::optional<uint32_t> request_line,
                                    std::optional<uint32_t> request_column) {
-  // 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.
   llvm::json::Object object;
-  if (!bp.IsValid())
-    return llvm::json::Value(std::move(object));
-
-  object.try_emplace("verified", bp.GetNumResolvedLocations() > 0);
-  object.try_emplace("id", 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
-  // other source file and line breakpoints in the same file.
-
-  // Below we search for the first resolved location in a breakpoint and report
-  // 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();
-  for (size_t i = 0; i < num_locs; ++i) {
-    bp_loc = 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);
-  auto bp_addr = bp_loc.GetAddress();
-
   if (request_path)
     object.try_emplace("source", CreateSource(*request_path));
-
-  if (bp_addr.IsValid()) {
-    std::string formatted_addr =
-        "0x" + llvm::utohexstr(bp_addr.GetLoadAddress(g_dap.target));
-    object.try_emplace("instructionReference", formatted_addr);
-    auto line_entry = bp_addr.GetLineEntry();
-    const auto line = line_entry.GetLine();
-    if (line != UINT32_MAX)
-      object.try_emplace("line", line);
-    const auto column = line_entry.GetColumn();
-    if (column != 0)
-      object.try_emplace("column", column);
-    object.try_emplace("source", CreateSource(line_entry));
-  }
+  bp->CreateJsonObject(object);
   // We try to add request_line as a fallback
   if (request_line)
     object.try_emplace("line", *request_line);
@@ -506,7 +466,7 @@ llvm::json::Value CreateModule(lldb::SBModule &module) {
   return llvm::json::Value(std::move(object));
 }
 
-void AppendBreakpoint(lldb::SBBreakpoint &bp, llvm::json::Array &breakpoints,
+void AppendBreakpoint(BreakpointBase *bp, llvm::json::Array &breakpoints,
                       std::optional<llvm::StringRef> request_path,
                       std::optional<uint32_t> request_line) {
   breakpoints.emplace_back(CreateBreakpoint(bp, request_path, request_line));
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index 62338548890c0..1515f5ba2e5f4 100644
--- a/lldb/tools/l...
[truncated]

Copy link

github-actions bot commented Feb 5, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

…only show up in `SourceBreakpoint` json object.
@ZequanWu
Copy link
Contributor Author

I uploaded #81541 as reference for reviewers to understand the motivation of this patch.

Copy link
Member

@walter-erquinigo walter-erquinigo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@ZequanWu ZequanWu merged commit d58c128 into llvm:main Feb 13, 2024
5 checks passed
ZequanWu added a commit that referenced this pull request Feb 13, 2024
This implements functionality to handle `DataBreakpointInfo` request and
`SetDataBreakpoints` request.

If variablesReference is 0 or not provided, interpret name as ${number
of bytes}@${expression} to set data breakpoint at the given expression
because the spec
https://microsoft.github.io/debug-adapter-protocol/specification#Requests_DataBreakpointInfo
doesn't say how the client could specify the number of bytes to watch.

This is based on top of #80753.
@ZequanWu ZequanWu deleted the lldb-dap-nfc branch February 15, 2024 18:31
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request May 21, 2024
)

This adds a layer between `SounceBreakpoint`/`FunctionBreakpoint` and
`BreakpointBase` to have better separation and encapsulation so we are
not directly operating on `SBBreakpoint`.

I basically moved the `SBBreakpoint` and the methods that requires it
from `BreakpointBase` to `Breakpoint`. This allows adding support for
data watchpoint easier by sharing the logic inside `BreakpointBase`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants