Yeah, it seems like making std::optional more in line with our own convention provides more merits than downsides here. People are using WTFMove as if it's some sort of a swap operation in our codebase, and as Maciej pointed out, having rules where people have to think carefully as to when & when not to use WTFMove seems more troublesome than the proposed fix, which would mean this work for optional.
- R. Niwa On Mon, Dec 17, 2018 at 2:24 PM Geoffrey Garen <gga...@apple.com> wrote: > I don’t understand the claim about “undefined behavior” here. As Maciej > pointed out, these are our libraries. We are free to define their behaviors. > > In general, “undefined behavior” is an unwanted feature of programming > languages and libraries, which we accept begrudgingly simply because there > are practical limits to what we can define. This acceptance is not a > mandate to carry forward undefined-ness as a badge of honor. In any case > where it would be practical to define a behavior, that defined behavior > would be preferable to undefined behavior. > > I agree that the behavior of move constructors in the standard library is > undefined. The proposal here, as I understand it, is to (a) define the > behaviors move constructors in WebKit and (b) avoid std::optional and use > an optional class with well-defined behavior instead. > > Because I do not ❤️ security updates, I do ❤️ defined behavior, and so I > ❤️ this proposal. > > Geoff > > On Dec 17, 2018, at 12:50 PM, Alex Christensen <achristen...@apple.com> > wrote: > > 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> wrote: > > > > On Dec 17, 2018, at 10:27 AM, Alex Christensen <achristen...@apple.com> > wrote: > > On Dec 14, 2018, at 1:37 PM, Chris Dumez <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 > > > _______________________________________________ > webkit-dev mailing list > webkit-dev@lists.webkit.org > https://lists.webkit.org/mailman/listinfo/webkit-dev > > > _______________________________________________ > webkit-dev mailing list > webkit-dev@lists.webkit.org > https://lists.webkit.org/mailman/listinfo/webkit-dev >
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
