Skip to content

Conversation

@ashgti
Copy link
Contributor

@ashgti ashgti commented Feb 27, 2025

This adds a new Protocol.{h,cpp} for defining structured types that represent Debug Adapter Protocol messages.

This adds static types to define well structure messages for the protocol. This iteration includes only the basic Event, Request and Response types.

These types help simplify and improve the validation of messages and give us additional static type checks on the overall structure of DAP messages, compared to today where we tend to use llvm::json::Value directly.

In a follow-up patch I plan on adding more types as need to allow for incrementally migrating raw llvm::json::Value usage to well defined types.

This adds a new Protocol.{h,cpp} for defining structured types that represent Debug Adapter Protocol messages.

This adds static types to define well structure messages for the protocol. This iteration includes only the basic `Event`, `Request` and `Response` types.

In a follow-up patch I plan on adding more types as need to allow for incrementally migrating raw `llvm::json::Value` usage to well defined types.
@ashgti
Copy link
Contributor Author

ashgti commented Feb 27, 2025

For reference, this is part of a larger refactor to support 'cancel' requests. I have a working prototype in https://github.com/ashgti/llvm-project/tree/lldb-dap-cancel that I am splitting up into smaller individual patches.

@ashgti ashgti changed the title [lldb-dap] Creating a new set of types for handling DAP messages. [lldb-dap] Creating well defined structures for DAP messages. Feb 27, 2025
@ashgti ashgti marked this pull request as ready for review February 27, 2025 23:57
@ashgti ashgti requested a review from JDevlieghere as a code owner February 27, 2025 23:57
@llvmbot
Copy link
Member

llvmbot commented Feb 27, 2025

@llvm/pr-subscribers-lldb

Author: John Harrison (ashgti)

Changes

This adds a new Protocol.{h,cpp} for defining structured types that represent Debug Adapter Protocol messages.

This adds static types to define well structure messages for the protocol. This iteration includes only the basic Event, Request and Response types.

These types help simplify and improve the validation of messages and give us additional static type checks on the overall structure of DAP messages, compared to today where we tend to use llvm::json::Value directly.

In a follow-up patch I plan on adding more types as need to allow for incrementally migrating raw llvm::json::Value usage to well defined types.


Full diff: https://github.com/llvm/llvm-project/pull/129155.diff

