[webkit-reviews] review granted: [Bug 189633] Add size() function to OptionSet : [Attachment 349978] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Woodrow Wang
<wwang153 at stanford.edu>'s request for review:
Bug 189633: Add size() function to OptionSet
https://bugs.webkit.org/show_bug.cgi?id=189633

Attachment 349978: Patch

https://bugs.webkit.org/attachment.cgi?id=349978&action=review




--- Comment #10 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 349978
  --> https://bugs.webkit.org/attachment.cgi?id=349978
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=349978&action=review

> Source/WTF/wtf/OptionSet.h:124
> +    constexpr unsigned size()

This function should be marked const as constexpr will no longer imply const in
a future C++ version.

> Source/WTF/wtf/OptionSet.h:127
> +	   static_assert(sizeof(StorageType) <= sizeof(unsigned long long),
"__builtin_popcount is not supported for the underlying OptionSet storage
type.");

__builtin_popcount => __builtin_popcountll

> Source/WTF/wtf/OptionSet.h:129
> +#else

We should also add a fast path for MSVC. According to
<https://docs.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-
2010/bb385231(v=vs.100)>, the MSVC equivalent of __builtin_popcountll is
__popcnt64 (defined in header <intrin.h>). Notice that it takes an unsigned
_int64. We should take care to cast m_storage to this type and then add a
static assert similar to the one for GCC/Clang.