[lldb-dap] Add unit test for breakpoint types

[JDevlieghere]

• Add unit test for breakpoint types for SourceBreakpoint, FunctionBreakpoint and DataBreakpoint.
• Rename DataBreakpointInfo to DataBreakpoint.
• Fix some mapOptions for optional fields.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

• Add unit test for breakpoint types for SourceBreakpoint, FunctionBreakpoint and DataBreakpoint.
• Rename DataBreakpointInfo to DataBreakpoint.
• Fix some mapOptions for optional fields.

---

Full diff: https://github.com/llvm/llvm-project/pull/139792.diff

6 Files Affected:

• (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+1-1)
• (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp (+61-16)
• (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.h (+5-3)
• (modified) lldb/tools/lldb-dap/Watchpoint.cpp (+1-1)
• (modified) lldb/tools/lldb-dap/Watchpoint.h (+1-1)
• (modified) lldb/unittests/DAP/ProtocolTypesTest.cpp (+62)

diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index c6456b4113320..710fa5d2c57ed 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -693,7 +693,7 @@ llvm::json::Value toJSON(const DataBreakpointInfoResponseBody &);
 struct SetDataBreakpointsArguments {
   /// The contents of this array replaces all existing data breakpoints. An
   /// empty array clears all data breakpoints.
-  std::vector<DataBreakpointInfo> breakpoints;
+  std::vector<DataBreakpoint> breakpoints;
 };
 bool fromJSON(const llvm::json::Value &, SetDataBreakpointsArguments &,
               llvm::json::Path);
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
index 7c2f4b20f4956..8d95687e00e53 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
@@ -446,18 +446,48 @@ bool fromJSON(const llvm::json::Value &Params, Breakpoint &BP,
 
 bool fromJSON(const llvm::json::Value &Params, SourceBreakpoint &SB,
               llvm::json::Path P) {
-  json::ObjectMapper O(Params, P);
-  return O && O.map("line", SB.line) && O.map("column", SB.column) &&
-         O.map("condition", SB.condition) &&
-         O.map("hitCondition", SB.hitCondition) &&
-         O.map("logMessage", SB.logMessage) && O.map("mode", SB.mode);
+  llvm::json::ObjectMapper O(Params, P);
+  return O && O.map("line", SB.line) && O.mapOptional("column", SB.column) &&
+         O.mapOptional("condition", SB.condition) &&
+         O.mapOptional("hitCondition", SB.hitCondition) &&
+         O.mapOptional("logMessage", SB.logMessage) &&
+         O.mapOptional("mode", SB.mode);
+}
+
+llvm::json::Value toJSON(const SourceBreakpoint &SB) {
+  llvm::json::Object result{{"line", SB.line}};
+
+  if (SB.column)
+    result.insert({"column", *SB.column});
+  if (SB.condition)
+    result.insert({"condition", *SB.condition});
+  if (SB.hitCondition)
+    result.insert({"hitCondition", *SB.hitCondition});
+  if (SB.logMessage)
+    result.insert({"logMessage", *SB.logMessage});
+  if (SB.mode)
+    result.insert({"mode", *SB.mode});
+
+  return result;
 }
 
 bool fromJSON(const llvm::json::Value &Params, FunctionBreakpoint &FB,
               llvm::json::Path P) {
-  json::ObjectMapper O(Params, P);
-  return O && O.map("name", FB.name) && O.map("condition", FB.condition) &&
-         O.map("hitCondition", FB.hitCondition);
+  llvm::json::ObjectMapper O(Params, P);
+  return O && O.map("name", FB.name) &&
+         O.mapOptional("condition", FB.condition) &&
+         O.mapOptional("hitCondition", FB.hitCondition);
+}
+
+llvm::json::Value toJSON(const FunctionBreakpoint &FB) {
+  llvm::json::Object result{{"name", FB.name}};
+
+  if (FB.condition)
+    result.insert({"condition", *FB.condition});
+  if (FB.hitCondition)
+    result.insert({"hitCondition", *FB.hitCondition});
+
+  return result;
 }
 
 bool fromJSON(const llvm::json::Value &Params, DataBreakpointAccessType &DBAT,
@@ -493,21 +523,36 @@ llvm::json::Value toJSON(const DataBreakpointAccessType &DBAT) {
   llvm_unreachable("unhandled data breakpoint access type.");
 }
 
-bool fromJSON(const llvm::json::Value &Params, DataBreakpointInfo &DBI,
+bool fromJSON(const llvm::json::Value &Params, DataBreakpoint &DBI,
               llvm::json::Path P) {
-  json::ObjectMapper O(Params, P);
+  llvm::json::ObjectMapper O(Params, P);
   return O && O.map("dataId", DBI.dataId) &&
-         O.map("accessType", DBI.accessType) &&
-         O.map("condition", DBI.condition) &&
-         O.map("hitCondition", DBI.hitCondition);
+         O.mapOptional("accessType", DBI.accessType) &&
+         O.mapOptional("condition", DBI.condition) &&
+         O.mapOptional("hitCondition", DBI.hitCondition);
+}
+
+llvm::json::Value toJSON(const DataBreakpoint &DBI) {
+  llvm::json::Object result{{"dataId", DBI.dataId}};
+
+  if (DBI.accessType)
+    result.insert({"accessType", *DBI.accessType});
+  if (DBI.condition)
+    result.insert({"condition", *DBI.condition});
+  if (DBI.hitCondition)
+    result.insert({"hitCondition", *DBI.hitCondition});
+
+  return result;
 }
 
 bool fromJSON(const llvm::json::Value &Params, InstructionBreakpoint &IB,
               llvm::json::Path P) {
-  json::ObjectMapper O(Params, P);
+  llvm::json::ObjectMapper O(Params, P);
   return O && O.map("instructionReference", IB.instructionReference) &&
-         O.map("offset", IB.offset) && O.map("condition", IB.condition) &&
-         O.map("hitCondition", IB.hitCondition) && O.map("mode", IB.mode);
+         O.mapOptional("offset", IB.offset) &&
+         O.mapOptional("condition", IB.condition) &&
+         O.mapOptional("hitCondition", IB.hitCondition) &&
+         O.mapOptional("mode", IB.mode);
 }
 
 } // namespace lldb_dap::protocol
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
index cab188637acd5..c48988a48a373 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
@@ -464,6 +464,7 @@ struct SourceBreakpoint {
   std::optional<std::string> mode;
 };
 bool fromJSON(const llvm::json::Value &, SourceBreakpoint &, llvm::json::Path);
+llvm::json::Value toJSON(const SourceBreakpoint &);
 
 /// Properties of a breakpoint passed to the `setFunctionBreakpoints` request.
 struct FunctionBreakpoint {
@@ -483,6 +484,7 @@ struct FunctionBreakpoint {
 };
 bool fromJSON(const llvm::json::Value &, FunctionBreakpoint &,
               llvm::json::Path);
+llvm::json::Value toJSON(const FunctionBreakpoint &);
 
 /// This enumeration defines all possible access types for data breakpoints.
 /// Values: ‘read’, ‘write’, ‘readWrite’
@@ -496,7 +498,7 @@ bool fromJSON(const llvm::json::Value &, DataBreakpointAccessType &,
 llvm::json::Value toJSON(const DataBreakpointAccessType &);
 
 /// Properties of a data breakpoint passed to the `setDataBreakpoints` request.
-struct DataBreakpointInfo {
+struct DataBreakpoint {
   /// An id representing the data. This id is returned from the
   /// `dataBreakpointInfo` request.
   std::string dataId;
@@ -511,8 +513,8 @@ struct DataBreakpointInfo {
   /// The debug adapter is expected to interpret the expression as needed.
   std::optional<std::string> hitCondition;
 };
-bool fromJSON(const llvm::json::Value &, DataBreakpointInfo &,
-              llvm::json::Path);
+bool fromJSON(const llvm::json::Value &, DataBreakpoint &, llvm::json::Path);
+llvm::json::Value toJSON(const DataBreakpoint &);
 
 /// Properties of a breakpoint passed to the `setInstructionBreakpoints` request
 struct InstructionBreakpoint {
diff --git a/lldb/tools/lldb-dap/Watchpoint.cpp b/lldb/tools/lldb-dap/Watchpoint.cpp
index 73ed4fdbae1b8..0acc980890be8 100644
--- a/lldb/tools/lldb-dap/Watchpoint.cpp
+++ b/lldb/tools/lldb-dap/Watchpoint.cpp
@@ -17,7 +17,7 @@
 #include <string>
 
 namespace lldb_dap {
-Watchpoint::Watchpoint(DAP &d, const protocol::DataBreakpointInfo &breakpoint)
+Watchpoint::Watchpoint(DAP &d, const protocol::DataBreakpoint &breakpoint)
     : BreakpointBase(d, breakpoint.condition, breakpoint.hitCondition) {
   llvm::StringRef dataId = breakpoint.dataId;
   auto [addr_str, size_str] = dataId.split('/');
diff --git a/lldb/tools/lldb-dap/Watchpoint.h b/lldb/tools/lldb-dap/Watchpoint.h
index b7fe58fe73501..d943e1218bdcd 100644
--- a/lldb/tools/lldb-dap/Watchpoint.h
+++ b/lldb/tools/lldb-dap/Watchpoint.h
@@ -22,7 +22,7 @@ namespace lldb_dap {
 
 class Watchpoint : public BreakpointBase {
 public:
-  Watchpoint(DAP &d, const protocol::DataBreakpointInfo &breakpoint);
+  Watchpoint(DAP &d, const protocol::DataBreakpoint &breakpoint);
   Watchpoint(DAP &d, lldb::SBWatchpoint wp) : BreakpointBase(d), m_wp(wp) {}
 
   void SetCondition() override;
diff --git a/lldb/unittests/DAP/ProtocolTypesTest.cpp b/lldb/unittests/DAP/ProtocolTypesTest.cpp
index 56b21f18fa7cd..f5d72f06432d5 100644
--- a/lldb/unittests/DAP/ProtocolTypesTest.cpp
+++ b/lldb/unittests/DAP/ProtocolTypesTest.cpp
@@ -132,3 +132,65 @@ TEST(ProtocolTypesTest, Breakpoint) {
   EXPECT_EQ(breakpoint.offset, deserialized_breakpoint->offset);
   EXPECT_EQ(breakpoint.reason, deserialized_breakpoint->reason);
 }
+
+TEST(ProtocolTypesTest, SourceBreakpoint) {
+  SourceBreakpoint source_breakpoint;
+  source_breakpoint.line = 42;
+  source_breakpoint.column = 5;
+  source_breakpoint.condition = "x > 10";
+  source_breakpoint.hitCondition = "5";
+  source_breakpoint.logMessage = "Breakpoint hit at line 42";
+  source_breakpoint.mode = "hardware";
+
+  llvm::Expected<SourceBreakpoint> deserialized_source_breakpoint =
+      roundtrip(source_breakpoint);
+  ASSERT_THAT_EXPECTED(deserialized_source_breakpoint, llvm::Succeeded());
+
+  EXPECT_EQ(source_breakpoint.line, deserialized_source_breakpoint->line);
+  EXPECT_EQ(source_breakpoint.column, deserialized_source_breakpoint->column);
+  EXPECT_EQ(source_breakpoint.condition,
+            deserialized_source_breakpoint->condition);
+  EXPECT_EQ(source_breakpoint.hitCondition,
+            deserialized_source_breakpoint->hitCondition);
+  EXPECT_EQ(source_breakpoint.logMessage,
+            deserialized_source_breakpoint->logMessage);
+  EXPECT_EQ(source_breakpoint.mode, deserialized_source_breakpoint->mode);
+}
+
+TEST(ProtocolTypesTest, FunctionBreakpoint) {
+  FunctionBreakpoint function_breakpoint;
+  function_breakpoint.name = "myFunction";
+  function_breakpoint.condition = "x == 0";
+  function_breakpoint.hitCondition = "3";
+
+  llvm::Expected<FunctionBreakpoint> deserialized_function_breakpoint =
+      roundtrip(function_breakpoint);
+  ASSERT_THAT_EXPECTED(deserialized_function_breakpoint, llvm::Succeeded());
+
+  EXPECT_EQ(function_breakpoint.name, deserialized_function_breakpoint->name);
+  EXPECT_EQ(function_breakpoint.condition,
+            deserialized_function_breakpoint->condition);
+  EXPECT_EQ(function_breakpoint.hitCondition,
+            deserialized_function_breakpoint->hitCondition);
+}
+
+TEST(ProtocolTypesTest, DataBreakpoint) {
+  DataBreakpoint data_breakpoint_info;
+  data_breakpoint_info.dataId = "variable1";
+  data_breakpoint_info.accessType = eDataBreakpointAccessTypeReadWrite;
+  data_breakpoint_info.condition = "x > 100";
+  data_breakpoint_info.hitCondition = "10";
+
+  llvm::Expected<DataBreakpoint> deserialized_data_breakpoint_info =
+      roundtrip(data_breakpoint_info);
+  ASSERT_THAT_EXPECTED(deserialized_data_breakpoint_info, llvm::Succeeded());
+
+  EXPECT_EQ(data_breakpoint_info.dataId,
+            deserialized_data_breakpoint_info->dataId);
+  EXPECT_EQ(data_breakpoint_info.accessType,
+            deserialized_data_breakpoint_info->accessType);
+  EXPECT_EQ(data_breakpoint_info.condition,
+            deserialized_data_breakpoint_info->condition);
+  EXPECT_EQ(data_breakpoint_info.hitCondition,
+            deserialized_data_breakpoint_info->hitCondition);
+}

[ashgti]

Looks good!