[Webkit-unassigned] [Bug 152601] New: Use of WTF::move prevents clang's move diagnostics from warning about several classes of mistakes

[bugzilla-daemon at webkit.org]

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

            Bug ID: 152601
           Summary: Use of WTF::move prevents clang's move diagnostics
                    from warning about several classes of mistakes
    Classification: Unclassified
           Product: WebKit
           Version: WebKit Nightly Build
          Hardware: All
                OS: All
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: Web Template Framework
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: aestes at apple.com

Clang has recently added warnings to catch certain classes of mistakes with the use of std::move(): -Wpessimizing-move (warns if moving prevents copy elision), -Wredundant-move (warns if a move is redundant), and -Wself-move (warns if moving to self). Enabling these warnings manually (by renaming WTF::move to std::move) have caught numerous mistakes in our codebase (see http://trac.webkit.org/changeset/194428).

It would be nice to be able to take advantage of these warnings, but doing so requires that we use std::move, not WTF::move. But since WTF::move does provide useful checks for which clang does not yet have warnings, we should find a way to write a best-of-both-worlds move function. I think we could do this like so:

    namespace WTF {
    enum CheckMoveParameterTag { CheckMoveParameter };
    }

    namespace std {
    template<WTF::CheckMoveParameterTag, typename T>
    ALWAYS_INLINE CONSTEXPR typename remove_reference<T>::type&& move(T&& value)
    {
        // static_asserts from WTF::move
        return std::move(std::forward<T>(value));
    }
    }

    #define WTF_MOVE(value) std::move<WTF::CheckMoveParameter>(value)

This move function satisfies clang's criteria for a move function (in namespace std, called "move", takes a single argument), but allows us to add our own compile-time asserts to check for moving from const and moving from rvalue reference. A macro called WTF_MOVE() is added for convenience.

I plan to address this in two stages:

1. Upload a patch that defines std::move<WTF::CheckMoveParameter>() and WTF_MOVE().
2. Upload a patch that removes WTF::move and renames all uses of WTF::move to WTF_MOVE.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20151230/62a87afa/attachment.html>