[lldb][lldb-dap] add review changes

Author: da-viper

lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py

@@ -79,7 +79,8 @@ def test_basic(self):
         self.assertIsNotNone(leaf_frame, "expect a leaf frame")
         self.assertEqual(step_in_targets[1]["label"], leaf_frame["name"])
 
-    def test_supported_capability(self):
+    @skipIf(archs=no_match(["x86", "x86_64"]))
+    def test_supported_capability_x86_arch(self):
         program = self.getBuildArtifact("a.out")
         self.build_and_launch(program)
         source = "main.cpp"
@@ -91,18 +92,35 @@ def test_supported_capability(self):
         is_supported = self.dap_server.get_initialize_value(
             "supportsStepInTargetsRequest"
         )
-        arch: str = self.getArchitecture()
-        if arch.startswith("x86"):
-            self.assertTrue(
-                is_supported,
-                f"expect capability `stepInTarget` is supported with architecture {arch}",
-            )
-        else:
-            self.assertFalse(
-                is_supported,
-                f"expect capability `stepInTarget` is not supported with architecture {arch}",
-            )
 
+        self.assertEqual(
+            is_supported,
+            True,
+            f"expect capability `stepInTarget` is supported with architecture {self.getArchitecture()}",
+        )
+        # clear breakpoints.
+        self.set_source_breakpoints(source, [])
+        self.continue_to_exit()
+
+    @skipIf(archs=["x86", "x86_64"])
+    def test_supported_capability_other_archs(self):
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        source = "main.cpp"
+        bp_lines = [line_number(source, "// set breakpoint here")]
+        breakpoint_ids = self.set_source_breakpoints(source, bp_lines)
+        self.assertEqual(
+            len(breakpoint_ids), len(bp_lines), "expect correct number of breakpoints"
+        )
+        is_supported = self.dap_server.get_initialize_value(
+            "supportsStepInTargetsRequest"
+        )
+
+        self.assertEqual(
+            is_supported,
+            False,
+            f"expect capability `stepInTarget` is not supported with architecture {self.getArchitecture()}",
+        )
         # clear breakpoints.
         self.set_source_breakpoints(source, [])
         self.continue_to_exit()

lldb/tools/lldb-dap/EventHelper.cpp

@@ -33,22 +33,21 @@ static void SendThreadExitedEvent(DAP &dap, lldb::tid_t tid) {
   dap.SendJSON(llvm::json::Value(std::move(event)));
 }
 
