[webkit-dev] Should we ever use std::function instead of WTF::Function?

[Filip Pizlo]

> On Jun 13, 2017, at 1:00 PM, Chris Dumez <cdumez at apple.com> wrote:
> 
> 
>> On Jun 13, 2017, at 12:51 PM, Filip Pizlo <fpizlo at apple.com <mailto:fpizlo at apple.com>> wrote:
>> 
>>> 
>>> On Jun 13, 2017, at 12:37 PM, Alex Christensen <achristensen at apple.com <mailto:achristensen at apple.com>> wrote:
>>> 
>>> Ok, maybe we can get rid of std::function, then!  I hadn’t used BlockPtr as much as Chris.  I’d be opposed to adding a copy constructor to WTF::Function because the non-copyability of WTF::Function is why we made it, and it has prevented many bugs.
>> 
>> I agree that the copy semantics of std::function are strange - each copy gets its own view of the state of the closure.  This gets super weird when you implement an algorithm that accidentally copies the function - the act of copying invokes copy constructors on all of the lambda-lifted state, which is probably not what you wanted.  So I’m not surprised that moving to WTF::Function avoided bugs.  What I’m proposing also prevents many of the same bugs because the lambda-lifted state never gets copied in my world.
> 
> I understand SharedTask avoids many of the same issues. My issue is that it adds an extra data member for refcounting that is very rarely needed in practice. Also, because this type is copyable, people can create refcounting churn by inadvertently copying.
> Because most of the time, we do not want to share and WTF::Function is sufficient as it stands, I do not think it is worth making WTF::Function refcounted.

Perf is a good argument against this, but then it seems like WTF::Function should by default allow block-style sharing semantics, and then there would be a WTF::UniqueFunction that is more optimal.

Also, who is “we” here?  If “we” includes JSC, then “we” don’t find WTF::Function sufficient.  I try to use WTF::Function whenever I can because I like the name and I like the API, but often I have to switch to SharedTask in order to make the code work.  B3’s use of SharedTask for its StackmapGenerator is a great example.  We can’t use WTF::Function in that code because the data structures that reference the generator get copied and that’s OK when we use SharedTask.

> 
>> 
>> Do you think that code that uses ObjC blocks encounters the kind of bugs that you saw WTF::Function preventing?  Or are the bugs that Function prevents more specific to std::function?  I guess I’d never heard of a need to change block semantics to avoid bugs, so I have a hunch that the bugs you guys prevented were specific to the fact that std::function copies instead of sharing.
> 
> To be cleared, we viewed this as “copies instead of truly moving”, not sharing. We never really needed sharing when WTF::Function was initially called WTF::NonCopyableFunction.
> Yes, the bugs we were trying to avoid were related to using std::function and copying things implicitly, even if you WTFMove() it around. Because we started using WTF::Function instead of std::function in more places though, having BlockPtr::fromCallable() to be able to pass a WTF::Function to an ObjC function expecting a block became handy though. 
> 
>> 
>>> 
>>> I’ve also seen many cases where I have a WTF::Function that I want to make sure is called once and only once before destruction.  I wouldn’t mind adding a WTF::Callback subclass that just asserts that it has been called once.  That would’ve prevented some bugs, too, but not every use of WTF::Function has such a requirement.
>>> 
>>>> On Jun 13, 2017, at 12:31 PM, Chris Dumez <cdumez at apple.com <mailto:cdumez at apple.com>> wrote:
>>>> 
>>>> We already have BlockPtr for passing a Function as a lambda block.
>>>> 
>>>> Chris Dumez
>>>> 
>>>> On Jun 13, 2017, at 12:29 PM, Alex Christensen <achristensen at apple.com <mailto:achristensen at apple.com>> wrote:
>>>> 
>>>>> std::function, c++ lambda, and objc blocks are all interchangeable.  WTF::Functions cannot be used as objc blocks because the latter must be copyable.  Until that changes or we stop using objc, we cannot completely eliminate std::function from WebKit.
>>>>> _______________________________________________
>>>>> webkit-dev mailing list
>>>>> webkit-dev at lists.webkit.org <mailto:webkit-dev at lists.webkit.org>
>>>>> https://lists.webkit.org/mailman/listinfo/webkit-dev <https://lists.webkit.org/mailman/listinfo/webkit-dev>
>>> 
>>> _______________________________________________
>>> webkit-dev mailing list
>>> webkit-dev at lists.webkit.org <mailto:webkit-dev at lists.webkit.org>
>>> https://lists.webkit.org/mailman/listinfo/webkit-dev <https://lists.webkit.org/mailman/listinfo/webkit-dev>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20170613/a66d36c8/attachment.html>