[webkit-reviews] review granted: [Bug 209637] [JSC] Make Operator an enum class to avoid Op* identifiers : [Attachment 394685] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Ross Kirsling
<ross.kirsling at sony.com>'s request for review:
Bug 209637: [JSC] Make Operator an enum class to avoid Op* identifiers
https://bugs.webkit.org/show_bug.cgi?id=209637

Attachment 394685: Patch

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




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 394685
  --> https://bugs.webkit.org/attachment.cgi?id=394685
Patch

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

I like this, but since it’s a coding style thing, and sometimes the people
working on JavaScript don’t agree with me on coding style (and I try not to be
heavy handed about my opinions in such cases) I am not sure if my review is
sufficient.

>> Source/JavaScriptCore/parser/Nodes.h:1416
>> +	    Operator m_operator;
> 
> A note of interest:
> Looks like this was overlooked in r233937 due to the obfuscation introduced
in r170381.
> (Not sure if these booleans still need the `: 1` either but I'll leave 'em
unless someone says otherwise.)

So this should reduce the bits from 30 to 8. But we could limit the number of
bits even further if we needed to? I’d want Yusuke to weigh on whether a slight
reduction in this node’s size would have a desirable effect and if so, then do
what it takes to use the smaller number of bits.