[Lldb-commits] [lldb] Add SBDebugger:: AddNotificationCallback API (PR #111206)

[via lldb-commits]

jimingham wrote:

> (This is in reply to Jim's comment [here](https://github.com/llvm/llvm-project/pull/111206#issuecomment-2400376223). I can't quote it, because github garbles it beyond recognition, probably because it was sent by email :/)
 
I'm trying to remember not to believe the bit where the footer in the email says you can reply to it directly.  But it's SO convenient...

> 
> You're right, I have misunderstood the intention. The thing that threw me off was the discussion of about the callback signature (which still looks like it wants to be universal). Given that, I'd like to agree with what you said in the last part of your email -- I think it'd be better for the callback signatures to not be universal as well (no one-signature-to-rule-them-all). Here are my reasons for that:
> 
> * we sidestep the "should execution context contain a debugger" question -- the debugger callback type can just take a debugger argument, and that's it. A thread callback type can take an SBThread instead of getting an SBExecutionContext and wondering what's inside it.

I don't think we need to have very many of these signatures, but I can think of more than just ExecutionContext ones pretty easily.   Your example of Symbol side ones later on is the other really obvious example.  And as you say, notifications are fine if you don't need to do something before the rest of the system sees the change, but sometimes you very much want to do that, particularly for module loading.  So having a callback for this makes a lot of sense.

It also seems to me that Platform lifecycle events would be something you might want callbacks for, and they don't neatly fit in the ExecutionContext mold.

It's worthwhile to have a unified system for this.  For instance, sometimes you want to observe and record information about some change, but you really don't want to junk up the console output with the results of that information.  So having a way for the callback to produce a stored result, and then some commands to say "show me the stored results for callback X" or even "show me the stored results for all callbacks between StopID 100 and StopID 200, etc. would be handy.  It would be sad if callback writers had to roll their own version of this sort of convenience...

> * we sidestep the "what is the scope of lower-level callback types" question -- it's clear that debugger lifetime callbacks have to be global, but for example, I think it would be reasonable to have e.g. target creation callbacks scoped to the debugger they are being created in (so you can monitor targets in one debugger but not interfere with other debuggers you possibly know nothing about). With the current design we're basically committing to global callbacks, but with the other one, we leave the option open.

Yes, particularly in environments like Xcode where all the concurrent debug session to a device share one lldb, it would be useful to scope these at least to the debugger.  I think it's worth figuring out a convenient way to do this.

> * we can pass event-specific data to the callbacks. For example, for my use case, the ideal event type would be "module got added to a target" (basically, a synchronous version of (eBroadcastBitModulesLoaded) or "target's executable module has changed". Fitting that into a universal callback type would be very tricky.
> 
> If we do this right, I don't believe this should result in a lot of duplicated code, as we could write (template?) code to handle the (de)registration in a generic way.

Registration should just require a callback type, that part should be easy.  To do this in a generic manner a callback entity would also need a filter, and a "callback-hit-instance-data".  The filter would take the instance data, but and get a yes or no back.  Other than that the generic callback management shouldn't need to reason about either of these bits, just pass them to the filter and the callback.  This should be amenable to a generic approach.



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