[Lldb-commits] [PATCH] D131160: [WIP][lldb] Add "event" capability to the MainLoop class

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

To be honest, I'm not sure if "event" is the best name for it. I get it's a WSA name but it's a bit non-obvious to outsiders (I mean, technically everything is an event). Not that I have a better name in mind.



================
Comment at: lldb/include/lldb/Host/MainLoop.h:66
 
+  EventHandleUP RegisterEvent(const Callback &callback);
+
----------------
Could you include a short comment explaining how it's used? I suppose something like "will be executed upon being explicitly signaled via `NotifyEvent()` from any thread".


================
Comment at: lldb/source/Host/common/MainLoop.cpp:121
 
-  if (result >= WSA_WAIT_EVENT_0 && result < WSA_WAIT_EVENT_0 + read_events.size()) {
+  if (result >= WSA_WAIT_EVENT_0 && result <= WSA_WAIT_EVENT_0 + events.size()) {
     signaled_event = result - WSA_WAIT_EVENT_0;
----------------
I think the reliance on `<` vs `<=` here could be confusing. Perhaps add a note that we popped the "event event" above, or maybe add a `last_event` const-var prior to popping it.


================
Comment at: lldb/source/Host/common/MainLoop.cpp:133
+    loop.ProcessReadObject(fd_info.first);
+  } else {
+    // Must be called before events are processed.
----------------
Let's make sure something didn't corrupt the state.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131160/new/

https://reviews.llvm.org/D131160