-void SendAdditionalCapabilities(DAP &dap) {
-  if (dap.target.IsValid()) {
-    // FIXME: stepInTargets request is only supported by the x86 architecture
-    // remove when `lldb::InstructionControlFlowKind` is supported by other
-    // architectures
-    const llvm::StringRef target_triple = dap.target.GetTriple();
-    if (!target_triple.starts_with("x86")) {
-      llvm::json::Object event(CreateEventObject("capabilities"));
-      llvm::json::Object capabilities{{"supportsStepInTargetsRequest", false}};
+void SendTargetBasedCapabilities(DAP &dap) {
+  if (!dap.target.IsValid())
+    return;
 
-      llvm::json::Object body;
-      body.try_emplace("capabilities", std::move(capabilities));
-      event.try_emplace("body", std::move(body));
-      dap.SendJSON(llvm::json::Value(std::move(event)));
-    }
-  }
+  // FIXME: stepInTargets request is only supported by the x86
+  // architecture remove when `lldb::InstructionControlFlowKind` is
+  // supported by other architectures
+  const llvm::StringRef target_triple = dap.target.GetTriple();
+  if (target_triple.starts_with("x86"))
+    return;
+
+  protocol::Event event;
+  event.event = "capabilities";
+  event.body = llvm::json::Object{{"supportsStepInTargetsRequest", false}};
+  dap.Send(event);
 }
 // "ProcessEvent": {
 //   "allOf": [

lldb/tools/lldb-dap/EventHelper.h

@@ -16,7 +16,7 @@ struct DAP;
 
 enum LaunchMethod { Launch, Attach, AttachForSuspendedLaunch };
 
-void SendAdditionalCapabilities(DAP &dap);
+void SendTargetBasedCapabilities(DAP &dap);
 
 void SendProcessEvent(DAP &dap, LaunchMethod launch_method);
 

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

@@ -141,7 +141,6 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const {
 
 void AttachRequestHandler::PostRun() const {
   dap.SendJSON(CreateEventObject("initialized"));
-  SendAdditionalCapabilities(dap);
 }
 
 } // namespace lldb_dap

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

@@ -30,6 +30,7 @@ llvm::Error
 ConfigurationDoneRequestHandler::Run(const ConfigurationDoneArguments &) const {
   dap.configuration_done = true;
 
+  SendTargetBasedCapabilities(dap);
   // Ensure any command scripts did not leave us in an unexpected state.
   lldb::SBProcess process = dap.target.GetProcess();
   if (!process.IsValid() ||

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

@@ -68,7 +68,6 @@ Error LaunchRequestHandler::Run(const LaunchRequestArguments &arguments) const {
 
 void LaunchRequestHandler::PostRun() const {
   dap.SendJSON(CreateEventObject("initialized"));
-  SendAdditionalCapabilities(dap);
 }
 
 } // namespace lldb_dap

lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp

@@ -582,8 +582,7 @@ llvm::json::Value toJSON(const SteppingGranularity &SG) {
   llvm_unreachable("unhandled stepping granularity.");
 }
 
-bool fromJSON(const llvm::json::Value &Params, StepInTarget &SIT,
-              llvm::json::Path P) {
+bool fromJSON(const json::Value &Params, StepInTarget &SIT, json::Path P) {
   json::ObjectMapper O(Params, P);
   return O && O.map("id", SIT.id) && O.map("label", SIT.label) &&
          O.mapOptional("line", SIT.line) &&
@@ -595,13 +594,13 @@ bool fromJSON(const llvm::json::Value &Params, StepInTarget &SIT,
 llvm::json::Value toJSON(const StepInTarget &SIT) {
   json::Object target{{"id", SIT.id}, {"label", SIT.label}};
 
-  if (SIT.line)
+  if (SIT.line && *SIT.line != LLDB_INVALID_LINE_NUMBER)
     target.insert({"line", SIT.line});
-  if (SIT.column)
+  if (SIT.column && *SIT.column != LLDB_INVALID_COLUMN_NUMBER)
     target.insert({"column", SIT.column});
-  if (SIT.endLine)
+  if (SIT.endLine && *SIT.endLine != LLDB_INVALID_LINE_NUMBER)
     target.insert({"endLine", SIT.endLine});
-  if (SIT.endColumn)
+  if (SIT.endColumn && *SIT.endLine != LLDB_INVALID_COLUMN_NUMBER)
     target.insert({"endColumn", SIT.endColumn});
 
   return target;

lldb/tools/lldb-dap/Protocol/ProtocolTypes.h

@@ -419,26 +419,26 @@ llvm::json::Value toJSON(const SteppingGranularity &);
 /// which single target the `stepIn` request should step.
 struct StepInTarget {
   /// Unique identifier for a step-in target.
-  uint64_t id = LLDB_INVALID_ADDRESS;
+  lldb::addr_t id = LLDB_INVALID_ADDRESS;
 
   /// The name of the step-in target (shown in the UI).
   std::string label;
 
   /// The line of the step-in target.
-  std::optional<uint64_t> line;
+  std::optional<uint32_t> line = LLDB_INVALID_LINE_NUMBER;
 
   /// Start position of the range covered by the step in target. It is measured
   /// in UTF-16 code units and the client capability `columnsStartAt1`
   /// determines whether it is 0- or 1-based.
-  std::optional<uint64_t> column;
+  std::optional<uint32_t> column = LLDB_INVALID_COLUMN_NUMBER;
 
   /// The end line of the range covered by the step-in target.
-  std::optional<uint64_t> endLine;
+  std::optional<uint32_t> endLine = LLDB_INVALID_LINE_NUMBER;
 
   /// End position of the range covered by the step in target. It is measured in
   /// UTF-16 code units and the client capability `columnsStartAt1` determines
   /// whether it is 0- or 1-based.
-  std::optional<uint64_t> endColumn;
+  std::optional<uint32_t> endColumn = LLDB_INVALID_COLUMN_NUMBER;
 };
 bool fromJSON(const llvm::json::Value &, StepInTarget &, llvm::json::Path);
 llvm::json::Value toJSON(const StepInTarget &);