Skip to content

[lldb-dap] Use protocol types for modules request and events. #146966

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

da-viper
Copy link
Contributor

@da-viper da-viper commented Jul 3, 2025

Can now limit the number of modules fetched.
Show the non-standard debugInfoSize field in the vs-code extension.
Update tests to fix silently failing test and handle when a module is removed.

@da-viper da-viper requested a review from ashgti July 3, 2025 21:52
@da-viper da-viper requested a review from JDevlieghere as a code owner July 3, 2025 21:52
@llvmbot llvmbot added the lldb label Jul 3, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 3, 2025

@llvm/pr-subscribers-lldb

Author: Ebuka Ezike (da-viper)

Changes

Can now limit the number of modules fetched.
Show the non-standard debugInfoSize field in the vs-code extension.
Update tests to fix silently failing test and handle when a module is removed.


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

25 Files Affected:

  • (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+10-4)
  • (modified) lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py (+13-6)
  • (modified) lldb/test/API/tools/lldb-dap/module-event/main.cpp (+1-1)
  • (modified) lldb/test/API/tools/lldb-dap/module/TestDAP_module.py (+26-6)
  • (modified) lldb/tools/lldb-dap/DAP.cpp (+1-39)
  • (modified) lldb/tools/lldb-dap/EventHelper.cpp (+46)
  • (modified) lldb/tools/lldb-dap/EventHelper.h (+2)
  • (modified) lldb/tools/lldb-dap/Handler/ModulesRequestHandler.cpp (+30-51)
  • (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+6-3)
  • (modified) lldb/tools/lldb-dap/JSONUtils.cpp (-95)
  • (modified) lldb/tools/lldb-dap/JSONUtils.h (-18)
  • (modified) lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp (+16)
  • (modified) lldb/tools/lldb-dap/Protocol/ProtocolEvents.h (+15)
  • (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+14)
  • (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+21)
  • (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp (+25)
  • (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.h (+42)
  • (modified) lldb/tools/lldb-dap/ProtocolUtils.cpp (+103)
  • (modified) lldb/tools/lldb-dap/ProtocolUtils.h (+20)
  • (modified) lldb/tools/lldb-dap/src-ts/debug-session-tracker.ts (+6-2)
  • (modified) lldb/tools/lldb-dap/src-ts/ui/modules-data-provider.ts (+9-6)
  • (modified) lldb/unittests/DAP/CMakeLists.txt (+1)
  • (modified) lldb/unittests/DAP/JSONUtilsTest.cpp (-11)
  • (modified) lldb/unittests/DAP/ProtocolTypesTest.cpp (+91)
  • (added) lldb/unittests/DAP/ProtocolUtilsTest.cpp (+24)
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 0fe36cd4bc71f..d227a66a703c1 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -199,8 +199,8 @@ def _read_packet_thread(self):
         finally:
             dump_dap_log(self.log_file)
 
-    def get_modules(self):
-        module_list = self.request_modules()["body"]["modules"]
+    def get_modules(self, startModule: int = 0, moduleCount: int = 0):
+        module_list = self.request_modules(startModule, moduleCount)["body"]["modules"]
         modules = {}
         for module in module_list:
             modules[module["name"]] = module
@@ -1143,8 +1143,14 @@ def request_completions(self, text, frameId=None):
         }
         return self.send_recv(command_dict)
 
-    def request_modules(self):
-        return self.send_recv({"command": "modules", "type": "request"})
+    def request_modules(self, startModule: int, moduleCount: int):
+        return self.send_recv(
+            {
+                "command": "modules",
+                "type": "request",
+                "arguments": {"startModule": startModule, "moduleCount": moduleCount},
+            }
+        )
 
     def request_stackTrace(
         self, threadId=None, startFrame=None, levels=None, format=None, dump=False
diff --git a/lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py b/lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py
index 1ef2f2a8235a4..64ed4154b035d 100644
--- a/lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py
+++ b/lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py
@@ -32,7 +32,7 @@ def test_module_event(self):
 
         # Make sure we got a module event for libother.
         event = self.dap_server.wait_for_event("module", 5)
-        self.assertTrue(event, "didn't get a module event")
+        self.assertIsNotNone(event, "didn't get a module event")
         module_name = event["body"]["module"]["name"]
         module_id = event["body"]["module"]["id"]
         self.assertEqual(event["body"]["reason"], "new")
@@ -43,13 +43,20 @@ def test_module_event(self):
 
         # Make sure we got a module event for libother.
         event = self.dap_server.wait_for_event("module", 5)
-        self.assertTrue(event, "didn't get a module event")
+        self.assertIsNotNone(event, "didn't get a module event")
         reason = event["body"]["reason"]
-        self.assertEqual(event["body"]["reason"], "removed")
+        self.assertEqual(reason, "removed")
         self.assertEqual(event["body"]["module"]["id"], module_id)
 
-        # The removed module event should omit everything but the module id.
-        # Check that there's no module name in the event.
-        self.assertNotIn("name", event["body"]["module"])
+        # The removed module event should omit everything but the module id and name
+        # as they are required fields.
+        module_data = event["body"]["module"]
+        required_keys = ["id", "name"]
+        self.assertListEqual(list(module_data.keys()), required_keys)
+        self.assertEqual(module_data["name"], "", "expects empty name.")
+
+        # Make sure we do not send another event
+        event = self.dap_server.wait_for_event("module", 3)
+        self.assertIsNone(event, "expects no events.")
 
         self.continue_to_exit()
diff --git a/lldb/test/API/tools/lldb-dap/module-event/main.cpp b/lldb/test/API/tools/lldb-dap/module-event/main.cpp
index 711471b9fadd0..283b2f8214b4a 100644
--- a/lldb/test/API/tools/lldb-dap/module-event/main.cpp
+++ b/lldb/test/API/tools/lldb-dap/module-event/main.cpp
@@ -18,5 +18,5 @@ int main(int argc, char const *argv[]) {
   dlclose(handle);
   printf("after dlclose\n"); // breakpoint 3
 
-  return 0; // breakpoint 1
+  return 0;
 }
diff --git a/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py b/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py
index 4fc221668a8ee..d8d9cf24c5e41 100644
--- a/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py
+++ b/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py
@@ -45,16 +45,20 @@ def run_test(self, symbol_basename, expect_debug_info_size):
             context="repl",
         )
 
-        def checkSymbolsLoadedWithSize():
+        def check_symbols_loaded_with_size():
             active_modules = self.dap_server.get_modules()
             program_module = active_modules[program_basename]
             self.assertIn("symbolFilePath", program_module)
             self.assertIn(symbols_path, program_module["symbolFilePath"])
-            symbol_regex = re.compile(r"[0-9]+(\.[0-9]*)?[KMG]?B")
-            return symbol_regex.match(program_module["symbolStatus"])
+            size_regex = re.compile(r"[0-9]+(\.[0-9]*)?[KMG]?B")
+            return size_regex.match(program_module["debugInfoSize"])
 
         if expect_debug_info_size:
-            self.waitUntil(checkSymbolsLoadedWithSize)
+            self.assertTrue(
+                self.waitUntil(check_symbols_loaded_with_size),
+                "expect has debug info size",
+            )
+
         active_modules = self.dap_server.get_modules()
         program_module = active_modules[program_basename]
         self.assertEqual(program_basename, program_module["name"])
@@ -84,6 +88,20 @@ def checkSymbolsLoadedWithSize():
         self.assertNotEqual(len(module_changed_names), 0)
         self.assertIn(program_module["name"], module_changed_names)
 
+        # fetch modules from offset
+        if len(active_modules.keys()) > 2:
+            resp = self.dap_server.request_modules(startModule=1, moduleCount=1)
+            self.assertTrue(resp["success"], f"expects a successful response {resp!r}")
+            resp_total_modules = resp["body"]["totalModules"]
+            self.assertEqual(resp_total_modules, len(active_modules))
+            resp_modules = resp["body"]["modules"]
+            self.assertEqual(len(resp_modules), 1, "expects only one module")
+
+            module_name = resp_modules[0]["name"]
+            self.assertIn(module_name, active_modules.keys())
+
+        self.continue_to_exit()
+
     @skipIfWindows
     def test_modules(self):
         """
@@ -119,8 +137,10 @@ def test_compile_units(self):
         lines = [breakpoint1_line]
         breakpoint_ids = self.set_source_breakpoints(source, lines)
         self.continue_to_breakpoints(breakpoint_ids)
-        moduleId = self.dap_server.get_modules()["a.out"]["id"]
-        response = self.dap_server.request_compileUnits(moduleId)
+        module_id = self.dap_server.get_modules()["a.out"]["id"]
+        response = self.dap_server.request_compileUnits(module_id)
         self.assertTrue(response["body"])
         cu_paths = [cu["compileUnitPath"] for cu in response["body"]["compileUnits"]]
         self.assertIn(main_source_path, cu_paths, "Real path to main.cpp matches")
+
+        self.continue_to_exit()
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index cd97458bd4aa8..1d97a6d26df28 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -1336,45 +1336,7 @@ void DAP::EventThread() {
           SendStdOutStdErr(*this, process);
         }
       } else if (lldb::SBTarget::EventIsTargetEvent(event)) {
-        if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded ||
-            event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded ||
-            event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded ||
-            event_mask & lldb::SBTarget::eBroadcastBitSymbolsChanged) {
-          const uint32_t num_modules =
-              lldb::SBTarget::GetNumModulesFromEvent(event);
-          std::lock_guard<std::mutex> guard(modules_mutex);
-          for (uint32_t i = 0; i < num_modules; ++i) {
-            lldb::SBModule module =
-                lldb::SBTarget::GetModuleAtIndexFromEvent(i, event);
-            if (!module.IsValid())
-              continue;
-            llvm::StringRef module_id = module.GetUUIDString();
-            if (module_id.empty())
-              continue;
-
-            llvm::StringRef reason;
-            bool id_only = false;
-            if (modules.contains(module_id)) {
-              if (event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded) {
-                modules.erase(module_id);
-                reason = "removed";
-                id_only = true;
-              } else {
-                reason = "changed";
-              }
-            } else {
-              modules.insert(module_id);
-              reason = "new";
-            }
-
-            llvm::json::Object body;
-            body.try_emplace("reason", reason);
-            body.try_emplace("module", CreateModule(target, module, id_only));
-            llvm::json::Object module_event = CreateEventObject("module");
-            module_event.try_emplace("body", std::move(body));
-            SendJSON(llvm::json::Value(std::move(module_event)));
-          }
-        }
+        HandleTargetEvent(*this, event);
       } else if (lldb::SBBreakpoint::EventIsBreakpointEvent(event)) {
         if (event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged) {
           auto event_type =
diff --git a/lldb/tools/lldb-dap/EventHelper.cpp b/lldb/tools/lldb-dap/EventHelper.cpp
index 364cc7ab4ef8c..5f7739da1580d 100644
--- a/lldb/tools/lldb-dap/EventHelper.cpp
+++ b/lldb/tools/lldb-dap/EventHelper.cpp
@@ -13,6 +13,8 @@
 #include "LLDBUtils.h"
 #include "Protocol/ProtocolEvents.h"
 #include "Protocol/ProtocolTypes.h"
+#include "ProtocolUtils.h"
+#include "lldb/API/SBEvent.h"
 #include "lldb/API/SBFileSpec.h"
 #include "llvm/Support/Error.h"
 
@@ -269,4 +271,48 @@ void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process) {
   dap.SendJSON(llvm::json::Value(std::move(event)));
 }
 
+void HandleTargetEvent(DAP &dap, const lldb::SBEvent &event) {
+  const lldb::SBTarget &target = dap.target;
+  const uint32_t event_mask = event.GetType();
+  if (!(event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded) &&
+      !(event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded) &&
+      !(event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded) &&
+      !(event_mask & lldb::SBTarget::eBroadcastBitSymbolsChanged))
+    return;
+
+  const uint32_t num_modules = lldb::SBTarget::GetNumModulesFromEvent(event);
+  std::lock_guard<std::mutex> guard(dap.modules_mutex);
+
+  for (uint32_t i = 0; i < num_modules; ++i) {
+    lldb::SBModule module = lldb::SBTarget::GetModuleAtIndexFromEvent(i, event);
+
+    const bool remove_module =
+        event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded;
+    auto p_module = lldb_dap::CreateModule(target, module, remove_module);
+    if (!p_module)
+      continue;
+
+    const llvm::StringRef module_id = p_module->id;
+    if (dap.modules.contains(module_id)) {
+      if (event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded) {
+        dap.modules.erase(module_id);
+        dap.Send(protocol::Event{
+            "module", protocol::ModuleEventBody{
+                          std::move(p_module).value(),
+                          protocol::ModuleEventBody::eReasonRemoved}});
+      } else {
+        dap.Send(protocol::Event{
+            "module", protocol::ModuleEventBody{
+                          std::move(p_module).value(),
+                          protocol::ModuleEventBody::eReasonChanged}});
+      }
+    } else if (!remove_module) {
+      dap.modules.insert(module_id);
+      dap.Send(protocol::Event{
+          "module",
+          protocol::ModuleEventBody{std::move(p_module).value(),
+                                    protocol::ModuleEventBody::eReasonNew}});
+    }
+  }
+}
 } // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/EventHelper.h b/lldb/tools/lldb-dap/EventHelper.h
index 72ad5308a2b0c..ba82c89a31119 100644
--- a/lldb/tools/lldb-dap/EventHelper.h
+++ b/lldb/tools/lldb-dap/EventHelper.h
@@ -32,6 +32,8 @@ void SendContinuedEvent(DAP &dap);
 
 void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process);
 
+void HandleTargetEvent(DAP &dap, const lldb::SBEvent &event);
+
 } // namespace lldb_dap
 
 #endif
diff --git a/lldb/tools/lldb-dap/Handler/ModulesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ModulesRequestHandler.cpp
index d37f302b06958..e8440f591d2b2 100644
--- a/lldb/tools/lldb-dap/Handler/ModulesRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/ModulesRequestHandler.cpp
@@ -7,64 +7,43 @@
 //===----------------------------------------------------------------------===//
 
 #include "DAP.h"
-#include "EventHelper.h"
-#include "JSONUtils.h"
+#include "ProtocolUtils.h"
 #include "RequestHandler.h"
 
+using namespace lldb_dap::protocol;
 namespace lldb_dap {
 
-// "modulesRequest": {
-//   "allOf": [ { "$ref": "#/definitions/Request" }, {
-//     "type": "object",
-//     "description": "Modules request; value of command field is
-//                     'modules'.",
-//     "properties": {
-//       "command": {
-//         "type": "string",
-//         "enum": [ "modules" ]
-//       },
-//     },
-//     "required": [ "command" ]
-//   }]
-// },
-// "modulesResponse": {
-//   "allOf": [ { "$ref": "#/definitions/Response" }, {
-//     "type": "object",
-//     "description": "Response to 'modules' request.",
-//     "properties": {
-//       "body": {
-//         "description": "Response to 'modules' request. Array of
-//                         module objects."
-//       }
-//     }
-//   }]
-// }
-void ModulesRequestHandler::operator()(
-    const llvm::json::Object &request) const {
-  llvm::json::Object response;
-  FillResponse(request, response);
-
-  llvm::json::Array modules;
-
-  {
-    std::lock_guard<std::mutex> guard(dap.modules_mutex);
-    for (size_t i = 0; i < dap.target.GetNumModules(); i++) {
-      lldb::SBModule module = dap.target.GetModuleAtIndex(i);
-      if (!module.IsValid())
-        continue;
-
-      llvm::StringRef module_id = module.GetUUIDString();
-      if (!module_id.empty())
-        dap.modules.insert(module_id);
-
-      modules.emplace_back(CreateModule(dap.target, module));
+/// Modules can be retrieved from the debug adapter with this request which can
+/// either return all modules or a range of modules to support paging.
+///
+/// Clients should only call this request if the corresponding capability
+/// `supportsModulesRequest` is true.
+llvm::Expected<ModulesResponseBody>
+ModulesRequestHandler::Run(const std::optional<ModulesArguments> &args) const {
+  ModulesResponseBody response;
+  const auto [start_module, module_count] = args.value_or({});
+
+  std::vector<Module> &modules = response.modules;
+  std::lock_guard<std::mutex> guard(dap.modules_mutex);
+  const uint32_t total_modules = dap.target.GetNumModules();
+  const uint32_t max_num_module =
+      module_count == 0 ? total_modules
+                        : std::min(start_module + module_count, total_modules);
+  modules.reserve(max_num_module);
+
+  response.totalModules = total_modules;
+
+  for (uint32_t i = start_module; i < max_num_module; i++) {
+    lldb::SBModule module = dap.target.GetModuleAtIndex(i);
+
+    std::optional<Module> result = CreateModule(dap.target, module);
+    if (result && !result->id.empty()) {
+      dap.modules.insert(result->id);
+      modules.emplace_back(std::move(result).value());
     }
   }
 
-  llvm::json::Object body;
-  body.try_emplace("modules", std::move(modules));
-  response.try_emplace("body", std::move(body));
-  dap.SendJSON(llvm::json::Value(std::move(response)));
+  return response;
 }
 
 } // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index e35b9830ab60f..3b910abe81992 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -466,14 +466,17 @@ class CompileUnitsRequestHandler : public LegacyRequestHandler {
   void operator()(const llvm::json::Object &request) const override;
 };
 
-class ModulesRequestHandler : public LegacyRequestHandler {
+class ModulesRequestHandler final
+    : public RequestHandler<std::optional<protocol::ModulesArguments>,
+                            llvm::Expected<protocol::ModulesResponseBody>> {
 public:
-  using LegacyRequestHandler::LegacyRequestHandler;
+  using RequestHandler::RequestHandler;
   static llvm::StringLiteral GetCommand() { return "modules"; }
   FeatureSet GetSupportedFeatures() const override {
     return {protocol::eAdapterFeatureModulesRequest};
   }
-  void operator()(const llvm::json::Object &request) const override;
+  llvm::Expected<protocol::ModulesResponseBody>
+  Run(const std::optional<protocol::ModulesArguments> &args) const override;
 };
 
 class PauseRequestHandler : public LegacyRequestHandler {
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 08e65ab835a57..f52661cf9d6c2 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -341,101 +341,6 @@ llvm::json::Value CreateScope(const llvm::StringRef name,
   return llvm::json::Value(std::move(object));
 }
 
-static uint64_t GetDebugInfoSizeInSection(lldb::SBSection section) {
-  uint64_t debug_info_size = 0;
-  llvm::StringRef section_name(section.GetName());
-  if (section_name.starts_with(".debug") ||
-      section_name.starts_with("__debug") ||
-      section_name.starts_with(".apple") || section_name.starts_with("__apple"))
-    debug_info_size += section.GetFileByteSize();
-  size_t num_sub_sections = section.GetNumSubSections();
-  for (size_t i = 0; i < num_sub_sections; i++) {
-    debug_info_size +=
-        GetDebugInfoSizeInSection(section.GetSubSectionAtIndex(i));
-  }
-  return debug_info_size;
-}
-
-static uint64_t GetDebugInfoSize(lldb::SBModule module) {
-  uint64_t debug_info_size = 0;
-  size_t num_sections = module.GetNumSections();
-  for (size_t i = 0; i < num_sections; i++) {
-    debug_info_size += GetDebugInfoSizeInSection(module.GetSectionAtIndex(i));
-  }
-  return debug_info_size;
-}
-
-static std::string ConvertDebugInfoSizeToString(uint64_t debug_info) {
-  std::ostringstream oss;
-  oss << std::fixed << std::setprecision(1);
-  if (debug_info < 1024) {
-    oss << debug_info << "B";
-  } else if (debug_info < 1024 * 1024) {
-    double kb = double(debug_info) / 1024.0;
-    oss << kb << "KB";
-  } else if (debug_info < 1024 * 1024 * 1024) {
-    double mb = double(debug_info) / (1024.0 * 1024.0);
-    oss << mb << "MB";
-  } else {
-    double gb = double(debug_info) / (1024.0 * 1024.0 * 1024.0);
-    oss << gb << "GB";
-  }
-  return oss.str();
-}
-
-llvm::json::Value CreateModule(lldb::SBTarget &target, lldb::SBModule &module,
-                               bool id_only) {
-  llvm::json::Object object;
-  if (!target.IsValid() || !module.IsValid())
-    return llvm::json::Value(std::move(object));
-
-  const char *uuid = module.GetUUIDString();
-  object.try_emplace("id", uuid ? std::string(uuid) : std::string(""));
-
-  if (id_only)
-    return llvm::json::Value(std::move(object));
-
-  object.try_emplace("name", std::string(module.GetFileSpec().GetFilename()));
-  char module_path_arr[PATH_MAX];
-  module.GetFileSpec().GetPath(module_path_arr, sizeof(module_path_arr));
-  std::string module_path(module_path_arr);
-  object.try_emplace("path", module_path);
-  if (module.GetNumCompileUnits() > 0) {
-    std::string symbol_str = "Symbols loaded.";
-    std::string debug_info_size;
-    uint64_t debug_info = GetDebugInfoSize(module);
-    if (debug_info > 0) {
-      debug_info_size = ConvertDebugInfoSizeToString(debug_info);
-    }
-    object.try_emplace("symbolStatus", symbol_str);
-    object.try_emplace("debugInfoSize", debug_info_size);
-    char symbol_path_arr[PATH_MAX];
-    module.GetSymbolFileSpec().GetPath(symbol_path_arr,
-                                       sizeof(symbol_path_arr));
-    std::string symbol_path(symbol_path_arr);
-    object.try_emplace("symbolFilePath", symbol_path);
-  } else {
-    object.try_emplace("symbolStatus", "Symbols not found.");
-  }
-  std::string load_address =
-      llvm::formatv("{0:x}",
-                    module.GetObjectFileHeaderAddress().GetLoadAddress(target))
-          .str();
-  object.try_emplace("addressRange", load_address);
-  std::string version_str;
-  uint32_t v...
[truncated]

@da-viper
Copy link
Contributor Author

da-viper commented Jul 3, 2025

Not to sure if I should split it into different PR since they are all module related.

@da-viper da-viper force-pushed the use-protocol-modules branch from 8fcb5ae to 4a9dd25 Compare July 3, 2025 22:19
Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

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

Can you separate out the functional changes and the protocol-type-related non-functional changes?

SendJSON(llvm::json::Value(std::move(module_event)));
}
}
HandleTargetEvent(*this, event);
Copy link
Member

Choose a reason for hiding this comment

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

This change seems orthogonal to the protocol types change and should probably be its own (small) PR.

Comment on lines +19 to +20
/// Clients should only call this request if the corresponding capability
/// `supportsModulesRequest` is true.
Copy link
Member

Choose a reason for hiding this comment

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

Support for paging is another functional change that's orthogonal to switching over to the protocol types.

Comment on lines +782 to +784
/* Custom fields */
/// Size of the debug_info sections in the module.
std::string debugInfoSize;
Copy link
Member

Choose a reason for hiding this comment

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

We don't use these style of comments. We can use Doxygen groups for this:

Suggested change
/* Custom fields */
/// Size of the debug_info sections in the module.
std::string debugInfoSize;
/// Custom fields
/// @{
/// Size of the debug_info sections in the module.
std::string debugInfoSize;
/// @}

Comment on lines +60 to +63
for (size_t i = 0; i < num_sub_sections; i++) {
debug_info_size +=
GetDebugInfoSizeInSection(section.GetSubSectionAtIndex(i));
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for (size_t i = 0; i < num_sub_sections; i++) {
debug_info_size +=
GetDebugInfoSizeInSection(section.GetSubSectionAtIndex(i));
}
for (size_t i = 0; i < num_sub_sections; i++)
debug_info_size +=
GetDebugInfoSizeInSection(section.GetSubSectionAtIndex(i));

Comment on lines +70 to +72
for (size_t i = 0; i < num_sections; i++) {
debug_info_size += GetDebugInfoSizeInSection(module.GetSectionAtIndex(i));
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for (size_t i = 0; i < num_sections; i++) {
debug_info_size += GetDebugInfoSizeInSection(module.GetSectionAtIndex(i));
}
for (size_t i = 0; i < num_sections; i++)
debug_info_size += GetDebugInfoSizeInSection(module.GetSectionAtIndex(i));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants