[webkit-reviews] review granted: [Bug 188713] Pack booleans in Event into a bitfield : [Attachment 347419] Cleanup

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 188713: Pack booleans in Event into a bitfield
https://bugs.webkit.org/show_bug.cgi?id=188713

Attachment 347419: Cleanup

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




--- Comment #3 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 347419
  --> https://bugs.webkit.org/attachment.cgi?id=347419
Cleanup

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

> Source/WebCore/dom/Event.cpp:37
> +ALWAYS_INLINE Event::Event(MonotonicTime createTime, const AtomicString&
type, IsTrusted isTrusted, CanBubble canBubble, IsCancelable cancelable,
IsComposed composed)

This feels like a step backwards from using default initializers. I am assuming
it is a memory footprint win. Once we move to C++20 we can use default
initalizers for bitfields.

> Source/WebCore/dom/Event.h:171
> +    unsigned m_eventPhase : 2; // 13-bits

I wish we had a good solution for this. Very easy for a person to add a new
every phase and forget to update the width (if needed). This problem is not
unique to this code. One tactic I have seen is to define a constexpr for the
width above the enum in the hope that a person sees it. Another tactic is to
add placeholder enum after last enumerator then compute width.