[cfe-dev] libcxx: std::function should be able to capture blocks

[Jared Grubb]

> On Oct 9, 2014, at 09:47, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Thu, Oct 9, 2014 at 12:11 AM, Jared Grubb <jared.grubb at gmail.com <mailto:jared.grubb at gmail.com>> wrote:
> 
>> On Oct 3, 2014, at 14:50, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>> On Fri, Oct 3, 2014 at 12:55 PM, Jared Grubb <jared.grubb at gmail.com <mailto:jared.grubb at gmail.com>> wrote:
>> I spoke with Marshall briefly about this at CppCon. My suggestion is that std::function should either:
>>  * correctly capture blocks and just work (as this patch does)
>>  * compile with error if you try it (and also maybe add a convenience adaptor like you suggest to allow it to manually work)
>> 
>> The current problem is that if you write something like this:
>>    template <typename F> doSomething(F&& functor) { std::function<...> capturing = std::forward<F>(functor); ... }
>> This works great for lambdas. For blocks, it compiles just fine but has runtime issues.
>> 
>> What runtime issues are you referring to? Could you provide a simple standalone complete example?
> 
> Here's an example.
> 
> int main(int argc, const char**) {
>     std::function<void()> f;
> 
>     std::vector<int> vec = {1, 2, 3};
>     if (argc == 1) {
>         f = ^{ std::cout << vec.size() << " (block)\n"; };
>     } else {
>         f = [=]{ std::cout << vec.size() << " (lambda)\n"; };
>     }
> 
>     f();
> }
> 
> On my machine, it doesnt crash, but the block part is definitely broken:
> 
> Fair enough.
>  
> $ ./a.out
> 0 (block)
> $ ./a.out 1
> 3 (lambda)
> 
> Other examples, especially multithreaded ones, will crash and behave in undefined ways.
> 
> Hmm - which kind of multithreading do you have in mind? I'm not sure why that'd be a particular problem.

Most of my uses of blocks are in multi-threading cases, where we throw blocks across threads (or dispatch_queues in OSX). Capturing blocks on one thread stack and running them on another stack exacerbates the core problem of "bad pointer to stack".

>> I assume the issue is that std::function will capture the block's (presumably raw) pointer by value but do none of the increment/decrement.
> 
> Yes.
> 
> Would it be possible/reasonable to warn/error on copying the raw block pointer? (passing it to any function at all) std::function isn't the only entity that could cause this kind of dangling block pointer bug & it might be nicer to fix it in the language for all such cases, than fix one (albeit common/easily triggered) case in the library?
> 
> I assume I could write the same bug with something like:
> 
> T *t; // not sure what 'T' is, but I imagine it's writeable?
> if (x) {
>   t = ^{ std::cout << vec.size() << " (block)\n"; };
> }
> t(); 

You make a great point here, and I think such a warning would be amazing. Block assignments that span different scopes have problems for all the reasons we're describing.

But I think that warning is orthogonal to this patch. Another example that I'm hoping to solve with this patch is something like this, where 'callback' is a std::function:

template <typename F>
void Foo::setCallback(F&& functor) {
	this->callback = std::forward<F>(functor);
}

Without this patch, you need to either overload for blocks, or use some sort of "make_function" helper that overloads on blocks.

My goal with this patch is to just make std::function accept block types correctly; it already captures them, it just does it incorrectly.

> 
>> This doesn't seem necessarily broken & I'm not sure (for myself) it merits library support to protect.
>> 
>> This would be the same behavior as any other lifetime issues with pointer captures.
>> 
>>   std::function<...> func() {
>>     int i;
>>     int *j = &i;
>>     return [=j] () { ... }; // captured j by value, but it points to something that goes out of scope at the end of the function. Bad.
>>   }
>> 
>> But similar code isn't a problem at all:
>> 
>>   void func(std::function<...>); // func calls the std::function, maybe copies it around, but doesn't copy it to global storage/anywhere that outlives the 
>>   ...
>>   int i;
>>   int *j = &i;
>>   func([=j]() { ... });
> 
> You're right that it's the same "bad pointer to stack" issue, but the problem is at a different level. In the example above, the closures themselves are perfectly fine. The problem is that std::function captures the closure incorrectly. That's why I think it's a library issue.
> 
> Jared

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20141016/73e5956c/attachment.html>