[webkit-dev] Should we ever use std::function instead of WTF::Function?
Chris Dumez
cdumez at apple.com
Tue Jun 13 13:00:30 PDT 2017
> On Jun 13, 2017, at 12:51 PM, Filip Pizlo <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.
>
> 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/87322c81/attachment.html>
More information about the webkit-dev
mailing list