Hi, On 2022-12-28 19:05:35 -0500, Tom Lane wrote: > Andres Freund <and...@anarazel.de> writes: > > On 2022-12-28 13:43:27 -0500, Tom Lane wrote: > >> Hmm ... I guess the buildfarm would tell us whether that detection works > >> correctly on platforms where it matters. Let's keep it simple if we > >> can. > > > Quick clarification question: Are you suggesting to use #ifdef __GNUC__, or > > that it suffices to use -Werror=unknown-pragmas without a separate configure > > check? > > I'd try -Werror=unknown-pragmas, and then go to the #ifdef if that > doesn't seem to work well.
It turns out to not work terribly well. gcc, quite reasonably, warns about the pragma used in .c files, and there's no easy way that I found to have autoconf name its test .h. We could include a test header in the compile test, but that also adds some complication. As gcc has supported the pragma since 2000, I think a simple #ifdef __GNUC__ #define HAVE_PRAGMA_SYSTEM_HEADER 1 #endif should suffice. I started to wonder if what we should do instead is to do something like #ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdeclaration-after-statement" #pragma GCC diagnostic ignored "-Wshadow=compatible-local" #endif #include "EXTERN.h" #include "perl.h" #ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC #pragma GCC diagnostic pop #endif but that ends up quite complicated because gcc will warn about unknown warnings being ignored: ../../../../home/andres/src/postgresql/src/pl/plperl/plperl.h:87:32: warning: unknown option after ‘#pragma GCC diagnostic’ kind [-Wpragmas] 87 | #pragma GCC diagnostic ignored "-Wfrakbar" which would mean we'd need to define a pg_config.h symbol for each potentially ignored warning, and to guard each '#pragma GCC diagnostic ignored "..."' with an #ifdef. Thus I propose the attached. Should we backpatch this? Given the volume of warnings it's probably a good idea. But I'd let it step in HEAD for a few days of buildfarm coverage first. Greetings, Andres Freund
>From c803f9362c551244cb4dee8eb053fbccdb796f68 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Wed, 28 Dec 2022 10:09:48 -0800 Subject: [PATCH v2] perl: Hide warnings inside perl.h when using gcc compatible compiler New versions of perl trigger warnings within perl.h with our compiler flags. At least -Wdeclaration-after-statement, -Wshadow=compatible-local are known to be problematic. To avoid these warnings, conditionally use #pragma GCC system_header before including plperl.h. Alternatively, we could add the include paths for problematic headers with -isystem, but that is a larger hammer and is harder to search for. A more granular alternative would be to use #pragma GCC diagnostic push/ignored/pop, but gcc warns about unknown warnings being ignored, so every to-be-ignored-temporarily compiler warning would require its own pg_config.h symbol and #ifdef. Author: Andres Freund <and...@anarazel.de> Discussion: Discussion: https://postgr.es/m/20221228182455.hfdwd22zztvko...@awork3.anarazel.de --- src/include/c.h | 28 ++++++++++++++++++++++++++++ src/pl/plperl/plperl.h | 9 +++++++++ 2 files changed, 37 insertions(+) diff --git a/src/include/c.h b/src/include/c.h index 7567ef7888b..a7563cb1e42 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -381,6 +381,34 @@ typedef void (*pg_funcptr_t) (void); */ #define FLEXIBLE_ARRAY_MEMBER /* empty */ +/* + * Does the compiler support #pragma GCC system_header? We optionally use it + * to avoid warnings that we can't fix (e.g. in the perl headers). + * See https://gcc.gnu.org/onlinedocs/cpp/System-Headers.html + * + * Headers for which we do not want to show compiler warnings can, + * conditionally, use #pragma GCC system_header to avoid warnings. Obviously + * this should only be used for external headers over which we do not have + * control. + * + * Support for the pragma is tested here, instead of during configure, as gcc + * also warns about the pragma being used in a .c file. It's surprisingly hard + * to get autoconf to use .h as the file-ending. Looks like gcc has + * implemented the pragma since the 2000, so this test should suffice. + * + * + * Alternatively, we could add the include paths for problematic headers with + * -isystem, but that is a larger hammer and is harder to search for. + * + * A more granular alternative would be to use #pragma GCC diagnostic + * push/ignored/pop, but gcc warns about unknown warnings being ignored, so + * every to-be-ignored-temporarily compiler warning would require its own + * pg_config.h symbol and #ifdef. + */ +#ifdef __GNUC__ +#define HAVE_PRAGMA_SYSTEM_HEADER 1 +#endif + /* ---------------------------------------------------------------- * Section 2: bool, true, false diff --git a/src/pl/plperl/plperl.h b/src/pl/plperl/plperl.h index 5243f308bd5..b4f693cbb2d 100644 --- a/src/pl/plperl/plperl.h +++ b/src/pl/plperl/plperl.h @@ -74,6 +74,15 @@ #define HAS_BOOL 1 #endif +/* + * Newer versions the perl headers trigger a lot of warnings with our compiler + * flags (at least -Wdeclaration-after-statement, -Wshadow=compatible-local + * are known to be problematic). The system_header pragma hides warnings from + * within the rest of this file, if supported. + */ +#ifdef HAVE_PRAGMA_SYSTEM_HEADER +#pragma GCC system_header +#endif /* * Get the basic Perl API. We use PERL_NO_GET_CONTEXT mode so that our code -- 2.38.0
