[Webkit-unassigned] [Bug 188598] [JSC] Remove gcc warnings on mips and armv7

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 17 04:33:45 PDT 2018


Adrian Perez <aperez at igalia.com> changed:

           What    |Removed                     |Added
                 CC|                            |aperez at igalia.com

--- Comment #7 from Adrian Perez <aperez at igalia.com> ---
Comment on attachment 347180
  --> https://bugs.webkit.org/attachment.cgi?id=347180

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

This is a nice patch, and I like it that you have added comments documenting
why it's safe to disable the compiler warning for the tricky cases. (Informal r+!)

> Source/JavaScriptCore/ChangeLog:28
> +        Exception * cast to a JSObject *. It is therefore safe to cast it back

Probabl your editor has messed a bit here with the star signs “*” used
as bullets and it may have reflowed the text without removing them, and
now there's too many inline stars in this paragraph.

Please remove the excess stars :-)

>>> Source/WTF/wtf/Compiler.h:400
>>> +#endif
>> Wow, very nice. You should use this to reduce all the manual use of #pragma GCC diagnostic and #pragma clang diagnostic throughout the project.
>> Unfortunately, I think you are going to need two separate definitions here for Clang and GCC, IGNORE_WARNING_GCC and IGNORE_WARNING_CLANG, or you'll just wind up introducing new warnings about unknown pragmas when the wrong compiler is used. At least I'm pretty sure GCC complains if it doesn't recognize the warning, and it definitely complains if it sees #pragma clang.
> I agree. I also think that should be a separate patch, WDYT?

I would be fine with this being solved in a follow-up patch, just don't
forget about it ;-)

You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180817/f6e9208a/attachment.html>

More information about the webkit-unassigned mailing list