<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">Right now, after a maybeMove type call on std::optional, you get either the original value, or a value that is officially valid but unspecified, and actually an optional that says it contains a value but doesn’t. With Chris’s proposal, you’d get a WTF::Optional with either the original value, or an optional that doesn’t contain a value and says it doesn’t. Among other things, this allows for a “did anything actually get left here” check after the function that may or may not move a value. Seems like an upgrade.</div><div class=""><br class=""></div><div><br class=""><blockquote type="cite" class=""><div class="">On Dec 17, 2018, at 3:55 PM, Alex Christensen <<a href="mailto:achristensen@apple.com" class="">achristensen@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html; charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Let me give a concrete example on why, even with our nice-to-use WTF types, the state of a C++ object is undefined after being moved from:<div class=""><br class=""></div><div class=""><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(195, 148, 149);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #f661f7" class="">#include</span><span style="font-variant-ligatures: no-common-ligatures;" class=""> </span><span style="font-variant-ligatures: no-common-ligatures" class=""><wtf/RefCounted.h></span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(195, 148, 149);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #f661f7" class="">#include</span><span style="font-variant-ligatures: no-common-ligatures;" class=""> </span><span style="font-variant-ligatures: no-common-ligatures" class=""><wtf/RefPtr.h></span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(195, 148, 149);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #f661f7" class="">#include</span><span style="font-variant-ligatures: no-common-ligatures;" class=""> </span><span style="font-variant-ligatures: no-common-ligatures" class=""><iostream></span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""></span><br class=""></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(57, 164, 40);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #d422ff" class="">class</span><span style="font-variant-ligatures: no-common-ligatures;" class=""> </span><span style="font-variant-ligatures: no-common-ligatures" class="">Test</span><span style="font-variant-ligatures: no-common-ligatures;" class=""> : </span><span style="font-variant-ligatures: no-common-ligatures; color: #d422ff" class="">public</span><span style="font-variant-ligatures: no-common-ligatures;" class=""> </span><span style="font-variant-ligatures: no-common-ligatures" class="">RefCounted</span><span style="font-variant-ligatures: no-common-ligatures;" class=""><</span><span style="font-variant-ligatures: no-common-ligatures" class="">Test</span><span style="font-variant-ligatures: no-common-ligatures;" class="">> { };</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""></span><br class=""></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(57, 164, 40);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">void</span><span style="font-variant-ligatures: no-common-ligatures;" class=""> </span><span style="font-variant-ligatures: no-common-ligatures; color: #6122ff" class="">useParameter</span><span style="font-variant-ligatures: no-common-ligatures;" class="">(</span><span style="font-variant-ligatures: no-common-ligatures" class="">RefPtr</span><span style="font-variant-ligatures: no-common-ligatures;" class=""><</span><span style="font-variant-ligatures: no-common-ligatures" class="">Test</span><span style="font-variant-ligatures: no-common-ligatures;" class="">>&& </span><span style="font-variant-ligatures: no-common-ligatures; color: #c79725" class="">param</span><span style="font-variant-ligatures: no-common-ligatures;" class="">)</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">{</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">  </span><span style="font-variant-ligatures: no-common-ligatures; color: #39a428" class="">RefPtr</span><span style="font-variant-ligatures: no-common-ligatures" class=""><</span><span style="font-variant-ligatures: no-common-ligatures; color: #39a428" class="">Test</span><span style="font-variant-ligatures: no-common-ligatures" class="">> </span><span style="font-variant-ligatures: no-common-ligatures; color: #c79725" class="">usedParam</span><span style="font-variant-ligatures: no-common-ligatures" class=""> = WTFMove(param);</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">}</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""></span><br class=""></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(97, 34, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #39a428" class="">void</span><span style="font-variant-ligatures: no-common-ligatures;" class=""> </span><span style="font-variant-ligatures: no-common-ligatures" class="">dontUseParameter</span><span style="font-variant-ligatures: no-common-ligatures;" class="">(</span><span style="font-variant-ligatures: no-common-ligatures; color: #39a428" class="">RefPtr</span><span style="font-variant-ligatures: no-common-ligatures;" class=""><</span><span style="font-variant-ligatures: no-common-ligatures; color: #39a428" class="">Test</span><span style="font-variant-ligatures: no-common-ligatures;" class="">>&&) { }</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""></span><br class=""></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #39a428" class="">int</span><span style="font-variant-ligatures: no-common-ligatures" class=""> </span><span style="font-variant-ligatures: no-common-ligatures; color: #6122ff" class="">main</span><span style="font-variant-ligatures: no-common-ligatures" class="">() {</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">  </span><span style="font-variant-ligatures: no-common-ligatures; color: #39a428" class="">RefPtr</span><span style="font-variant-ligatures: no-common-ligatures" class=""><</span><span style="font-variant-ligatures: no-common-ligatures; color: #39a428" class="">Test</span><span style="font-variant-ligatures: no-common-ligatures" class="">> </span><span style="font-variant-ligatures: no-common-ligatures; color: #c79725" class="">a</span><span style="font-variant-ligatures: no-common-ligatures" class=""> = adoptRef(</span><span style="font-variant-ligatures: no-common-ligatures; color: #d422ff" class="">new</span><span style="font-variant-ligatures: no-common-ligatures" class=""> </span><span style="font-variant-ligatures: no-common-ligatures; color: #39a428" class="">Test</span><span style="font-variant-ligatures: no-common-ligatures" class="">);</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">  </span><span style="font-variant-ligatures: no-common-ligatures; color: #39a428" class="">RefPtr</span><span style="font-variant-ligatures: no-common-ligatures" class=""><</span><span style="font-variant-ligatures: no-common-ligatures; color: #39a428" class="">Test</span><span style="font-variant-ligatures: no-common-ligatures" class="">> </span><span style="font-variant-ligatures: no-common-ligatures; color: #c79725" class="">b</span><span style="font-variant-ligatures: no-common-ligatures" class=""> = adoptRef(</span><span style="font-variant-ligatures: no-common-ligatures; color: #d422ff" class="">new</span><span style="font-variant-ligatures: no-common-ligatures" class=""> </span><span style="font-variant-ligatures: no-common-ligatures; color: #39a428" class="">Test</span><span style="font-variant-ligatures: no-common-ligatures" class="">);</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">  </span><span style="font-variant-ligatures: no-common-ligatures; color: #69c0be" class="">std</span><span style="font-variant-ligatures: no-common-ligatures" class="">::cout << </span><span style="font-variant-ligatures: no-common-ligatures; color: #c39495" class="">"a null? "</span><span style="font-variant-ligatures: no-common-ligatures" class=""> << !a << </span><span style="font-variant-ligatures: no-common-ligatures; color: #69c0be" class="">std</span><span style="font-variant-ligatures: no-common-ligatures" class="">::endl;</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">  </span><span style="font-variant-ligatures: no-common-ligatures; color: #69c0be" class="">std</span><span style="font-variant-ligatures: no-common-ligatures" class="">::cout << </span><span style="font-variant-ligatures: no-common-ligatures; color: #c39495" class="">"b null? "</span><span style="font-variant-ligatures: no-common-ligatures" class=""> << !b << </span><span style="font-variant-ligatures: no-common-ligatures; color: #69c0be" class="">std</span><span style="font-variant-ligatures: no-common-ligatures" class="">::endl;</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">  useParameter(WTFMove(a));</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">  dontUseParameter(WTFMove(b));</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">  </span><span style="font-variant-ligatures: no-common-ligatures; color: #69c0be" class="">std</span><span style="font-variant-ligatures: no-common-ligatures" class="">::cout << </span><span style="font-variant-ligatures: no-common-ligatures; color: #c39495" class="">"a null? "</span><span style="font-variant-ligatures: no-common-ligatures" class=""> << !a << </span><span style="font-variant-ligatures: no-common-ligatures; color: #69c0be" class="">std</span><span style="font-variant-ligatures: no-common-ligatures" class="">::endl;</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">  </span><span style="font-variant-ligatures: no-common-ligatures; color: #69c0be" class="">std</span><span style="font-variant-ligatures: no-common-ligatures" class="">::cout << </span><span style="font-variant-ligatures: no-common-ligatures; color: #c39495" class="">"b null? "</span><span style="font-variant-ligatures: no-common-ligatures" class=""> << !b << </span><span style="font-variant-ligatures: no-common-ligatures; color: #69c0be" class="">std</span><span style="font-variant-ligatures: no-common-ligatures" class="">::endl;</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(212, 34, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures;" class="">  </span><span style="font-variant-ligatures: no-common-ligatures" class="">return</span><span style="font-variant-ligatures: no-common-ligatures;" class=""> 0;</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">}</span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""></span><br class=""></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(224, 51, 34);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">// clang++ test.cpp -I Source/WTF -L WebKitBuild/Debug -l WTF -framework Foundation -L /usr/lib -l icucore --std=c++17 && ./a.out                           </span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(224, 51, 34);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">// a null? 0                                                                                                                                                </span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(224, 51, 34);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">// b null? 0                                                                                                                                                </span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(224, 51, 34);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">// a null? 1                                                                                                                                                </span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(224, 51, 34);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">// b null? 0                                                                                                                                               </span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""><br class=""></span></div><div style="margin: 0px; font-stretch: normal; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div class="">As you can see, the internals of callee dontUseParameter (which could be in a different translation unit) affects the state of the local variable b in this function.  This is one of the reasons why the state of a moved-from variable is intentionally undefined, and we can’t fix that by using our own std::optional replacement.  If we care about the state of a moved-from object, that is what std::exchange is for.  I think we should do something to track and prevent the use of moved-from values instead of introducing our own std::optional replacement.</div><div class=""><br class=""><blockquote type="cite" class=""><div class="">On Dec 17, 2018, at 2:47 PM, Ryosuke Niwa <<a href="mailto:rniwa@webkit.org" class="">rniwa@webkit.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div class="">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.</div><div class=""><br class=""></div><div class="">- R. Niwa</div><div dir="ltr" class=""><br class=""></div><div class="gmail_quote"><div dir="ltr" class="">On Mon, Dec 17, 2018 at 2:24 PM Geoffrey Garen <<a href="mailto:ggaren@apple.com" class="">ggaren@apple.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space" class="">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.<div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">Because I do not ❤️ security updates, I do ❤️ defined behavior, and so I ❤️ this proposal.</div><div class=""><br class=""></div><div class=""><div class="">Geoff<br class=""><div class=""><br class=""><blockquote type="cite" class=""><div class="">On Dec 17, 2018, at 12:50 PM, Alex Christensen <<a href="mailto:achristensen@apple.com" target="_blank" class="">achristensen@apple.com</a>> wrote:</div><br class="gmail-m_-5606773433173637452Apple-interchange-newline"><div class=""><div style="word-wrap:break-word;line-break:after-white-space" class="">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 <a href="https://trac.webkit.org/changeset/198755/webkit" target="_blank" class="">https://trac.webkit.org/changeset/198755/webkit</a> 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.<br class=""><div class=""><br class=""><blockquote type="cite" class=""><div class="">On Dec 17, 2018, at 11:24 AM, Chris Dumez <<a href="mailto:cdumez@apple.com" target="_blank" class="">cdumez@apple.com</a>> wrote:</div><br class="gmail-m_-5606773433173637452Apple-interchange-newline"><div class=""><div style="word-wrap:break-word;line-break:after-white-space" class=""><br class="">
<div class=""><br class=""><blockquote type="cite" class=""><div class="">On Dec 17, 2018, at 11:10 AM, Chris Dumez <<a href="mailto:cdumez@apple.com" target="_blank" class="">cdumez@apple.com</a>> wrote:</div><br class="gmail-m_-5606773433173637452Apple-interchange-newline"><div class=""><div style="word-wrap:break-word;line-break:after-white-space" class=""><br class="">
<div class=""><br class=""><blockquote type="cite" class=""><div class="">On Dec 17, 2018, at 10:27 AM, Alex Christensen <<a href="mailto:achristensen@apple.com" target="_blank" class="">achristensen@apple.com</a>> wrote:</div><br class="gmail-m_-5606773433173637452Apple-interchange-newline"><div class=""><div style="word-wrap:break-word;line-break:after-white-space" class=""><div style="word-wrap:break-word;line-break:after-white-space" class=""><div class=""><blockquote type="cite" class=""><div class=""><blockquote type="cite" class=""><div class=""><div class=""><blockquote type="cite" class=""><div class=""><div class=""><blockquote type="cite" class=""><div class="">On Dec 14, 2018, at 1:37 PM, Chris Dumez <<a href="mailto:cdumez@apple.com" target="_blank" class="">cdumez@apple.com</a>> wrote:</div></blockquote></div><div class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap:break-word;line-break:after-white-space" class=""><div class=""><br class=""></div><div class="">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.</div></div></div></blockquote></div></div></blockquote></div></div></blockquote></div></blockquote>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.</div></div></div></div></blockquote><div class=""><br class=""></div><div class="">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.</div><div class="">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.</div></div></div></div></blockquote><div class=""><br class=""></div><div class="">Also note that may of us already rely on our move constructors’ behavior, just search for WTFMove(m_responseCompletionHandler) in:</div><div class=""><a href="https://trac.webkit.org/changeset/236463/webkit" target="_blank" class="">https://trac.webkit.org/changeset/236463/webkit</a></div></div></div></div></blockquote></div><br class=""></div>_______________________________________________<br class="">webkit-dev mailing list<br class=""><a href="mailto:webkit-dev@lists.webkit.org" target="_blank" class="">webkit-dev@lists.webkit.org</a><br class=""><a href="https://lists.webkit.org/mailman/listinfo/webkit-dev" target="_blank" class="">https://lists.webkit.org/mailman/listinfo/webkit-dev</a><br class=""></div></blockquote></div><br class=""></div></div></div>_______________________________________________<br class="">
webkit-dev mailing list<br class="">
<a href="mailto:webkit-dev@lists.webkit.org" target="_blank" class="">webkit-dev@lists.webkit.org</a><br class="">
<a href="https://lists.webkit.org/mailman/listinfo/webkit-dev" rel="noreferrer" target="_blank" class="">https://lists.webkit.org/mailman/listinfo/webkit-dev</a><br class="">
</blockquote></div></div>
</div></blockquote></div><br class=""></div></div>_______________________________________________<br class="">webkit-dev mailing list<br class=""><a href="mailto:webkit-dev@lists.webkit.org" class="">webkit-dev@lists.webkit.org</a><br class="">https://lists.webkit.org/mailman/listinfo/webkit-dev<br class=""></div></blockquote></div><br class=""></body></html>