This one and the many others like it are fragile, relying on undefined behavior, and should be replaced by std::exchange. Such a change was made in https://trac.webkit.org/changeset/198755/webkit and we probably need many more like that, but we are getting away with relying on undefined behavior which works for us in most places.
> On Dec 17, 2018, at 11:24 AM, Chris Dumez <cdu...@apple.com> wrote: > > > >> On Dec 17, 2018, at 11:10 AM, Chris Dumez <cdu...@apple.com >> <mailto:cdu...@apple.com>> wrote: >> >> >> >>> On Dec 17, 2018, at 10:27 AM, Alex Christensen <achristen...@apple.com >>> <mailto:achristen...@apple.com>> wrote: >>> >>>>>>> On Dec 14, 2018, at 1:37 PM, Chris Dumez <cdu...@apple.com >>>>>>> <mailto:cdu...@apple.com>> wrote: >>>>>> >>>>>>> >>>>>>> As far as I know, our convention in WebKit so far for our types has >>>>>>> been that types getting moved-out are left in a valid “empty” state. >>> This is not necessarily true. When we move out of an object to pass into a >>> function parameter, for example, the state of the moved-from object depends >>> on the behavior of the callee. If the callee function uses the object, we >>> often have behavior that leaves the object in an “empty” state of some >>> kind, but we are definitely relying on fragile undefined behavior when we >>> do so because changing the callee to not use the parameter changes the >>> state of the caller. We should never assume that WTFMove or std::move >>> leaves the object in an empty state. That is always a bug that needs to be >>> replaced by std::exchange. >> >> Feel like we’re taking about different things. I am talking about move >> constructors (and assignment operators), which have a well defined behavior >> in WebKit. And it seems you are talking about WTFMove(), which despite the >> name does not “move” anything, it is merely a cast. >> In the case you’re talking about the caller does NOT call the move >> constructor, it merely does a cast so I do not think your comment >> invalidates my statement. Note that in my patch, I was nearly WTFMove()ing >> the data member and assigning it to a local variable right away, calling the >> move constructor. > > Also note that may of us already rely on our move constructors’ behavior, > just search for WTFMove(m_responseCompletionHandler) in: > https://trac.webkit.org/changeset/236463/webkit > <https://trac.webkit.org/changeset/236463/webkit>
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