3 Files Affected:

  • (modified) lldb/tools/lldb-dap/CMakeLists.txt (+1)
  • (added) lldb/tools/lldb-dap/Protocol.cpp (+191)
  • (added) lldb/tools/lldb-dap/Protocol.h (+187)
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt
index 8b3c520ec4360..9a2d604f4d573 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -35,6 +35,7 @@ add_lldb_tool(lldb-dap
   ProgressEvent.cpp
   RunInTerminal.cpp
   SourceBreakpoint.cpp
+  Protocol.cpp
   Watchpoint.cpp
 
   Handler/ResponseHandler.cpp
diff --git a/lldb/tools/lldb-dap/Protocol.cpp b/lldb/tools/lldb-dap/Protocol.cpp
new file mode 100644
index 0000000000000..71c4dc080a5b1
--- /dev/null
+++ b/lldb/tools/lldb-dap/Protocol.cpp
@@ -0,0 +1,191 @@
+//===-- Protocol.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.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/JSON.h"
+#include <optional>
+#include <utility>
+
+namespace llvm {
+namespace json {
+bool fromJSON(const llvm::json::Value &Params, llvm::json::Value &V,
+              llvm::json::Path P) {
+  V = std::move(Params);
+  return true;
+}
+} // namespace json
+} // namespace llvm
+
+namespace lldb_dap {
+namespace protocol {
+
+enum class MessageType { request, response, event };
+
+bool fromJSON(const llvm::json::Value &Params, MessageType &M,
+              llvm::json::Path P) {
+  auto rawType = Params.getAsString();
+  if (!rawType) {
+    P.report("expected a string");
+    return false;
+  }
+  std::optional<MessageType> type =
+      llvm::StringSwitch<std::optional<MessageType>>(*rawType)
+          .Case("request", MessageType::request)
+          .Case("response", MessageType::response)
+          .Case("event", MessageType::event)
+          .Default(std::nullopt);
+  if (!type) {
+    P.report("unexpected value");
+    return false;
+  }
+  M = *type;
+  return true;
+}
+
+llvm::json::Value toJSON(const Request &R) {
+  llvm::json::Object Result{
+      {"type", "request"},
+      {"seq", R.seq},
+      {"command", R.command},
+  };
+  if (R.rawArguments)
+    Result.insert({"arguments", R.rawArguments});
+  return std::move(Result);
+}
+
+bool fromJSON(llvm::json::Value const &Params, Request &R, llvm::json::Path P) {
+  llvm::json::ObjectMapper O(Params, P);
+  MessageType type;
+  if (!O.map("type", type)) {
+    return false;
+  }
+  if (type != MessageType::request) {
+    P.field("type").report("expected to be 'request'");
+    return false;
+  }
+
+  return O && O.map("command", R.command) && O.map("seq", R.seq) &&
+         O.map("arguments", R.rawArguments);
+}
+
+llvm::json::Value toJSON(const Response &R) {
+  llvm::json::Object Result{{"type", "response"},
+                            {"req", 0},
+                            {"command", R.command},
+                            {"request_seq", R.request_seq},
+                            {"success", R.success}};
+
+  if (R.message)
+    Result.insert({"message", R.message});
+  if (R.rawBody)
+    Result.insert({"body", R.rawBody});
+
+  return std::move(Result);
+}
+
+bool fromJSON(llvm::json::Value const &Params, Response &R,
+              llvm::json::Path P) {
+  llvm::json::ObjectMapper O(Params, P);
+  MessageType type;
+  if (!O.map("type", type)) {
+    return false;
+  }
+  if (type != MessageType::response) {
+    P.field("type").report("expected to be 'response'");
+    return false;
+  }
+  return O && O.map("command", R.command) &&
+         O.map("request_seq", R.request_seq) && O.map("success", R.success) &&
+         O.mapOptional("message", R.message) &&
+         O.mapOptional("body", R.rawBody);
+}
+
+llvm::json::Value toJSON(const Event &E) {
+  llvm::json::Object Result{
+      {"type", "event"},
+      {"seq", 0},
+      {"event", E.event},
+  };
+  if (E.rawBody)
+    Result.insert({"body", E.rawBody});
+  if (E.statistics)
+    Result.insert({"statistics", E.statistics});
+  return std::move(Result);
+}
+
+bool fromJSON(llvm::json::Value const &Params, Event &E, llvm::json::Path P) {
+  llvm::json::ObjectMapper O(Params, P);
+  MessageType type;
+  if (!O.map("type", type)) {
+    return false;
+  }
+  if (type != MessageType::event) {
+    P.field("type").report("expected to be 'event'");
+    return false;
+  }
+
+  return O && O.map("event", E.event) && O.mapOptional("body", E.rawBody) &&
+         O.mapOptional("statistics", E.statistics);
+}
+
+bool fromJSON(const llvm::json::Value &Params, ProtocolMessage &PM,
+              llvm::json::Path P) {
+  llvm::json::ObjectMapper O(Params, P);
+  if (!O)
+    return false;
+
+  MessageType type;
+  if (!O.map("type", type))
+    return false;
+
+  switch (type) {
+  case MessageType::request: {
+    Request req;
+    if (!fromJSON(Params, req, P)) {
+      return false;
+    }
+    PM = std::move(req);
+    return true;
+  }
+  case MessageType::response: {
+    Response resp;
+    if (!fromJSON(Params, resp, P)) {
+      return false;
+    }
+    PM = std::move(resp);
+    return true;
+  }
+  case MessageType::event:
+    Event evt;
+    if (!fromJSON(Params, evt, P)) {
+      return false;
+    }
+    PM = std::move(evt);
+    return true;
+  }
+  llvm_unreachable("Unsupported protocol message");
+}
+
+llvm::json::Value toJSON(const ProtocolMessage &PM) {
+  if (auto const *Req = std::get_if<Request>(&PM)) {
+    return toJSON(*Req);
+  }
+  if (auto const *Resp = std::get_if<Response>(&PM)) {
+    return toJSON(*Resp);
+  }
+  if (auto const *Evt = std::get_if<Event>(&PM)) {
+    return toJSON(*Evt);
+  }
+  llvm_unreachable("Unsupported protocol message");
+}
+
+} // namespace protocol
+} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Protocol.h b/lldb/tools/lldb-dap/Protocol.h
new file mode 100644
index 0000000000000..0d38f8b249094
--- /dev/null
+++ b/lldb/tools/lldb-dap/Protocol.h
@@ -0,0 +1,187 @@
+//===-- Protocol.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_H
+#define LLDB_TOOLS_LLDB_DAP_PROTOCOL_H
+
+#include "llvm/Support/JSON.h"
+#include <cstddef>
+#include <cstdint>
+#include <optional>
+#include <string>
+#include <variant>
+
+namespace lldb_dap {
+namespace protocol {
+
+// MARK: Base Protocol
+
+// "Request": {
+//   "allOf": [ { "$ref": "#/definitions/ProtocolMessage" }, {
+//     "type": "object",
+//     "description": "A client or debug adapter initiated request.",
+//     "properties": {
+//       "type": {
+//         "type": "string",
+//         "enum": [ "request" ]
+//       },
+//       "command": {
+//         "type": "string",
+//         "description": "The command to execute."
+//       },
+//       "arguments": {
+//         "type": [ "array", "boolean", "integer", "null", "number" , "object",
+//         "string" ], "description": "Object containing arguments for the
+//         command."
+//       }
+//     },
+//     "required": [ "type", "command" ]
+//   }]
+// },
+struct Request {
+  int64_t seq;
+  std::string command;
+  std::optional<llvm::json::Value> rawArguments;
+};
+llvm::json::Value toJSON(const Request &);
+bool fromJSON(const llvm::json::Value &, Request &, llvm::json::Path);
+
+// "Event": {
+//   "allOf": [ { "$ref": "#/definitions/ProtocolMessage" }, {
+//     "type": "object",
+//     "description": "A debug adapter initiated event.",
+//     "properties": {
+//       "type": {
+//         "type": "string",
+//         "enum": [ "event" ]
+//       },
+//       "event": {
+//         "type": "string",
+//         "description": "Type of event."
+//       },
+//       "body": {
+//         "type": [ "array", "boolean", "integer", "null", "number" , "object",
+//         "string" ], "description": "Event-specific information."
+//       }
+//     },
+//     "required": [ "type", "event" ]
+//   }]
+// },
+struct Event {
+  std::string event;
+  std::optional<llvm::json::Value> rawBody;
+
+  /// lldb-dap specific extension on the 'terminated' event specifically.
+  std::optional<llvm::json::Value> statistics;
+};
+llvm::json::Value toJSON(const Event &);
+bool fromJSON(const llvm::json::Value &, Event &, llvm::json::Path);
+
+// "Response" : {
+//   "allOf" : [
+//     {"$ref" : "#/definitions/ProtocolMessage"}, {
+//       "type" : "object",
+//       "description" : "Response for a request.",
+//       "properties" : {
+//         "type" : {"type" : "string", "enum" : ["response"]},
+//         "request_seq" : {
+//           "type" : "integer",
+//           "description" : "Sequence number of the corresponding request."
+//         },
+//         "success" : {
+//           "type" : "boolean",
+//           "description" :
+//               "Outcome of the request.\nIf true, the request was successful "
+//               "and the `body` attribute may contain the result of the "
+//               "request.\nIf the value is false, the attribute `message` "
+//               "contains the error in short form and the `body` may contain "
+//               "additional information (see `ErrorResponse.body.error`)."
+//         },
+//         "command" :
+//             {"type" : "string", "description" : "The command requested."},
+//         "message" : {
+//           "type" : "string",
+//           "description" :
+//               "Contains the raw error in short form if `success` is "
+//               "false.\nThis raw error might be interpreted by the client and
+//               " "is not shown in the UI.\nSome predefined values exist.",
+//           "_enum" : [ "cancelled", "notStopped" ],
+//           "enumDescriptions" : [
+//             "the request was cancelled.", "the request may be retried once
+//             the "
+//                                           "adapter is in a 'stopped' state."
+//           ]
+//         },
+//         "body" : {
+//           "type" : [
+//             "array", "boolean", "integer", "null", "number", "object",
+//             "string"
+//           ],
+//           "description" : "Contains request result if success is true and "
+//                           "error details if success is false."
+//         }
+//       },
+//       "required" : [ "type", "request_seq", "success", "command" ]
+//     }
+//   ]
+// }
+struct Response {
+  int64_t request_seq;
+  bool success;
+  std::string command;
+  std::optional<std::string> message;
+  std::optional<llvm::json::Value> rawBody;
+};
+bool fromJSON(const llvm::json::Value &, Response &, llvm::json::Path);
+llvm::json::Value toJSON(const Response &);
+
+// A void response body for any response without a specific value.
+using VoidResponseBody = std::nullptr_t;
+
+// "ProtocolMessage": {
+//   "type": "object",
+//   "title": "Base Protocol",
+//   "description": "Base class of requests, responses, and events.",
+//   "properties": {
+//     "seq": {
+//       "type": "integer",
+//       "description": "Sequence number of the message (also known as
+//       message ID). The `seq` for the first message sent by a client or
+//       debug adapter is 1, and for each subsequent message is 1 greater
+//       than the previous message sent by that actor. `seq` can be used to
+//       order requests, responses, and events, and to associate requests
+//       with their corresponding responses. For protocol messages of type
+//       `request` the sequence number can be used to cancel the request."
+//     },
+//     "type": {
+//       "type": "string",
+//       "description": "Message type.",
+//       "_enum": [ "request", "response", "event" ]
+//     }
+//   },
+//   "required": [ "seq", "type" ]
+// },
+using ProtocolMessage = std::variant<Request, Response, Event>;
+bool fromJSON(const llvm::json::Value &, ProtocolMessage &, llvm::json::Path);
+llvm::json::Value toJSON(const ProtocolMessage &);
+
+} // namespace protocol
+} // namespace lldb_dap
+
+#endif

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.

LGTM modulo inline comments (mostly code style).

Copy link
Collaborator

@labath labath left a comment

Choose a reason for hiding this comment

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

Looks except for the part where you're defining a function in the llvm namespace. That's the way towards ODR violations. I think that function needs to be in our own namespace, or defined (in another PR) in the llvm header which defines those types.

It's funny that you're using the variant class, as I was just yesterday working on another patch which introduces std::variant in a completely different part of lldb. I'm going to take that as a sign.

Comment on lines 17 to 25
namespace llvm {
namespace json {
bool fromJSON(const llvm::json::Value &Params, llvm::json::Value &V,
llvm::json::Path P) {
V = std::move(Params);
return true;
}
} // namespace json
} // namespace llvm
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's this about? Why are we defining a function in a namespace we don't own, for argument types we don't own?

Copy link
Contributor Author

@ashgti ashgti Feb 28, 2025

Choose a reason for hiding this comment

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

In each of the Event, Response, Request types I have a raw<prop> field that is used to collect the property for later decoding.

In ashgti@f32d984 from my prototype I have additional helpers to decode the fields. That updates the handlers to RequestHandler<ArgsType, ResponeBodyType> like:

class SourceRequestHandler
    : public RequestHandler<protocol::SourceArguments,
                            protocol::SourceResponseBody> {
public:
  using RequestHandler::RequestHandler;
  static llvm::StringLiteral getCommand() { return "source"; }
  llvm::Expected<protocol::SourceResponseBody>
  Execute(const protocol::SourceArguments &args) const override;
};

Which does the decoding of the rawArguments into the arguments and the return into the rawBody for the response.

llvm::json::Value toJSON(const Response &);

// A void response body for any response without a specific value.
using VoidResponseBody = std::nullptr_t;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not yet, I can remove this for now and add it to a downstream PR.

@ashgti
Copy link
Contributor Author

ashgti commented Feb 28, 2025

I also added some additional validations like checking that an Event has a "seq": 0 (per the spec) and to make sure various strings are not empty.

Copy link
Member

@walter-erquinigo walter-erquinigo left a comment

Choose a reason for hiding this comment

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

beautiful

@ashgti ashgti merged commit 3f12155 into llvm:main Mar 5, 2025
10 checks passed
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.

6 participants