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

[David Blaikie]

On Thu, Oct 16, 2014 at 9:56 AM, Jared Grubb <jared.grubb at gmail.com> wrote:

>
> 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>
> wrote:
>
>>
>> On Oct 3, 2014, at 14:50, David Blaikie <dblaikie at gmail.com> wrote:
>> On Fri, Oct 3, 2014 at 12:55 PM, Jared Grubb <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.
>

*nod* I'd just be hesitant to add such a special case to the standard
library for a variety of reasons - one would be that this could create
subtle portability concerns if you ever port the code to a standard library
without this feature? (granted the inverse would be true if you ever took
libc++ and ran it on a compiler without the hypothetical warnings I'm
suggesting)

Anyway - I'm not the decider in this instance & don't have much involvement
in libc++ at all - I just wanted to raise this possible alternative
path/concerns. I'll leave it up to Marshall to give it due consideration.

It's a great thing to bring up/figure out in any case, I'm sure.

- David


>
>
>> 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/20141017/95cefa45/attachment.html>