[webkit-dev] Watch out for std::optional's move constructor

[Ryosuke Niwa]

On Mon, Dec 17, 2018 at 6:40 PM Fujii Hironori <fujii.hironori at gmail.com>
wrote:

>
> On Tue, Dec 18, 2018 at 11:16 AM Maciej Stachowiak <mjs at apple.com> wrote:
>
>>
>>  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.
>>
>>
> Don't recommend such checks. It is simply use-after-move. The expression
> WTFMove(x) means marking x as disposable.
>

That's simply not true. The semantics of WTFMove / std::move is really that
of a type cast. I think std::move was ill-named. It really should have been
called std::movable_cast or something. Also, as Geoff has already pointed
out, the behavior of STL doesn't prevent us from writing our own template
library to always have a very well specified & good state after
std::move'ed & its value was move-constructed.

It's one thing to consider that new WebKit contributors who are familiar
with C++ semantics of std::move and std::optional getting confused by
WebKit's implementation of std::option. But then our behavior of HashMap
which doesn't accept the POD integral value of 0 as a key, and how Ref and
RefPtr work and they're used by our WebCore code is probably more
problematic than this std::optional behavior.

In general, I find these kinds of dogmatic adoption of the best practices /
recommended ways of coding to be highly problematic. We shouldn't care who
or what entity recommends writing code in one way or another. We should be
questioning every and each recommendation anyone makes, and judging for
ourselves whether it makes a sense for the WebKit project.

For example, there is a group of people who swear by always wrapping each
single statement with { ~ } as a way of preventing bugs which stems from
forgetting { ~ } around multiple statements. Yet in my nine years of
writing & reviewing code in WebKit, I've never once came across such a
mistake made by either me or any other contributor.

- R. Niwa
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20181217/a5914a45/attachment.html>