[Lldb-commits] [lldb] Allow multiple destroy callbacks in `SBDebugger::SetDestroyCallback()` (PR #89868)

[via lldb-commits]

jimingham wrote:

For legacy reasons, I think we have to keep SetDestroyCallbacks doing what it did, but we should comment that it is deprecated and to use the Add and Remove to only affect your own callbacks.

Jim


> On Apr 24, 2024, at 6:19 PM, royitaqi ***@***.***> wrote:
> 
> 
> @royitaqi commented on this pull request.
> 
> In lldb/include/lldb/API/SBDebugger.h <https://github.com/llvm/llvm-project/pull/89868#discussion_r1578689718>:
> 
> >    void SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
>                            void *baton);
>  
> +  void ClearDestroyCallback();
> It seems wrong that one client can only remove its destroy callback by removing ones it didn't install.
> Makes sense to me.
> 
> I think you are suggesting:
> 
> TToken AddDestroyCallback(callback, baton)
> bool RemoveDestroyCallback(TToken token) - returns success/fail
> No/remove ClearDestroyCallbacks - because no one is supposed to clear
> Then what should SetDestroyCallback be?
> 
> It clears existing callbacks, so I think in this new world this semantics isn't allowed, similar to how ClearDestroyCallbacks shouldn't exist.
> That is, unless, we introduce the concept of a client ID, where SetDestroyCallback will remove all callbacks from the specified client ID. But then I feel the introduction of the client ID opens up even more questions, and it's not an existing pattern in the Debugger class. So it's probably not a good idea.
> —
> Reply to this email directly, view it on GitHub <https://github.com/llvm/llvm-project/pull/89868#discussion_r1578689718>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUPVW5DTSDZRU33KPW67BTY7BK23AVCNFSM6AAAAABGWDO5TCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMRRGIYTQMRVGY>.
> You are receiving this because you were mentioned.
> 



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