Re: [lmi] [lmi-commits] master 1372a42 6/7: Use '-isystem' to suppress warnings in third-party headers

[Greg Chicares]

On 2020-06-08 11:51, Vadim Zeitlin wrote:
> On Sun,  7 Jun 2020 18:03:29 -0400 (EDT) Greg Chicares 
> <gchicares@sbcglobal.net> wrote:
[...]
> GC> commit 1372a42340285f6f3604db30a371142b54261991
[...]
> GC>     Use '-isystem' to suppress warnings in third-party headers
> 
>  Funnily enough, a commit[1] with the message
> 
>       Replace `-isystem` compile flag with `-I`
> 
>       Using `-isystem` flag causes compilation errors with GCC10.
>       Replace it with `-I` flag.
> 
> was merged into another project I follow/participate in just yesterday. And
> the discussion of this commit links to gcc bug[2] where one of gcc
> maintainers wrote
> 
>> you shouldn't use -isystem unless you really need to change the search
>> order for system directories!
> 
> and recommended using GCC diagnostic pragmas for disabling the warnings
> instead.

Fascinating. The documentation says something different:

  https://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html
  | If you really need to change the search order for system directories,
  | use the -nostdinc and/or -isystem options.

He presents his conclusion as a
  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70129#c8
"corollary" that follows from the "clearly documented" behavior. But
that's a spurious contraposition, akin to:
  If you are starving, then you should eat.
  Therefore, you shouldn't eat unless you are starving.
That doesn't necessarily mean that his advice should not be taken,
but OTOH I'm not alone in thinking this is a valid use case:

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70129#c3
| I think you'll find most build systems that do "-isystem /usr/include"
| instead of "-I /usr/include" are only using "-isystem" for the change
| in the warning behavior.  The change in the include path order is not
| wanted...

and I think recent lmi commits have shown that '-isystem' has
important advantages that alternatives lack.

> GC>     Some third-party headers elicit warnings, which '-Werror' turns into
> GC>     errors. There are several ways to avoid that:
> GC>     
> GC>     (1) Use pragmata to suppress the warnings before including such 
> headers,
> GC>        and "pop" their suppression right after including them. That 
> doesn't
> GC>        suppress warnings stemming from macros in those headers. In theory,
> GC>        each such macro expansion could be surrounded by pragmata, but 
> that's
> GC>        tedious and ugly.
> 
>  I haven't tried it, but what about defining our own macros using pragmas
> around the original macro invocation, shouldn't this work?

IOW, instead of

  #if defined __GNUC__
  #   pragma GCC diagnostic push
  #   pragma GCC diagnostic ignored "-Wcast-function-type"
  #endif // defined __GNUC__
  some_actual_code;
  #if defined __GNUC__
  #   pragma GCC diagnostic pop
  #endif // defined __GNUC__

we could perhaps write

  WARNING_PUSH(list,of,warnings)
  some_actual_code;
  WARNING_POP

Is that what you have in mind? I'd say that's still ugly and intrusive.

Here's another option, which I hadn't mentioned in that commit message:

  https://gcc.gnu.org/onlinedocs/cpp/System-Headers.html
| There is also a directive, #pragma GCC system_header, which tells GCC
| to consider the rest of the current include file a system header, no
| matter where it was found. Code that comes before the ‘#pragma’ in
| the file is not affected. #pragma GCC system_header has no effect in
| the primary source file.

That pragma could be added (before the first line of code) to every
third-party header file, by such means as:
 - convincing upstream maintainers to add it; or
 - forcibly inserting it, e.g. with 'sed'.

Anyway, my plan is to continue pursuing the '-isystem' method, and
then go back later and consider whether there's a better way.