As per discussion in another thread (http://lists.llvm.org/pipermail/lldb-dev/2016-September/011397.html)
I have started refactoring the private side of Break/Watchpoint. Mostly this involves fairly trivial and obvious changes and I expect the first patch to be ready fairly soon. Still, there are a couple points I would like to get feedback on. 1. Having reviewed the llvm::function_ref template (see http://llvm.org/docs/doxygen/html/STLExtras_8h_source.html#l00059), I can tentatively agree that it is suitable for break/watch callbacks. The concern is the "This class does not own the callable, so it is not in general safe to store a function_ref" bit. This is not an issue provided I ensure that a) StoppointOptions does "own" the callable OR ensures its validity prior to invoking the callback AND b) ~StoppointOptions behaves responsibly in the face of this provision If we are just handed a function pointer then there is nothing to clean up. If we are handed a callable object things are a little more complex. We could make a copy of the provided object and just let the dtors clean up for us. That does put a few restrictions on the type of callback objects we can accept (e.g. user must account for side effects in ctor/dtors, object must be copy constructible, performance and memory overhead). We could require move semantics. We could use unique_ptr, shared_ptr, weak_ptr. Feelings? 2. llvm::function_ref and std::function are great, but they accept ANY callable. When I write templates I generally write helper classes which try to validate the template params. Here is a simple sketch of the type of thing I mean: ```cpp #include <type_traits> namespace callback { template <class Callable> struct IsReturnTypeValid { static constexpr bool value = false; }; template <class Ret, class ...OtherParams> struct IsReturnTypeValid<Ret(OtherParams...)> { static constexpr bool value = std::is_same< typename std::remove_const<Ret>::type, bool >::value; }; template <class Callable> constexpr void assert_valid_callback() { static_assert(IsReturnTypeValid<Callable>::value, "Return type of Stoppoint callback must be bool!"); } } // end namespace ::callback bool valid(void* userData, int some, double other, char params) { return true; } double invalid(int stuff, double moreStuff) { return 0; } int main(void) { // compiles callback::assert_valid_callback<decltype(valid)>(); // will not compile callback::assert_valid_callback<decltype(invalid)>(); return 0; } ``` I find that the makes debugging far easier for people using the library. I also find that writing this kind of SFINAE stuff can be difficult and time consuming. Do you think this sort of validation is actually an asset (especially since it will only be available on the private side of things in the near term)?
signature.asc
Description: OpenPGP digital signature
_______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
