<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Sep 29, 2016 at 5:34 PM Daniel Austin Noland via lldb-dev <<a href="mailto:lldb-dev@lists.llvm.org">lldb-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">As per discussion in another thread<br class="gmail_msg">
<br class="gmail_msg">
(<a href="http://lists.llvm.org/pipermail/lldb-dev/2016-September/011397.html" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/pipermail/lldb-dev/2016-September/011397.html</a>)<br class="gmail_msg">
<br class="gmail_msg">
I have started refactoring the private side of Break/Watchpoint.<br class="gmail_msg">
<br class="gmail_msg">
Mostly this involves fairly trivial and obvious changes and I expect the<br class="gmail_msg">
first patch to be ready fairly soon.<br class="gmail_msg">
<br class="gmail_msg">
Still, there are a couple points I would like to get feedback on.<br class="gmail_msg">
<br class="gmail_msg">
1. Having reviewed the llvm::function_ref template (see<br class="gmail_msg">
<a href="http://llvm.org/docs/doxygen/html/STLExtras_8h_source.html#l00059" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/docs/doxygen/html/STLExtras_8h_source.html#l00059</a>), I<br class="gmail_msg">
can tentatively agree that it is suitable for break/watch callbacks.<br class="gmail_msg">
<br class="gmail_msg">
The concern is the "This class does not own the callable, so it is not<br class="gmail_msg">
in general safe to store a function_ref" bit. This is not an issue<br class="gmail_msg">
provided I ensure that<br class="gmail_msg">
<br class="gmail_msg">
a) StoppointOptions does "own" the callable OR ensures its validity<br class="gmail_msg">
prior to invoking the callback<br class="gmail_msg">
<br class="gmail_msg">
AND<br class="gmail_msg">
<br class="gmail_msg">
b) ~StoppointOptions behaves responsibly in the face of this provision<br class="gmail_msg">
<br class="gmail_msg">
If we are just handed a function pointer then there is nothing to clean up.<br class="gmail_msg">
<br class="gmail_msg">
If we are handed a callable object things are a little more complex.<br class="gmail_msg">
<br class="gmail_msg">
We could make a copy of the provided object and just let the dtors clean<br class="gmail_msg">
up for us. That does put a few restrictions on the type of callback<br class="gmail_msg">
objects we can accept (e.g. user must account for side effects in<br class="gmail_msg">
ctor/dtors, object must be copy constructible, performance and memory<br class="gmail_msg">
overhead).<br class="gmail_msg">
<br class="gmail_msg">
We could require move semantics.<br class="gmail_msg">
<br class="gmail_msg">
We could use unique_ptr, shared_ptr, weak_ptr.<br class="gmail_msg">
<br class="gmail_msg">
Feelings?<br class="gmail_msg">
<br class="gmail_msg">
2. llvm::function_ref and std::function are great, but they accept ANY<br class="gmail_msg">
callable.<br class="gmail_msg">
<br class="gmail_msg">
When I write templates I generally write helper classes which try to<br class="gmail_msg">
validate the template params.<br class="gmail_msg">
<br class="gmail_msg">
Here is a simple sketch of the type of thing I mean:<br class="gmail_msg">
<br class="gmail_msg">
```cpp<br class="gmail_msg">
<br class="gmail_msg">
#include <type_traits><br class="gmail_msg">
namespace callback {<br class="gmail_msg">
<br class="gmail_msg">
template <class Callable><br class="gmail_msg">
struct IsReturnTypeValid {<br class="gmail_msg">
static constexpr bool value = false;<br class="gmail_msg">
};<br class="gmail_msg">
<br class="gmail_msg">
template <class Ret, class ...OtherParams><br class="gmail_msg">
struct IsReturnTypeValid<Ret(OtherParams...)> {<br class="gmail_msg">
static constexpr bool value = std::is_same<<br class="gmail_msg">
typename std::remove_const<Ret>::type,<br class="gmail_msg">
bool<br class="gmail_msg">
>::value;<br class="gmail_msg">
};<br class="gmail_msg">
<br class="gmail_msg">
template <class Callable><br class="gmail_msg">
constexpr<br class="gmail_msg">
void assert_valid_callback() {<br class="gmail_msg">
static_assert(IsReturnTypeValid<Callable>::value,<br class="gmail_msg">
"Return type of Stoppoint callback must be bool!");<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
} // end namespace ::callback<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
bool valid(void* userData, int some, double other, char params) {<br class="gmail_msg">
return true;<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
double invalid(int stuff, double moreStuff) {<br class="gmail_msg">
return 0;<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
int main(void) {<br class="gmail_msg">
<br class="gmail_msg">
// compiles<br class="gmail_msg">
<br class="gmail_msg">
callback::assert_valid_callback<decltype(valid)>();<br class="gmail_msg">
// will not compile<br class="gmail_msg">
<br class="gmail_msg">
callback::assert_valid_callback<decltype(invalid)>();<br class="gmail_msg">
return 0;<br class="gmail_msg">
}<br class="gmail_msg">
```<br class="gmail_msg">
<br class="gmail_msg">
I find that the makes debugging far easier for people using the library.<br class="gmail_msg">
<br class="gmail_msg"></blockquote><div>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?</div><div><br></div><div> </div></div></div>