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

[Jonas Devlieghere 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;
----------------
JDevlieghere wrote:

I agree with @vogelsgesang regarding the downside of callbacks, and I pushed back against introducing more callbacks in the [cancellation RFC](https://discourse.llvm.org/t/rfc-lldb-dap-refactoring-to-support-async-operations-and-cancellation/84739). 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.

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