[Lldb-commits] [lldb] [lldb-dap] Migrating DAP 'initialize' to new typed RequestHandler. (PR #133007)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Mar 25 15:00:10 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: John Harrison (ashgti)
<details>
<summary>Changes</summary>
This adds new types and helpers to support the 'initialize' request with the new typed RequestHandler. While working on this I found there were a few cases where we incorrectly treated initialize arguments as capabilities. The new `lldb_dap::protocol::InitializeRequestArguments` and `lldb_dap::protocol::Capabilities` uncovered the inconsistencies.
---
Patch is 56.73 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/133007.diff
13 Files Affected:
- (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+3-2)
- (modified) lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py (+1-2)
- (modified) lldb/tools/lldb-dap/DAP.cpp (+126-19)
- (modified) lldb/tools/lldb-dap/DAP.h (+4-2)
- (modified) lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp (+25-137)
- (modified) lldb/tools/lldb-dap/Handler/RequestHandler.cpp (+1)
- (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+57-32)
- (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+6-28)
- (modified) lldb/tools/lldb-dap/JSONUtils.h (+2-1)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+52)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+69)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp (+199)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.h (+255)
``````````diff
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 359ac718138b2..01ef4b68f2653 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
@@ -776,7 +776,8 @@ def request_initialize(self, sourceInitFile):
"supportsVariablePaging": True,
"supportsVariableType": True,
"supportsStartDebuggingRequest": True,
- "sourceInitFile": sourceInitFile,
+ "supportsProgressReporting": True,
+ "$__lldb_sourceInitFile": sourceInitFile,
},
}
response = self.send_recv(command_dict)
@@ -1261,7 +1262,7 @@ def launch(cls, /, executable, env=None, log_file=None, connection=None):
expected_prefix = "Listening for: "
out = process.stdout.readline().decode()
if not out.startswith(expected_prefix):
- self.process.kill()
+ process.kill()
raise ValueError(
"lldb-dap failed to print listening address, expected '{}', got '{}'".format(
expected_prefix, out
diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
index 0c92e5bff07c6..64c99019a1c9b 100644
--- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
+++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
@@ -524,8 +524,7 @@ def test_version(self):
# The first line is the prompt line like "(lldb) version", so we skip it.
version_eval_output_without_prompt_line = version_eval_output.splitlines()[1:]
- lldb_json = self.dap_server.get_initialize_value("__lldb")
- version_string = lldb_json["version"]
+ version_string = self.dap_server.get_initialize_value("$__lldb_version")
self.assertEqual(
version_eval_output_without_prompt_line,
version_string.splitlines(),
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 65de0488729e5..0da8ce43f73c4 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -14,6 +14,7 @@
#include "LLDBUtils.h"
#include "OutputRedirector.h"
#include "Protocol/ProtocolBase.h"
+#include "Protocol/ProtocolTypes.h"
#include "Transport.h"
#include "lldb/API/SBBreakpoint.h"
#include "lldb/API/SBCommandInterpreter.h"
@@ -1144,31 +1145,137 @@ lldb::SBValue Variables::FindVariable(uint64_t variablesReference,
return variable;
}
-llvm::StringMap<bool> DAP::GetCapabilities() {
- llvm::StringMap<bool> capabilities;
+static void mergeCapabilities(protocol::Capabilities &into,
+ const protocol::Capabilities &from) {
+ if (from.supportsConfigurationDoneRequest)
+ into.supportsConfigurationDoneRequest =
+ *from.supportsConfigurationDoneRequest;
+ if (from.supportsFunctionBreakpoints)
+ into.supportsFunctionBreakpoints = *from.supportsFunctionBreakpoints;
+ if (from.supportsConditionalBreakpoints)
+ into.supportsConditionalBreakpoints = *from.supportsConditionalBreakpoints;
+ if (from.supportsHitConditionalBreakpoints)
+ into.supportsHitConditionalBreakpoints =
+ *from.supportsHitConditionalBreakpoints;
+ if (from.supportsEvaluateForHovers)
+ into.supportsEvaluateForHovers = *from.supportsEvaluateForHovers;
+ if (from.exceptionBreakpointFilters)
+ into.exceptionBreakpointFilters = *from.exceptionBreakpointFilters;
+ if (from.supportsStepBack)
+ into.supportsStepBack = *from.supportsStepBack;
+ if (from.supportsSetVariable)
+ into.supportsSetVariable = *from.supportsSetVariable;
+ if (from.supportsRestartFrame)
+ into.supportsRestartFrame = *from.supportsRestartFrame;
+ if (from.supportsGotoTargetsRequest)
+ into.supportsGotoTargetsRequest = *from.supportsGotoTargetsRequest;
+ if (from.supportsStepInTargetsRequest)
+ into.supportsStepInTargetsRequest = *from.supportsStepInTargetsRequest;
+ if (from.supportsCompletionsRequest)
+ into.supportsCompletionsRequest = *from.supportsCompletionsRequest;
+ if (from.completionTriggerCharacters)
+ into.completionTriggerCharacters = *from.completionTriggerCharacters;
+ if (from.supportsModulesRequest)
+ into.supportsModulesRequest = *from.supportsModulesRequest;
+ if (from.additionalModuleColumns)
+ into.additionalModuleColumns = *from.additionalModuleColumns;
+ if (from.supportedChecksumAlgorithms)
+ into.supportedChecksumAlgorithms = *from.supportedChecksumAlgorithms;
+ if (from.supportsRestartRequest)
+ into.supportsRestartRequest = *from.supportsRestartRequest;
+ if (from.supportsExceptionOptions)
+ into.supportsExceptionOptions = *from.supportsExceptionOptions;
+ if (from.supportsValueFormattingOptions)
+ into.supportsValueFormattingOptions = *from.supportsValueFormattingOptions;
+ if (from.supportsExceptionInfoRequest)
+ into.supportsExceptionInfoRequest = *from.supportsExceptionInfoRequest;
+ if (from.supportTerminateDebuggee)
+ into.supportTerminateDebuggee = *from.supportTerminateDebuggee;
+ if (from.supportSuspendDebuggee)
+ into.supportSuspendDebuggee = *from.supportSuspendDebuggee;
+ if (from.supportsDelayedStackTraceLoading)
+ into.supportsDelayedStackTraceLoading =
+ *from.supportsDelayedStackTraceLoading;
+ if (from.supportsLoadedSourcesRequest)
+ into.supportsLoadedSourcesRequest = *from.supportsLoadedSourcesRequest;
+ if (from.supportsLogPoints)
+ into.supportsLogPoints = *from.supportsLogPoints;
+ if (from.supportsTerminateThreadsRequest)
+ into.supportsTerminateThreadsRequest =
+ *from.supportsTerminateThreadsRequest;
+ if (from.supportsSetExpression)
+ into.supportsSetExpression = *from.supportsSetExpression;
+ if (from.supportsTerminateRequest)
+ into.supportsTerminateRequest = *from.supportsTerminateRequest;
+ if (from.supportsDataBreakpoints)
+ into.supportsDataBreakpoints = *from.supportsDataBreakpoints;
+ if (from.supportsReadMemoryRequest)
+ into.supportsReadMemoryRequest = *from.supportsReadMemoryRequest;
+ if (from.supportsWriteMemoryRequest)
+ into.supportsWriteMemoryRequest = *from.supportsWriteMemoryRequest;
+ if (from.supportsDisassembleRequest)
+ into.supportsDisassembleRequest = *from.supportsDisassembleRequest;
+ if (from.supportsCancelRequest)
+ into.supportsCancelRequest = *from.supportsCancelRequest;
+ if (from.supportsBreakpointLocationsRequest)
+ into.supportsBreakpointLocationsRequest =
+ *from.supportsBreakpointLocationsRequest;
+ if (from.supportsClipboardContext)
+ into.supportsClipboardContext = *from.supportsClipboardContext;
+ if (from.supportsSteppingGranularity)
+ into.supportsSteppingGranularity = *from.supportsSteppingGranularity;
+ if (from.supportsInstructionBreakpoints)
+ into.supportsInstructionBreakpoints = *from.supportsInstructionBreakpoints;
+ if (from.supportsExceptionFilterOptions)
+ into.supportsExceptionFilterOptions = *from.supportsExceptionFilterOptions;
+ if (from.supportsSingleThreadExecutionRequests)
+ into.supportsSingleThreadExecutionRequests =
+ *from.supportsSingleThreadExecutionRequests;
+ if (from.supportsDataBreakpointBytes)
+ into.supportsDataBreakpointBytes = *from.supportsDataBreakpointBytes;
+ if (from.breakpointModes)
+ into.breakpointModes = *from.breakpointModes;
+ if (from.supportsANSIStyling)
+ into.supportsANSIStyling = *from.supportsANSIStyling;
+}
+
+protocol::Capabilities DAP::GetCapabilities() {
+ protocol::Capabilities capabilities;
// Supported capabilities.
- capabilities["supportTerminateDebuggee"] = true;
- capabilities["supportsDataBreakpoints"] = true;
- capabilities["supportsDelayedStackTraceLoading"] = true;
- capabilities["supportsEvaluateForHovers"] = true;
- capabilities["supportsExceptionOptions"] = true;
- capabilities["supportsLogPoints"] = true;
- capabilities["supportsProgressReporting"] = true;
- capabilities["supportsSteppingGranularity"] = true;
- capabilities["supportsValueFormattingOptions"] = true;
+ capabilities.supportTerminateDebuggee = true;
+ capabilities.supportsDataBreakpoints = true;
+ capabilities.supportsDelayedStackTraceLoading = true;
+ capabilities.supportsEvaluateForHovers = true;
+ capabilities.supportsExceptionOptions = true;
+ capabilities.supportsLogPoints = true;
+ capabilities.supportsSteppingGranularity = true;
+ capabilities.supportsValueFormattingOptions = true;
// Unsupported capabilities.
- capabilities["supportsGotoTargetsRequest"] = false;
- capabilities["supportsLoadedSourcesRequest"] = false;
- capabilities["supportsRestartFrame"] = false;
- capabilities["supportsStepBack"] = false;
+ capabilities.supportsGotoTargetsRequest = false;
+ capabilities.supportsLoadedSourcesRequest = false;
+ capabilities.supportsRestartFrame = false;
+ capabilities.supportsStepBack = false;
// Capabilities associated with specific requests.
- for (auto &kv : request_handlers) {
- for (auto &request_kv : kv.second->GetCapabilities())
- capabilities[request_kv.getKey()] = request_kv.getValue();
- }
+ for (auto &kv : request_handlers)
+ mergeCapabilities(capabilities, kv.second->GetCapabilities());
+
+ // Available filters or options for the setExceptionBreakpoints request.
+ std::vector<protocol::ExceptionBreakpointsFilter> filters;
+ for (const auto &exc_bp : *exception_breakpoints)
+ filters.emplace_back(CreateExceptionBreakpointFilter(exc_bp));
+ capabilities.exceptionBreakpointFilters = std::move(filters);
+
+ std::vector<std::string> completion_characters;
+ completion_characters.emplace_back(".");
+ completion_characters.emplace_back(" ");
+ completion_characters.emplace_back("\t");
+ capabilities.completionTriggerCharacters = std::move(completion_characters);
+
+ // Put in non-DAP specification lldb specific information.
+ capabilities.lldbVersion = debugger.GetVersionString();
return capabilities;
}
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 4b4d471161137..2f23e5a116bc3 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -16,6 +16,7 @@
#include "OutputRedirector.h"
#include "ProgressEvent.h"
#include "Protocol/ProtocolBase.h"
+#include "Protocol/ProtocolTypes.h"
#include "SourceBreakpoint.h"
#include "Transport.h"
#include "lldb/API/SBBroadcaster.h"
@@ -180,6 +181,7 @@ struct DAP {
bool enable_auto_variable_summaries;
bool enable_synthetic_child_debugging;
bool display_extended_backtrace;
+ bool supports_run_in_terminal_request;
// The process event thread normally responds to process exited events by
// shutting down the entire adapter. When we're restarting, we keep the id of
// the old process here so we can detect this case and keep running.
@@ -363,8 +365,8 @@ struct DAP {
request_handlers[Handler::GetCommand()] = std::make_unique<Handler>(*this);
}
- /// Return a key-value list of capabilities.
- llvm::StringMap<bool> GetCapabilities();
+ /// The set of capablities supported by this adapter.
+ protocol::Capabilities GetCapabilities();
/// Debuggee will continue from stopped state.
void WillContinue() { variables.Clear(); }
diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
index a8fe0d6ffce8b..b85424975a0fc 100644
--- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
@@ -9,6 +9,7 @@
#include "DAP.h"
#include "EventHelper.h"
#include "JSONUtils.h"
+#include "Protocol/ProtocolRequests.h"
#include "RequestHandler.h"
#include "lldb/API/SBEvent.h"
#include "lldb/API/SBListener.h"
@@ -229,118 +230,29 @@ static void EventThreadFunction(DAP &dap) {
}
}
-// "InitializeRequest": {
-// "allOf": [ { "$ref": "#/definitions/Request" }, {
-// "type": "object",
-// "description": "Initialize request; value of command field is
-// 'initialize'.",
-// "properties": {
-// "command": {
-// "type": "string",
-// "enum": [ "initialize" ]
-// },
-// "arguments": {
-// "$ref": "#/definitions/InitializeRequestArguments"
-// }
-// },
-// "required": [ "command", "arguments" ]
-// }]
-// },
-// "InitializeRequestArguments": {
-// "type": "object",
-// "description": "Arguments for 'initialize' request.",
-// "properties": {
-// "clientID": {
-// "type": "string",
-// "description": "The ID of the (frontend) client using this adapter."
-// },
-// "adapterID": {
-// "type": "string",
-// "description": "The ID of the debug adapter."
-// },
-// "locale": {
-// "type": "string",
-// "description": "The ISO-639 locale of the (frontend) client using
-// this adapter, e.g. en-US or de-CH."
-// },
-// "linesStartAt1": {
-// "type": "boolean",
-// "description": "If true all line numbers are 1-based (default)."
-// },
-// "columnsStartAt1": {
-// "type": "boolean",
-// "description": "If true all column numbers are 1-based (default)."
-// },
-// "pathFormat": {
-// "type": "string",
-// "_enum": [ "path", "uri" ],
-// "description": "Determines in what format paths are specified. The
-// default is 'path', which is the native format."
-// },
-// "supportsVariableType": {
-// "type": "boolean",
-// "description": "Client supports the optional type attribute for
-// variables."
-// },
-// "supportsVariablePaging": {
-// "type": "boolean",
-// "description": "Client supports the paging of variables."
-// },
-// "supportsRunInTerminalRequest": {
-// "type": "boolean",
-// "description": "Client supports the runInTerminal request."
-// }
-// },
-// "required": [ "adapterID" ]
-// },
-// "InitializeResponse": {
-// "allOf": [ { "$ref": "#/definitions/Response" }, {
-// "type": "object",
-// "description": "Response to 'initialize' request.",
-// "properties": {
-// "body": {
-// "$ref": "#/definitions/Capabilities",
-// "description": "The capabilities of this debug adapter."
-// }
-// }
-// }]
-// }
-void InitializeRequestHandler::operator()(
- const llvm::json::Object &request) const {
- llvm::json::Object response;
- FillResponse(request, response);
- llvm::json::Object body;
-
- const auto *arguments = request.getObject("arguments");
- // sourceInitFile option is not from formal DAP specification. It is only
- // used by unit tests to prevent sourcing .lldbinit files from environment
- // which may affect the outcome of tests.
- bool source_init_file =
- GetBoolean(arguments, "sourceInitFile").value_or(true);
-
+/// Initialize request; value of command field is 'initialize'.
+llvm::Expected<protocol::InitializeResponseBody> InitializeRequestHandler::Run(
+ const protocol::InitializeRequestArguments &arguments) const {
// Do not source init files until in/out/err are configured.
dap.debugger = lldb::SBDebugger::Create(false);
dap.debugger.SetInputFile(dap.in);
- auto out_fd = dap.out.GetWriteFileDescriptor();
- if (llvm::Error err = out_fd.takeError()) {
- response["success"] = false;
- EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
- dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
+
+ llvm::Expected<int> out_fd = dap.out.GetWriteFileDescriptor();
+ if (!out_fd)
+ return out_fd.takeError();
dap.debugger.SetOutputFile(lldb::SBFile(*out_fd, "w", false));
- auto err_fd = dap.err.GetWriteFileDescriptor();
- if (llvm::Error err = err_fd.takeError()) {
- response["success"] = false;
- EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
- dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
+
+ llvm::Expected<int> err_fd = dap.err.GetWriteFileDescriptor();
+ if (!err_fd)
+ return err_fd.takeError();
dap.debugger.SetErrorFile(lldb::SBFile(*err_fd, "w", false));
auto interp = dap.debugger.GetCommandInterpreter();
- if (source_init_file) {
+ // sourceInitFile option is not from formal DAP specification. It is only
+ // used by unit tests to prevent sourcing .lldbinit files from environment
+ // which may affect the outcome of tests.
+ if (arguments.sourceInitFile.value_or(true)) {
dap.debugger.SkipLLDBInitFiles(false);
dap.debugger.SkipAppInitFiles(false);
lldb::SBCommandReturnObject init;
@@ -348,59 +260,35 @@ void InitializeRequestHandler::operator()(
interp.SourceInitFileInHomeDirectory(init);
}
- if (llvm::Error err = dap.RunPreInitCommands()) {
- response["success"] = false;
- EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
- dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
+ if (llvm::Error err = dap.RunPreInitCommands())
+ return err;
dap.PopulateExceptionBreakpoints();
auto cmd = dap.debugger.GetCommandInterpreter().AddMultiwordCommand(
"lldb-dap", "Commands for managing lldb-dap.");
- if (GetBoolean(arguments, "supportsStartDebuggingRequest").value_or(false)) {
+ if (arguments.supportsStartDebuggingRequest.value_or(false)) {
cmd.AddCommand(
"start-debugging", new StartDebuggingRequestHandler(dap),
"Sends a startDebugging request from the debug adapter to the client "
"to start a child debug session of the same type as the caller.");
}
+ dap.supports_run_in_terminal_request =
+ arguments.supportsRunInTerminalRequest.value_or(false);
cmd.AddCommand(
"repl-mode", new ReplModeRequestHandler(dap),
"Get or set the repl behavior of lldb-dap evaluation requests.");
cmd.AddCommand("send-event", new SendEventRequestHandler(dap),
"Sends an DAP event to the client.");
- dap.progress_event_thread =
- std::thread(ProgressEventThreadFunction, std::ref(dap));
+ if (arguments.supportsProgressReporting)
+ dap.progress_event_thread =
+ std::thread(ProgressEventThreadFunction, std::ref(dap));
// Start our event thread so we can receive events from the debugger, target,
// process and more.
dap.event_thread = std::thread(EventThreadFunction, std::ref(dap));
- llvm::StringMap<bool> capabilities = dap.GetCapabilities();
- for (auto &kv : capabilities)
- body.try_emplace(kv.getKey(), kv.getValue());
-
- // Available filters or options for the setExceptionBreakpoints request.
- llvm::json::Array filters;
- for (const auto &exc_bp : *dap.exception_breakpoints)
- filters.emplace_back(CreateExceptionBreakpointFilter(exc_bp));
- body.try_emplace("exceptionBreakpointFilters", std::move(filters));
-
- llvm::json::Array completion_characters;
- completion_characters.emplace_back(".");
- completion_characters.emplace_back(" ");
- completion_characters.emplace_back("\t");
- body.try_emplace("completionTriggerCharacters",
- std::move(completion_characters));
-
- // Put in non-DAP specification lldb specific information.
- llvm::json::Object lldb_json;
- lldb_json.try_emplace("version", dap.debugger.GetVersionString());
- body.try_emplace("__lldb", std::move(lldb_json));
-
- response.try_emplace("body", std::move(body));
- dap.SendJSON(llvm::json::Value(std::move(response)));
+ return dap.GetCapabilities();
}
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
index 60c82649938d6..d2a0da420739e 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
@@ -201,6 +201,7 @@ BaseRequestHandler::LaunchProcess(const llvm::json::Object &request) const {
const auto timeout_seconds =
GetInteger<uint64_t>(arguments, "timeout").value_or(30);
+ // FIXME: Check dap.supports_run_in_terminal_request.
if (GetBoolean(arguments, "runInTerminal").value_or(false)) {
if (llvm::Error err = RunInTerminal(dap, request, timeout_seconds))
error.SetErrorString(llvm::toString(std::move(err)).c_str());
diff --git a/lldb/tools/lldb-dap/Handler/...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/133007
More information about the lldb-commits
mailing list