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

Fujii Hironori fujii.hironori at gmail.com
Sun Dec 16 23:41:55 PST 2018

I've changed my mind.

It is ensured std::vector move constructor makes moved value empty.

> After the move, other is guaranteed to be empty().


Unfortunately, WTF::Vector move constructor just swaps values.
I think this swapping is not a specified behavior WebKit developers can
rely on.


I think WTF::Vector also should ensure moved values become empty.

Looking through the grepping "WTFMove(m_”result, some of them are valid
1. It is in dtor. 'this' object is about to be destructed.
2. It is in ref-qualified method. 'this' object is about to be destructed.
3. It is Ref<>. As well as std::shared_ptr, it should be defined to become
null after moved.

Common misuse cases are moving Vector like following code.

void SVGDocumentExtensions::rebuildElements()
> {
>     Vector<SVGElement*> shadowRebuildElements = WTFMove(m_rebuildElements);

I thought std::exchange should be used for these purpose.
But, if WTF::Vector move constructor is changed, this code would be valid.
m_rebuildElements would become empty as expected, not accidentally.

If above code would be valid, there would be no reason to object we have
custom Optional class which guarantees moved values become empty.

clang-tidy has use-after-move checker.

clang-tidy - bugprone-use-after-move — Extra Clang Tools 8 documentation

As expected, it doesn't support member variables.

The check currently only considers calls of std::move on local variables or
> function parameters. It does not check moves of member variables or global
> variables.

It seems UBSan doesn't check use-after-move.

UndefinedBehaviorSanitizer — Clang 8 documentation
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20181217/c9b42ead/attachment.html>

More information about the webkit-dev mailing list