[Lldb-commits] [lldb] [lldb-dap] Adding support for well typed events. (PR #130104)

[Adrian Vogelsgesang via lldb-commits]

================
@@ -316,6 +316,36 @@ struct Source {
 bool fromJSON(const llvm::json::Value &, Source &, llvm::json::Path);
 llvm::json::Value toJSON(const Source &);
 
+// MARK: Events
+
+// "ExitedEvent": {
+//   "allOf": [ { "$ref": "#/definitions/Event" }, {
+//     "type": "object",
+//     "description": "The event indicates that the debuggee has exited and
+//     returns its exit code.", "properties": {
+//       "event": {
+//         "type": "string",
+//         "enum": [ "exited" ]
+//       },
+//       "body": {
+//         "type": "object",
+//         "properties": {
+//           "exitCode": {
+//             "type": "integer",
+//             "description": "The exit code returned from the debuggee."
+//           }
+//         },
+//         "required": [ "exitCode" ]
+//       }
+//     },
+//     "required": [ "event", "body" ]
+//   }]
+// }
+struct ExitedEventBody {
+  int exitCode;
----------------
vogelsgesang wrote:

> 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

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