[webkit-reviews] review denied: [Bug 195573] Alter Tools/Scripts/dump-class-layout to be able to dump all classes with suspicious padding : [Attachment 364329] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Mar 11 19:27:30 PDT 2019


Simon Fraser (smfr) <simon.fraser at apple.com> has denied Robin Morisset
<rmorisset at apple.com>'s request for review:
Bug 195573: Alter Tools/Scripts/dump-class-layout to be able to dump all
classes with suspicious padding
https://bugs.webkit.org/show_bug.cgi?id=195573

Attachment 364329: Patch

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




--- Comment #6 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 364329
  --> https://bugs.webkit.org/attachment.cgi?id=364329
Patch

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

r- for another round.

>> Tools/Scripts/dump-class-layout:55
>> +	    help='name of the class or struct to dump. ALL_WASTEFUL is a
special name that makes it dump all classes with at least 8 bytes of padding at
the top level.')
> 
> OK as-is, but in my eyes this just reads lazy and not at all intuitive. But
hey, you explained in the usage so I guess that’s fair :/. I personally use
this tool quite a bit. So I find this kind of Jack very bitter to swallow, but
maybe I won’t ever use this feature. Exposing a new command line flag is the
correct approach to this situation and the most Unix-like.

I think it would be better as a new argument --all-wasteful that just overrides
classname.


More information about the webkit-reviews mailing list