[Webkit-unassigned] [Bug 212714] Release Assert @ WebCore::RenderTreeBuilder::RenderTreeBuilder

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jun 10 16:48:02 PDT 2020


https://bugs.webkit.org/show_bug.cgi?id=212714

--- Comment #26 from Chris Dumez <cdumez at apple.com> ---
(In reply to Geoffrey Garen from comment #25)
> > > Source/WebCore/rendering/RenderWidget.cpp:60
> > > +        auto map = WTFMove(widgetNewParentMap());
> > 
> > In cases like this where we *rely* in the behavior that the source ends up
> > empty afterward, instead of just allowing move as an optimization, we should
> > write using std::exchange instead.
> > 
> >     auto map = std::exchange(widgetNewParentMap(), { });
> > 
> > The WTFMove form allows move as an optimization, but also allows the source
> > map to have a remnant value other than empty. The std::exchange form
> > guarantees that the source map will be empty. Both should generate nearly
> > identical code if all the optimization is working properly.
> 
> +Chris because I think he has an opinion on move producing an empty value.

I indeed prefer if our move constructors leave the object empty and clean to reduce the change of bugs. We've had a lot of bugs like that in the past. This makes adopting of std::optional a bit controversial because its move constructor leaves the optional in a bad state.

That said, I am all for using std::exchange() instead of WTFMove() when we know we are going to re-use the value after. It is the proper C++ way and I think we should enforce it in new code. It would make a switch to std::optional some day easier too although I have concerns about that part :)

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200610/b1c5993f/attachment-0001.htm>


More information about the webkit-unassigned mailing list