[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:

@vogelsgesang Apologies, I think I might have misunderstood your original concern. I thought the thing that worried you was that for the RequestHandlers, once you did the lookup in the map, you don't know the dynamic type of the object and so your IDE takes you to function (`operator()`) in the base class. I was arguing that if you know the dynamic type, like what this patch currently does, that's not actually a concern, because your IDE will do the right thing. But upon re-reading your message, I think you're still concerned about that. Especially if events share common logic, implementing them as classes provides a nice level of abstraction. 

As you pointed out, the RequestHandlers are trickier, but I think it can be done, although it would make the code more verbose and probably less performant than the current hash lookup + indirect call. Anyway, what I had in mind for that was a string lookup to enum and a switch. Something like this:

```
CommandType type = GetType(command_string);
switch (type) {
  case FooRequest:
    return dap.request_handlers.foo_request_handler(); 
  case BarRequest:
    [...]
}
```

Instead of registering all the request handlers (and putting them in a map), you'd have a `struct` with an instance for each handler. As I'm writing this, I'm realizing I prefer the current approach and I think the indirection is worth it. 
```

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