royitaqi wrote: Thank you, @JDevlieghere and @jimingham for the input.
IIUR, you are basically advocating for the following alternative approach (that was mentioned in the "Alternatives considered" section in the above): > Instead of changing SetDestroyCallback(), a new method AddDestroyCallback() > can be added, which use the same std::vector<std::pair<>> implementation. > Together with ClearDestroyCallback() (see below), they will replace and > deprecate SetDestroyCallback(). Meanwhile, in order to be backward > compatible, SetDestroyCallback() need to be updated to clear the std::vector > and then add the new callback. Pros: The end state is semantically more > correct. Cons: More steps to take; potentially maintaining an "incorrect" > behavior (of "overwrite"). > A new method ClearDestroyCallback() can be added. Might be unnecessary at > this point, because workflows which need to set then clear callbacks may > exist but shouldn't be too common at least for now. Such method can be added > later when needed. I agree that adding new methods have more clear naming and don't change the semantics of the eixsting method. I will update this PR (or, **should I create a new PR?** - Noob I am.) 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
