Hi all,
Through my work to clean up compilation warnings on some platforms[1], I
realized that there are many places in the code where we explicitly
ignore compiler warnings. This means that we have a lot of repetition of
boilerplate code that typically looks something like:
#if COMPILER(CLANG) && defined(__has_warning)
#pragma clang diagnostic push
#if __has_warning("-Wfoo-bar")
#pragma clang diagnostic ignored "-Wfoo-bar"
#endif
#endif
// Code that triggers the warning.
#if COMPILER(CLANG) && defined(__has_warning)
#pragma clang diagnostic pop
#endif
There are also some inconsistencies on how we do this (e.g. sometimes we
don't check for the compiler, or use __has_warning). So, I would like to
replace that boilerplate with a set of macros that do all the necessary
checks, so that the above would become:
IGNORE_WARNING_CLANG("-Wfoo-bar");
// Code that triggers the warning.
IGNORE_WARNING_CLANG_END("-Wfoo-bar");
Since we want to ignore warnings for gcc too, and sometimes for both gcc
and clang, I also made IGNORE_WARNING_GCC[_END]() and
IGNORE_WARNING_GCC_OR_CLANG[_END]().
On top of that, since the places we ignore warnings happen both inside
and outside of function definitions, I made _TOP_LEVEL variants of all
the above macros, the difference being that the non-_TOP_LEVEL versions
are wrapped inside a "do { ... } while(false)", so that they behave like
an instruction. Though I am starting to agree with Michael[2] that this
might be overcomplicating things, and we might be better off with only
one variant. In that case, I would prefer to add "PRAGMA" in the name of
the macro to make things a little more explicit as to why we wouldn't
have semicolons, so that the above example would become:
PRAGMA_IGNORE_WARNING_CLANG("-Wfoo-bar")
// Code that triggers the warning.
PRAGMA_IGNORE_WARNING_CLANG_END("-Wfoo-bar")
Note the absence of semicolons. Also, in such a case, I'm not sure
whether we would want to indent the macro like the surrounding code, or
at the beginning of the line like a #pragma (I would naturally opt for
the former option).
So, this is my proposal, I'd like to hear opinions, and ideally reach
consensus, on the best way to do this, or if it is a good idea at all.
I filed a bug[3] and already made a patch, though as discussed above,
I'm happy to change things a bit depending on the consensus.
Cheers,
Guij
[1] https://webkit.org/b/188598
[2] https://bugs.webkit.org/show_bug.cgi?id=188996#c3
[3] https://bugs.webkit.org/show_bug.cgi?id=188996
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
