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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
