jingham added inline comments.

================
Comment at: lldb/include/lldb/API/SBDebugger.h:46
 
+  static const char *GetBroadcasterClass();
+
----------------
clayborg wrote:
> @jingham: do we need this GetBroadcasterClass()? Or would this only be used 
> to listen to all debugger events as new debuggers are created? Also see my 
> question later that asks if we need the broadcast manager in the Broadcaster 
> we are using in the Debugger.h/Debugger.cpp.
It is needed by the mechanism for waiting for the events from all broadcasters 
of a given type.  Not sure why you wouldn't want to do this for Debugger events.

GetBroadcasterClass is used in the Debugger DefaultEventHandler to separate the 
treatment for the various event sources the Debugger waits for.  One of the 
missing pieces of this patch is to get the Driver's default event handling to 
do something with progress updates.  I presume that would go through the same 
DefaultEventHandler, and you'd use the comparison of GetStaticBroadcasterClass 
and GetBroadcasterClass in the same way that code does for all the other event 
classes.


================
Comment at: lldb/source/Core/Debugger.cpp:672
       Properties(std::make_shared<OptionValueProperties>()),
+      Broadcaster(nullptr, GetStaticBroadcasterClass().AsCString()),
       m_input_file_sp(std::make_shared<NativeFile>(stdin, false)),
----------------
clayborg wrote:
> @jingham: do we need to the m_broadcaster_manager_sp in this broadcaster? 
> Debugger inherits from Broadcaster, but we could change this to "has a" 
> instead of "is a" if we need the broadcast manager. 
Given that at least in the first pass all the events are not specific to a 
debugger, in a Multi-Debugger UI I could very well imagine that instead of 
having any individual debugger's Listener listen for these events, you would 
make some thread listen for them all, and then put up some System Wide progress 
UI.  That would make it much less confusing than seeming to associate events 
with debuggers you don't know they pertain to.  

And even when we start reporting Progress that was associated with a Debugger, 
we will still have all these unassociated events from the module cache.  So you 
could still imagine having a listener for all debuggers, that only reports 
progress that has no debugger associated with it, and then have individual 
Debugger's report progress pertaining directly to them.

So I do think we need to have a BroadcasterManager that allows you to sign up 
for all Debugger events from any debugger that will be created.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97739

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to