On Thu, Sep 29, 2016 at 5:34 PM Daniel Austin Noland via lldb-dev < lldb-dev@lists.llvm.org> wrote:
> 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. > > This seems unnecessary. If you've got an llvm::function_ref<bool(void*, int, double, char)> then it's simply not going to compile if you pass it a mismatched callable. Is this not sufficient?
_______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
