[Lldb-commits] [PATCH] D157556: Replace the singleton "ShadowListener" with a primary and N secondary listeners

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: lldb/source/Target/Process.cpp:372
 
+const int Process::g_all_event_bits = eBroadcastBitStateChanged 
+                              | eBroadcastBitInterrupt
----------------
mib wrote:
> nit: this could probably be `constexpr`
yes, this can be a constexpr and be defined in only in the header file. No need to create storage for it by making it a real static variable


================
Comment at: lldb/source/Utility/Broadcaster.cpp:154-157
+  // The primary listener listens for all event bits:
+  if (m_primary_listener_sp)
+    return true;
+
----------------
Does the primary listener need to listen to all event bits?


================
Comment at: lldb/source/Utility/Broadcaster.cpp:229-235
+  ListenerSP primary_listener_sp = hijacking_listener_sp;
+  bool is_hijack = false;
+  
+  if (primary_listener_sp)
+    is_hijack = true;
+  else
+    primary_listener_sp = m_primary_listener_sp;
----------------
mib wrote:
> nit: We could simplify the logic here a little bit
I agree this would clean this up nicely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157556