[Lldb-commits] [lldb] [lldb-dap] Creating well defined structures for DAP messages. (PR #129155)

[Adrian Vogelsgesang via lldb-commits]

================
@@ -0,0 +1,182 @@
+//===-- 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::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;
+};
----------------
vogelsgesang wrote:

I think we should replace this by something like

```suggestion
struct Response {
  int64_t request_seq;
  enum State {
     Success,
     Cancelled,
     Error
  };
  State state;
  std::string command;
  std::optional<llvm::json::Value> rawBody;
};
```

i.e. replace both `message` and `success` by a single enum.

We should not use `std::optional<std::string>` for `message`, because the `message` field in the debug adapter protocol should not hold arbitrary strings. This field is meant to contain an enum-like categorization of the error. Currently, lldb-dap is misusing that `message` field for the user-facing error. However, the user-facing error should go into `ErrorResponse.message.format` instead. Using an enum for the `message` field will force us to not abuse the `message` field for messages which should be user-visible.

(This is also the reason, why VS-Code currently does not display any of the lldb-dap-reported errors to the user in the UI. We need to set `ErrorResponse.message.showUser` for that...)

Furthermore, the `message` should only be set if `success == false`. As such, I went one step further, and also merged the success state with the `message` enum.

Not sure if this is the best way to model those constraints in the C++ type system, though... WDYT? Are those restrictions something we want to encode into the types here? Is there an even better way to do so?

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