[Lldb-commits] [lldb] [lldb-dap] Updating RequestHandler to encode/decode arguments and response. (PR #130090)

[Adrian Vogelsgesang via lldb-commits]

================
@@ -57,235 +68,288 @@ class RequestHandler {
   DAP &dap;
 };
 
-class AttachRequestHandler : public RequestHandler {
-public:
-  using RequestHandler::RequestHandler;
+/// Base class for handling DAP requests. Handlers should declare their
+/// arguments and response body types like:
+///
+/// class MyRequestHandler : public RequestHandler<Arguments, ResponseBody> {
+///   ....
+/// };
+template <typename Args, typename Body>
+class RequestHandler : public BaseRequestHandler {
+  using BaseRequestHandler::BaseRequestHandler;
+
+  void operator()(const llvm::json::Object &request) const override {
+    /* no-op, the other overload handles json coding. */
+  }
+
+  void operator()(const protocol::Request &request) const override {
+    protocol::Response response;
+    response.request_seq = request.seq;
+    response.command = request.command;
+    Args arguments;
+    llvm::json::Path::Root root;
+    if (request.rawArguments &&
+        !fromJSON(request.rawArguments, arguments, root)) {
----------------
vogelsgesang wrote:

> although all the existing request's require an argument today.

Afaict, the [BreakpointLocationsArguments](https://microsoft.github.io/debug-adapter-protocol/specification#Requests_BreakpointLocations), [ConfigurationDone](https://microsoft.github.io/debug-adapter-protocol/specification#Requests_ConfigurationDone), [Disconnect](https://microsoft.github.io/debug-adapter-protocol/specification#Requests_Disconnect), [RestartRequest](https://microsoft.github.io/debug-adapter-protocol/specification#Requests_Restart) and a couple more all have an optional `arguments` parameter.

Initially, I thought we would some commands which always take arguments and some commands which never accept arguments. Apparently, I was wrong. Instead, we apparently have commands which *optionally* accept arguments, and commands which always expect arguments, but no commands which never accept arguments.

I think we should produce a clear error message for commands which miss their required arguments.
Not sure how we want to model the distinction between optional and required arguments? Maybe something like the following distinction?

```
/// Requires `arguments` to be provided
/// Not providing `arguments` is an error
class MyRequestHandler : public RequestHandler<Arguments, ResponseBody> {

/// `arguments` are optional
class MyRequestHandler : public RequestHandler<std::optional<Arguments>, ResponseBody> {
```

https://github.com/llvm/llvm-project/pull/130090