labath added a comment. In D100418#2689425 <https://reviews.llvm.org/D100418#2689425>, @mgorny wrote:
> In D100418#2689348 <https://reviews.llvm.org/D100418#2689348>, @labath wrote: > >> I've been thinking about whether this should be done here, or via a separate >> multiplexer entity. It's not clear to me which one is cleaner so I suppose >> we can go with what you have done here. >> >> That said, I'm not too happy about this callback_id business. IIUC, it's >> only there to enable removing a specific callback (because you can't have a >> set of std::functions). But there are other ways to achieve that, and they >> don't leak this detail to the users. One option would be to hold the >> callbacks in a std::list, and use the (stable) iterator as the ID.... >> >> It could also use a test... something like, register a callback, fire a >> signal and check it's called, registers a second callback and check that >> both are called, unregister the first one and check that only the second one >> is called, ... > > Hmm, my first though was to use a `static int` that increases with every > call. But the `std::list` idea is interesting, so I'll explore that. That would work too, though it should definitely be a member variable instead of a global (thread safety). The only difference should be in the memory footprint. I suspect the list solution will be slightly smaller, unless you use DenseMap. OTOH, a DenseMap with only a few items tends to waste space... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100418/new/ https://reviews.llvm.org/D100418 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
