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

[Fujii Hironori]

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().

https://en.cppreference.com/w/cpp/container/vector/vector

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

https://trac.webkit.org/browser/webkit/trunk/Source/WTF/wtf/Vector.h?rev=238031#L950

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

Looking through the grepping "WTFMove(m_”result, some of them are valid
because:
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.


BWT, FWIW,
clang-tidy has use-after-move checker.

clang-tidy - bugprone-use-after-move — Extra Clang Tools 8 documentation
http://clang.llvm.org/extra/clang-tidy/checks/bugprone-use-after-move.html


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
https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#ubsan-checks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20181217/c9b42ead/attachment.html>