[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