[Webkit-unassigned] [Bug 188996] Add IGNORE_WARNING_.* macros

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=188996

--- Comment #23 from Darin Adler <darin at apple.com> ---
(In reply to Guillaume Emont from comment #21)
> As I explained in https://bugs.webkit.org/show_bug.cgi?id=188996#c18 the
> rationale to keep the argument is:
>  - consistency and clarity (we don't need to add a "// -Wfoo-bar" comment at
> the end when a large portion of code is within the ignore)
>  - removing the argument to the _END macros would mean we have to always
> emit the diagnostic push/pop even if we don't add an ignore
> 
> If you believe that considering these two aspects it's still better to
> remove the argument from _END macros, I'm happy to do it.

I do. My view is that an ignored argument that can be wrong is not *better* than a comment. In fact it’s *worse* because it’s a comment in disguise and a comment that you always must include. You could put "<<<just making the macro happen>>>" in and it would work compile just fine.

I don’t think that END macros should have comments in any normal circumstances; they are going to be just a line or two down from the start macros. Just as we wouldn’t want to comment every close brace with an explanation of the open brace from a line or two above. Maybe occasionally a comment would be needed for clarity but it seems unlikely.

I’m not at all concerned about compiling more push/pop pairs but maybe you could show me an example of a specific case where that is bad and help me understand why you are concerned about it.

>  - As Michael was pointing out, gcc warns if you try to ignore a warning it
> does not know about, so at least some of the clang only warnings need to
> remain that way.

That’s fine, but I believe the default should be the "both" version and the unusual, longer-named, more-special version should be the single-compiler only versions.

The goal of the compiler file is to create mostly compiler-independent abstractions and one thing I don’t like about this is that we also support the Windows compiler and these macros don’t help us ignore the corresponding warnings on Windows in an elegant way. Not a problem in the short term, but a problem with the design. Unless none of these warning concepts make sense on Windows. But that seems unlikely.

There are some strange intersections here. The "ignore deprecation" warning is Apple-platform-specific so that explains why it will never come up on Windows, and that's more than half of the use of this.

>  - We could change the gcc-only warnings to gcc or clang¸ since clang
> provides __has_warning(). Would you like me to do that?

Generally speaking, yes I would, if there are a lot of these. But to be sure I’d need some examples of specifics.

Our default should be cross-compiler things, and our compiler-specific things should be treated as "grudging complexities". The issue with GCC complaining about unknown warnings is not just an issue for clang-specific things, by the way, it’s an issue for multiple versions of GCC. If a new version adds a new warning, we need to make the warning disabling apply only under the newer compiler.

I’m not 100% sure that we should do -Wno-unknown-pragmas, by the way, since that warning aspires to be a useful way to detect typos in pragmas and I would love to do that even if it creates a bit of inconvenience for us. But there may be no practical way to leave it on and support multiple compilers and compiler versions elegantly. I don’t know why clang doesn’t offer a similar warning.

-- 
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/20180829/0fc5acbd/attachment.html>