[lldb-dap] Adding support for well typed events. #130104
Conversation
lldb/tools/lldb-dap/Protocol.h
Outdated
| // }] | ||
| // } | ||
| struct ExitedEventBody { | ||
| int exitCode; |
There was a problem hiding this comment.
I think we should specify the value for the "event" property here.
After all, the comment just above
// "event": {
// "type": "string",
// "enum": [ "exited" ]
// },
also provides this piece of information.
That way, RegisterEvent(llvm::StringLiteral Event) would no longer need the Event parameter
static const char* event = "exited";
int exitCode;
There was a problem hiding this comment.
I've updated my approach to follow a similar line to the RequestHandlers. The EventHandlers can register a function that converts any arguments they need when invoking the event.
LMKWYT
There was a problem hiding this comment.
I am not convinced that the RequestHandlers abstraction is the right abstraction for events.
In general, I like to have as much "static typing" and "direct function calls" as possible. This makes reading code much simpler. E.g., "go to implementation" on a normal function directly takes me to its implementation. But if we are using a virtual interface (RequestHandlers, EventHandlers), simple things like "go to implementation" now require me to go through additional indirections.
For the code proposed in this PR, "go to implementation" on dap.onProcess() in AttachRequestHandler.cpp would take me to line 243 in DAP.h, i.e. to OutgoingEvent<> onProcess. To get to the actual implementation, I will now have to look for "who actually set this onProcess member variable?" to find the constructor, and from there I will be able to navigate to ProcessEventHandler.
In contrast, if onProcess would just be a plain old member function named sendProcessEvent, a single "go to implementation" would directly take me to the correct function body.
The RequestHandlers abstraction is necessary, because we do need an indirect dispatch based on the command string we received from the client. But for events which we are sending to the client, we don't need any indirect dispatching. We could instead stick to simpler C++ methods / functions, keeping the code simpler to read and navigate (and as additional benefit: potentially even more performant; although that perf difference probably will be so small that it won't even be measurable)
There was a problem hiding this comment.
I agree with @vogelsgesang regarding the downside of callbacks, and I pushed back against introducing more callbacks in the cancellation RFC. That said, I haven't found a reason that these things actually need to be callbacks (event the RequestHandlers). Unless we need to register them dynamically, there's really no need to do this at runtime, and we can easily have a string-switch that dispatches to the appropriate the handler. That doesn't mean those handles cannot be abstracted as classes. This is something I was planning on pursuing at some point, but I haven't had the time to get back to it.
To summarize my position: I think having an EventHandler abstraction is perfectly fine (and even desirable from an abstraction point of view) but I would encourage implementing it through a direct call instead.
There was a problem hiding this comment.
To summarize my position: I think having an EventHandler abstraction is perfectly fine (and even desirable from an abstraction point of view) but I would encourage implementing it through a direct call instead.
@JDevlieghere Could you elaborate on how the "perfect" design would look in you opinion? I currently fail to imagine the combination of "direct call" with "EventHandler abstraction" / "base class"
There was a problem hiding this comment.
@vogelsgesang Apologies, I think I might have misunderstood your original concern. I thought the thing that worried you was that for the RequestHandlers, once you did the lookup in the map, you don't know the dynamic type of the object and so your IDE takes you to function (operator()) in the base class. I was arguing that if you know the dynamic type, like what this patch currently does, that's not actually a concern, because your IDE will do the right thing. But upon re-reading your message, I think you're still concerned about that. Especially if events share common logic, implementing them as classes provides a nice level of abstraction.
As you pointed out, the RequestHandlers are trickier, but I think it can be done, although it would make the code more verbose and probably less performant than the current hash lookup + indirect call. Anyway, what I had in mind for that was a string lookup to enum and a switch. Something like this:
CommandType type = GetType(command_string);
switch (type) {
case FooRequest:
return dap.request_handlers.foo_request_handler();
case BarRequest:
[...]
}
Instead of registering all the request handlers (and putting them in a map), you'd have a struct with an instance for each handler. As I'm writing this, I'm realizing I prefer the current approach and I think the indirection is worth it.
There was a problem hiding this comment.
As I'm writing this, I'm realizing I prefer the current approach and I think the indirection is worth it.
+1. For RequestHandlers, I agree that the current abstractions are worth it.
Especially if events share common logic, implementing them as classes provides a nice level of abstraction.
Having events as separate classes makes sense (and I agree that we should keep ProtocolEvents.h which defines the event types). For event handlers, I am not sure if we really need to introduce another class hierarchy, though.
At least looking at the lldb/tools/lldb-dap/Events/EventHandler.h as currently in review, I don't see any logic being shared between event handlers. Also, I am not sure if we will ever have a need for such a reuse.
Instead of
/// Sends an event that the debuggee has exited.
ExitedEventHandler SendExited;
/// Sends an event that indicates that the debugger has begun debugging a new
/// process.
ProcessEventHandler SendProcess;
/// Sends an event that indicates the target has produced some output.
OutputEventHandler SendOutput;
I would find
/// Sends an event that the debuggee has exited.
void SendExited(lldb::SBProcess &process);
/// Sends an event that indicates that the debugger has begun debugging a new
/// process.
void SendProcess(llvm::StringRef output, OutputCategory category);
/// Sends an event that indicates the target has produced some output.
void SendOutput(SBTarget &target, ProcessStartMethod startMethod);
more straightforward
There was a problem hiding this comment.
Cool, I think we're on the same page. I'm fine with either. Unless @ashgti has an argument in favor of the classes I'm happy to go with this.
There was a problem hiding this comment.
I'll take another look at simplifying this and make a new PR, I think this got to complicated when it could be a lot more simple.
lldb/tools/lldb-dap/DAP.h
Outdated
| /// Registeres an event handler for sending Debug Adapter Protocol events. | ||
| template <typename Body> | ||
| OutgoingEvent<Body> RegisterEvent(llvm::StringLiteral Event); |
There was a problem hiding this comment.
what is the benefit of registering the events?
With the static const char* event proposed in the other comment, I think we could simply write
template<typename T>
concept EventBodyType = requires(T a)
{
{ T::event } -> std::same<const char*>;
{ toJSON(a) } -> std::same<llvm::json::Value>;
};
template <EventBodyType E>
void SendEvent(E event) {
protocol::Event Evt;
Evt.event = E::eventType;
Evt.rawBody = std::move(B);
SendJSON(std::move(Evt));
}
There was a problem hiding this comment.
I've updated this to follow the pattern used by the RequestHandlers. This allows us to a simple interface for triggering an event and then the EventHandler can build the appropriate response based on the event trigger.
ashgti
force-pushed
the
event-handler-types
branch
from
b9fb980 to
2c51a8b
Compare
This adds a mechanism for registering well typed events with the DAP. For a proof of concept, this updates the 'exited' and the 'process' event to use the new protocol serialization handlers and updates the call sites to use the new helper.
ashgti
force-pushed
the
event-handler-types
branch
from
2c51a8b to
ab4e9d8
Compare
|
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesThis adds a mechanism for registering well typed events with the DAP. For a proof of concept, this updates the 'exited' and the 'process' Full diff: https://github.com/llvm/llvm-project/pull/130104.diff 15 Files Affected:
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt
index adad75a79fa7a..54e6f3ead2695 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -37,6 +37,9 @@ add_lldb_tool(lldb-dap
Transport.cpp
Watchpoint.cpp
+ Events/ExitedEventHandler.cpp
+ Events/ProcessEventHandler.cpp
+
Handler/ResponseHandler.cpp
Handler/AttachRequestHandler.cpp
Handler/BreakpointLocationsHandler.cpp
@@ -75,8 +78,9 @@ add_lldb_tool(lldb-dap
Handler/VariablesRequestHandler.cpp
Protocol/ProtocolBase.cpp
- Protocol/ProtocolTypes.cpp
+ Protocol/ProtocolEvents.cpp
Protocol/ProtocolRequests.cpp
+ Protocol/ProtocolTypes.cpp
LINK_LIBS
liblldb
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index a1e2187288768..a2ae96eb5d967 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -8,6 +8,7 @@
#include "DAP.h"
#include "DAPLog.h"
+#include "Events/EventHandler.h"
#include "Handler/RequestHandler.h"
#include "Handler/ResponseHandler.h"
#include "JSONUtils.h"
@@ -82,7 +83,9 @@ DAP::DAP(llvm::StringRef path, std::ofstream *log,
configuration_done_sent(false), waiting_for_run_in_terminal(false),
progress_event_reporter(
[&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }),
- reverse_request_seq(0), repl_mode(default_repl_mode) {}
+ reverse_request_seq(0), repl_mode(default_repl_mode),
+ onExited(ExitedEventHandler(*this)),
+ onProcess(ProcessEventHandler(*this)) {}
DAP::~DAP() = default;
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 4c57f9fef3d89..fe902e670cf04 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -58,6 +58,9 @@ typedef llvm::StringMap<FunctionBreakpoint> FunctionBreakpointMap;
typedef llvm::DenseMap<lldb::addr_t, InstructionBreakpoint>
InstructionBreakpointMap;
+/// A debug adapter initiated event.
+template <typename... Args> using OutgoingEvent = std::function<void(Args...)>;
+
enum class OutputType { Console, Stdout, Stderr, Telemetry };
/// Buffer size for handling output events.
@@ -230,6 +233,17 @@ struct DAP {
void operator=(const DAP &rhs) = delete;
/// @}
+ /// Typed Events Handlers
+ /// @{
+
+ /// onExited sends an event that the debuggee has exited.
+ OutgoingEvent<> onExited;
+ /// onProcess sends an event that indicates that the debugger has begun
+ /// debugging a new process.
+ OutgoingEvent<> onProcess;
+
+ /// @}
+
ExceptionBreakpoint *GetExceptionBreakpoint(const std::string &filter);
ExceptionBreakpoint *GetExceptionBreakpoint(const lldb::break_id_t bp_id);
diff --git a/lldb/tools/lldb-dap/EventHelper.cpp b/lldb/tools/lldb-dap/EventHelper.cpp
index 705eb0a457d9c..7908674eb4642 100644
--- a/lldb/tools/lldb-dap/EventHelper.cpp
+++ b/lldb/tools/lldb-dap/EventHelper.cpp
@@ -32,87 +32,6 @@ static void SendThreadExitedEvent(DAP &dap, lldb::tid_t tid) {
dap.SendJSON(llvm::json::Value(std::move(event)));
}
-// "ProcessEvent": {
-// "allOf": [
-// { "$ref": "#/definitions/Event" },
-// {
-// "type": "object",
-// "description": "Event message for 'process' event type. The event
-// indicates that the debugger has begun debugging a
-// new process. Either one that it has launched, or one
-// that it has attached to.",
-// "properties": {
-// "event": {
-// "type": "string",
-// "enum": [ "process" ]
-// },
-// "body": {
-// "type": "object",
-// "properties": {
-// "name": {
-// "type": "string",
-// "description": "The logical name of the process. This is
-// usually the full path to process's executable
-// file. Example: /home/myproj/program.js."
-// },
-// "systemProcessId": {
-// "type": "integer",
-// "description": "The system process id of the debugged process.
-// This property will be missing for non-system
-// processes."
-// },
-// "isLocalProcess": {
-// "type": "boolean",
-// "description": "If true, the process is running on the same
-// computer as the debug adapter."
-// },
-// "startMethod": {
-// "type": "string",
-// "enum": [ "launch", "attach", "attachForSuspendedLaunch" ],
-// "description": "Describes how the debug engine started
-// debugging this process.",
-// "enumDescriptions": [
-// "Process was launched under the debugger.",
-// "Debugger attached to an existing process.",
-// "A project launcher component has launched a new process in
-// a suspended state and then asked the debugger to attach."
-// ]
-// }
-// },
-// "required": [ "name" ]
-// }
-// },
-// "required": [ "event", "body" ]
-// }
-// ]
-// }
-void SendProcessEvent(DAP &dap, LaunchMethod launch_method) {
- lldb::SBFileSpec exe_fspec = dap.target.GetExecutable();
- char exe_path[PATH_MAX];
- exe_fspec.GetPath(exe_path, sizeof(exe_path));
- llvm::json::Object event(CreateEventObject("process"));
- llvm::json::Object body;
- EmplaceSafeString(body, "name", std::string(exe_path));
- const auto pid = dap.target.GetProcess().GetProcessID();
- body.try_emplace("systemProcessId", (int64_t)pid);
- body.try_emplace("isLocalProcess", true);
- const char *startMethod = nullptr;
- switch (launch_method) {
- case Launch:
- startMethod = "launch";
- break;
- case Attach:
- startMethod = "attach";
- break;
- case AttachForSuspendedLaunch:
- startMethod = "attachForSuspendedLaunch";
- break;
- }
- body.try_emplace("startMethod", startMethod);
- event.try_emplace("body", std::move(body));
- dap.SendJSON(llvm::json::Value(std::move(event)));
-}
-
// Send a thread stopped event for all threads as long as the process
// is stopped.
void SendThreadStoppedEvent(DAP &dap) {
@@ -235,13 +154,4 @@ void SendContinuedEvent(DAP &dap) {
dap.SendJSON(llvm::json::Value(std::move(event)));
}
-// Send a "exited" event to indicate the process has exited.
-void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process) {
- llvm::json::Object event(CreateEventObject("exited"));
- llvm::json::Object body;
- body.try_emplace("exitCode", (int64_t)process.GetExitStatus());
- event.try_emplace("body", std::move(body));
- dap.SendJSON(llvm::json::Value(std::move(event)));
-}
-
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/EventHelper.h b/lldb/tools/lldb-dap/EventHelper.h
index 90b009c73089e..e6a54e63d00df 100644
--- a/lldb/tools/lldb-dap/EventHelper.h
+++ b/lldb/tools/lldb-dap/EventHelper.h
@@ -14,10 +14,6 @@
namespace lldb_dap {
struct DAP;
-enum LaunchMethod { Launch, Attach, AttachForSuspendedLaunch };
-
-void SendProcessEvent(DAP &dap, LaunchMethod launch_method);
-
void SendThreadStoppedEvent(DAP &dap);
void SendTerminatedEvent(DAP &dap);
@@ -26,8 +22,6 @@ void SendStdOutStdErr(DAP &dap, lldb::SBProcess &process);
void SendContinuedEvent(DAP &dap);
-void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process);
-
} // namespace lldb_dap
#endif
diff --git a/lldb/tools/lldb-dap/Events/EventHandler.h b/lldb/tools/lldb-dap/Events/EventHandler.h
new file mode 100644
index 0000000000000..f0fac0d635ce2
--- /dev/null
+++ b/lldb/tools/lldb-dap/Events/EventHandler.h
@@ -0,0 +1,57 @@
+//===-- EventHandler.h ----------------------------------------------------===//
+//
+// 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_EVENTS_EVENT_HANDLER
+#define LLDB_TOOLS_LLDB_DAP_EVENTS_EVENT_HANDLER
+
+#include "DAP.h"
+#include "Protocol/ProtocolBase.h"
+#include "Protocol/ProtocolEvents.h"
+#include "lldb/API/SBProcess.h"
+
+namespace lldb_dap {
+
+template <typename Body, typename... Args> class BaseEventHandler {
+public:
+ BaseEventHandler(DAP &dap) : dap(dap) {}
+
+ virtual ~BaseEventHandler() = default;
+
+ virtual llvm::StringLiteral getEvent() const = 0;
+ virtual Body Handler(Args...) const = 0;
+
+ void operator()(Args... args) const {
+ Body body = Handler(args...);
+ protocol::Event event{/*event=*/getEvent().str(), /*body=*/std::move(body)};
+ dap.Send(event);
+ }
+
+protected:
+ DAP &dap;
+};
+
+/// Handler for the event indicates that the debuggee has exited and returns its
+/// exit code.
+class ExitedEventHandler : public BaseEventHandler<protocol::ExitedEventBody> {
+public:
+ using BaseEventHandler::BaseEventHandler;
+ llvm::StringLiteral getEvent() const override { return "exited"; }
+ protocol::ExitedEventBody Handler() const override;
+};
+
+class ProcessEventHandler
+ : public BaseEventHandler<protocol::ProcessEventBody> {
+public:
+ using BaseEventHandler::BaseEventHandler;
+ llvm::StringLiteral getEvent() const override { return "process"; }
+ protocol::ProcessEventBody Handler() const override;
+};
+
+} // end namespace lldb_dap
+
+#endif
diff --git a/lldb/tools/lldb-dap/Events/ExitedEventHandler.cpp b/lldb/tools/lldb-dap/Events/ExitedEventHandler.cpp
new file mode 100644
index 0000000000000..010b2df3de456
--- /dev/null
+++ b/lldb/tools/lldb-dap/Events/ExitedEventHandler.cpp
@@ -0,0 +1,20 @@
+//===-- ExitedEventHandler.cpp --------------------------------------------===//
+//
+// 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 "Events/EventHandler.h"
+#include "lldb/API/SBProcess.h"
+
+namespace lldb_dap {
+
+protocol::ExitedEventBody ExitedEventHandler::Handler() const {
+ protocol::ExitedEventBody body;
+ body.exitCode = dap.target.GetProcess().GetExitStatus();
+ return body;
+}
+
+} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Events/ProcessEventHandler.cpp b/lldb/tools/lldb-dap/Events/ProcessEventHandler.cpp
new file mode 100644
index 0000000000000..52b3ae1982126
--- /dev/null
+++ b/lldb/tools/lldb-dap/Events/ProcessEventHandler.cpp
@@ -0,0 +1,34 @@
+//===-- ProcessEventHandler.cpp -------------------------------------------===//
+//
+// 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 "Events/EventHandler.h"
+#include "Protocol/ProtocolEvents.h"
+#include "lldb/API/SBProcess.h"
+#include "lldb/API/SBTarget.h"
+
+using namespace llvm;
+using namespace lldb_dap::protocol;
+
+namespace lldb_dap {
+
+ProcessEventBody ProcessEventHandler::Handler() const {
+ ProcessEventBody body;
+
+ char path[PATH_MAX] = {0};
+ dap.target.GetExecutable().GetPath(path, sizeof(path));
+ body.name = path;
+ body.systemProcessId = dap.target.GetProcess().GetProcessID();
+ body.isLocalProcess = dap.target.GetPlatform().GetName() ==
+ lldb::SBPlatform::GetHostPlatform().GetName();
+ body.startMethod = dap.is_attach ? ProcessEventBody::StartMethod::attach
+ : ProcessEventBody::StartMethod::launch;
+ body.pointerSize = dap.target.GetAddressByteSize();
+ return body;
+}
+
+} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
index 20f7c80a1ed90..95516610eb5b5 100644
--- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
@@ -201,7 +201,7 @@ void AttachRequestHandler::operator()(const llvm::json::Object &request) const {
dap.SendJSON(llvm::json::Value(std::move(response)));
if (error.Success()) {
- SendProcessEvent(dap, Attach);
+ dap.onProcess();
dap.SendJSON(CreateEventObject("initialized"));
}
}
diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
index 3262b70042a0e..d9b905b6914cc 100644
--- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
@@ -178,7 +178,7 @@ static void EventThreadFunction(DAP &dap) {
// Run any exit LLDB commands the user specified in the
// launch.json
dap.RunExitCommands();
- SendProcessExitedEvent(dap, process);
+ dap.onExited();
dap.SendTerminatedEvent();
done = true;
}
diff --git a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp
index f64c186376a36..2c8dddef9bde5 100644
--- a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp
@@ -124,10 +124,7 @@ void LaunchRequestHandler::operator()(const llvm::json::Object &request) const {
dap.SendJSON(llvm::json::Value(std::move(response)));
if (!status.Fail()) {
- if (dap.is_attach)
- SendProcessEvent(dap, Attach); // this happens when doing runInTerminal
- else
- SendProcessEvent(dap, Launch);
+ dap.onProcess();
}
dap.SendJSON(CreateEventObject("initialized"));
}
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolBase.h b/lldb/tools/lldb-dap/Protocol/ProtocolBase.h
index e10a903b80aaa..2ddfeb8ae0852 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolBase.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolBase.h
@@ -17,8 +17,8 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLDB_TOOLS_LLDB_DAP_PROTOCOL_H
-#define LLDB_TOOLS_LLDB_DAP_PROTOCOL_H
+#ifndef LLDB_TOOLS_LLDB_DAP_PROTOCOL_PROTOCOL_BASE_H
+#define LLDB_TOOLS_LLDB_DAP_PROTOCOL_PROTOCOL_BASE_H
#include "llvm/Support/JSON.h"
#include <cstdint>
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp
new file mode 100644
index 0000000000000..5c0eb911408e7
--- /dev/null
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp
@@ -0,0 +1,46 @@
+//===-- ProtocolEvents.cpp ------------------------------------------------===//
+//
+// 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 "Protocol/ProtocolEvents.h"
+#include "llvm/Support/JSON.h"
+
+using namespace llvm;
+
+namespace lldb_dap::protocol {
+
+json::Value toJSON(const ExitedEventBody &EEB) {
+ return json::Object{{"exitCode", EEB.exitCode}};
+}
+
+json::Value toJSON(const ProcessEventBody::StartMethod &m) {
+ switch (m) {
+ case ProcessEventBody::StartMethod::launch:
+ return "launch";
+ case ProcessEventBody::StartMethod::attach:
+ return "attach";
+ case ProcessEventBody::StartMethod::attachForSuspendedLaunch:
+ return "attachForSuspendedLaunch";
+ }
+}
+
+json::Value toJSON(const ProcessEventBody &PEB) {
+ json::Object result{{"name", PEB.name}};
+
+ if (PEB.systemProcessId)
+ result.insert({"systemProcessId", PEB.systemProcessId});
+ if (PEB.isLocalProcess)
+ result.insert({"isLocalProcess", PEB.isLocalProcess});
+ if (PEB.startMethod)
+ result.insert({"startMethod", PEB.startMethod});
+ if (PEB.pointerSize)
+ result.insert({"pointerSize", PEB.pointerSize});
+
+ return std::move(result);
+}
+
+} // namespace lldb_dap::protocol
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolEvents.h b/lldb/tools/lldb-dap/Protocol/ProtocolEvents.h
new file mode 100644
index 0000000000000..bea5a63dac720
--- /dev/null
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolEvents.h
@@ -0,0 +1,76 @@
+//===-- ProtocolEvents.h --------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// This file contains POD structs based on the DAP specification at
+// https://microsoft.github.io/debug-adapter-protocol/specification
+//
+// This is not meant to be a complete implementation, new interfaces are added
+// when they're needed.
+//
+// Each struct has a toJSON and fromJSON function, that converts between
+// the struct and a JSON representation. (See JSON.h)
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_TOOLS_LLDB_DAP_PROTOCOL_PROTOCOL_EVENTS_H
+#define LLDB_TOOLS_LLDB_DAP_PROTOCOL_PROTOCOL_EVENTS_H
+
+#include "lldb/lldb-types.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/JSON.h"
+
+namespace lldb_dap::protocol {
+
+// MARK: Events
+
+/// The event indicates that the debuggee has exited and returns its exit code.
+struct ExitedEventBody {
+ static llvm::StringLiteral getEvent() { return "exited"; }
+
+ /// The exit code returned from the debuggee.
+ int exitCode;
+};
+llvm::json::Value toJSON(const ExitedEventBody &);
+
+/// The event indicates that the debugger has begun debugging a new process.
+/// Either one that it has launched, or one that it has attached to.
+struct ProcessEventBody {
+ /// The logical name of the process. This is usually the full path to
+ /// process's executable file. Example: /home/example/myproj/program.js.
+ std::string name;
+
+ /// The process ID of the debugged process, as assigned by the operating
+ /// system. This property should be omitted for logical processes that do not
+ /// map to operating system processes on the machine.
+ std::optional<lldb::pid_t> systemProcessId;
+
+ /// If true, the process is running on the same computer as the debug adapter.
+ std::optional<bool> isLocalProcess;
+
+ enum class StartMethod {
+ /// Process was launched under the debugger.
+ launch,
+ /// Debugger attached to an existing process.
+ attach,
+ /// A project launcher component has launched a new process in a suspended
+ /// state and then asked the debugger to attach.
+ attachForSuspendedLaunch
+ };
+
+ /// Describes how the debug engine started debugging this process.
+ std::optional<StartMethod> startMethod;
+
+ /// The size of a pointer or address for this process, in bits. This value may
+ /// be used by clients when formatting addresses for display.
+ std::optional<int> pointerSize;
+};
+llvm::json::Value toJSON(const ProcessEventBody &);
+
+} // namespace lldb_dap::protocol
+
+#endif
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 59e31cf8e2cc8..f2ff2c48d5afc 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -9,6 +9,7 @@
#include "DAP.h"
#include "DAPLog.h"
#include "EventHelper.h"
+#include "Events/EventHandler.h"
#include "Handler/RequestHandler.h"
#include "RunInTerminal.h"
#include "Transport.h"
|
|
|
||
| /// The event indicates that the debuggee has exited and returns its exit code. | ||
| struct ExitedEventBody { | ||
| static llvm::StringLiteral getEvent() { return "exited"; } |
There was a problem hiding this comment.
seems to be unused? who calls this?
|
|
||
| void operator()(Args... args) const { | ||
| Body body = Handler(args...); | ||
| protocol::Event event{/*event=*/getEvent().str(), /*body=*/std::move(body)}; |
There was a problem hiding this comment.
should this be using Body.getEvent() instead? Afaict, we wouldn't need a virtual call to getEvent then
…lex usage. Directly setting the event handlers on the DAP object and using the previous naming convention of 'Send<event>' instead of 'on<event>'.
This adds a mechanism for registering well typed events with the DAP.
For a proof of concept, this updates the 'exited' and the 'process'
event to use the new protocol serialization handlers and updates the
call sites to use the new helper.