Обсуждение: perl 5.36, C99, -Wdeclaration-after-statement -Wshadow=compatible-local
Hi,
Tom pinged me privately because mylodon, an animal enforcing C89/C99
compatibility, was failed. This is due to perl on the machine being upgraded
to perl 5.36.
Mylodon was failing because of:
configure:18839: ccache clang-13 -c -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement
-Werror=vla-Werror=unguarded-availability-new -Wendif-labels -Wmissing-format-attribute -Wcast-function-type
-Wformat-security-fno-strict-aliasing -fwrapv -Wno-unused-command-line-argument -Wno-compound-token-split-by-macro -g
-O1-ggdb -g3 -fno-omit-frame-pointer -Wall -Wextra -Wno-unused-parameter -Wno-sign-compare
-Wno-missing-field-initializers-Wno-array-bounds -std=c99 -Wc11-extensions -Werror=c11-extensions -D_GNU_SOURCE
-I/usr/include/libxml2 -I/usr/lib/x86_64-linux-gnu/perl/5.36/CORE conftest.c >&5
In file included from conftest.c:170:
In file included from /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/perl.h:5777:
/usr/lib/x86_64-linux-gnu/perl/5.36/CORE/thread.h:386:8: error: '_Thread_local' is a C11 extension
[-Werror,-Wc11-extensions]
extern PERL_THREAD_LOCAL void *PL_current_context;
^
/usr/lib/x86_64-linux-gnu/perl/5.36/CORE/config.h:5154:27: note: expanded from macro 'PERL_THREAD_LOCAL'
#define PERL_THREAD_LOCAL _Thread_local /**/
^
1 error generated.
I.e. perl's headers use C11 features, which unsurprisingly doesn't work when
using -Wc11-extensions -Werror=c11-extensions.
For now I worked around this by disabling perl for mylodon, but that's
obviously not a great fix.
perl 5.36 also causes a bunch of warnings locally, where I obviously don't
use -Wc11-extensions -Werror=c11-extensions:
-Wdeclaration-after-statement produces a few copies of:
[1767/2259 42 78%] Compiling C object src/pl/plperl/plperl.so.p/meson-generated_.._SPI.c.o
In file included from /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/perl.h:7242,
from ../../../../home/andres/src/postgresql/src/pl/plperl/plperl.h:82,
from ../../../../home/andres/src/postgresql/src/pl/plperl/SPI.xs:15:
/usr/lib/x86_64-linux-gnu/perl/5.36/CORE/inline.h: In function ‘Perl_cop_file_avn’:
/usr/lib/x86_64-linux-gnu/perl/5.36/CORE/inline.h:3489:5: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
3489 | const char *file = CopFILE(cop);
| ^~~~~
And -Wshadow=compatible-local triggers the following, very verbose, warning:
[1767/2259 42 78%] Compiling C object src/pl/plperl/plperl.so.p/meson-generated_.._SPI.c.o
...
In file included from /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/perl.h:4155:
/usr/lib/x86_64-linux-gnu/perl/5.36/CORE/sv_inline.h: In function ‘Perl_newSV_type’:
/usr/lib/x86_64-linux-gnu/perl/5.36/CORE/handy.h:97:35: warning: declaration of ‘p_’ shadows a previous local
[-Wshadow=compatible-local]
97 | # define MUTABLE_PTR(p) ({ void *p_ = (p); p_; })
| ^~
/usr/lib/x86_64-linux-gnu/perl/5.36/CORE/sv.h:1394:54: note: in definition of macro ‘SvSTASH_set’
1394 | (((XPVMG*) SvANY(sv))->xmg_stash = (val)); } STMT_END
| ^~~
/usr/lib/x86_64-linux-gnu/perl/5.36/CORE/handy.h:105:32: note: in expansion of macro ‘MUTABLE_PTR’
105 | #define MUTABLE_HV(p) ((HV *)MUTABLE_PTR(p))
| ^~~~~~~~~~~
/usr/lib/x86_64-linux-gnu/perl/5.36/CORE/sv_inline.h:487:29: note: in expansion of macro ‘MUTABLE_HV’
487 | SvSTASH_set(io, MUTABLE_HV(SvREFCNT_inc(GvHV(iogv))));
| ^~~~~~~~~~
/usr/lib/x86_64-linux-gnu/perl/5.36/CORE/handy.h:107:32: note: in expansion of macro ‘MUTABLE_PTR’
107 | #define MUTABLE_SV(p) ((SV *)MUTABLE_PTR(p))
| ^~~~~~~~~~~
/usr/lib/x86_64-linux-gnu/perl/5.36/CORE/sv.h:346:59: note: in expansion of macro ‘MUTABLE_SV’
346 | #define SvREFCNT_inc(sv) Perl_SvREFCNT_inc(MUTABLE_SV(sv))
| ^~~~~~~~~~
/usr/lib/x86_64-linux-gnu/perl/5.36/CORE/sv_inline.h:487:40: note: in expansion of macro ‘SvREFCNT_inc’
487 | SvSTASH_set(io, MUTABLE_HV(SvREFCNT_inc(GvHV(iogv))));
| ^~~~~~~~~~~~
/usr/lib/x86_64-linux-gnu/perl/5.36/CORE/handy.h:97:35: note: shadowed declaration is here
97 | # define MUTABLE_PTR(p) ({ void *p_ = (p); p_; })
| ^~
/usr/lib/x86_64-linux-gnu/perl/5.36/CORE/sv.h:1394:54: note: in definition of macro ‘SvSTASH_set’
1394 | (((XPVMG*) SvANY(sv))->xmg_stash = (val)); } STMT_END
| ^~~
/usr/lib/x86_64-linux-gnu/perl/5.36/CORE/handy.h:105:32: note: in expansion of macro ‘MUTABLE_PTR’
105 | #define MUTABLE_HV(p) ((HV *)MUTABLE_PTR(p))
| ^~~~~~~~~~~
/usr/lib/x86_64-linux-gnu/perl/5.36/CORE/sv_inline.h:487:29: note: in expansion of macro ‘MUTABLE_HV’
487 | SvSTASH_set(io, MUTABLE_HV(SvREFCNT_inc(GvHV(iogv))));
| ^~~~~~~~~~
I don't know how much longer we can rely on headers being
-Wdeclaration-after-statement clean, my impression is that people don't have a
lot of patience for C89isms anymore.
I suspect the shadowing issue might get fixed if we report it, there've been a
bunch of fixes around that not too long ago.
I wonder if we should try to use -isystem for a bunch of external
dependencies. That way we can keep the more aggressive warnings with a lower
likelihood of conflicting with stuff outside of our control.
Greetings,
Andres Freund
Re: perl 5.36, C99, -Wdeclaration-after-statement -Wshadow=compatible-local
От
Dagfinn Ilmari Mannsåker
Дата:
Andres Freund <andres@anarazel.de> writes: > Hi, > > Tom pinged me privately because mylodon, an animal enforcing C89/C99 > compatibility, was failed. This is due to perl on the machine being upgraded > to perl 5.36. > > Mylodon was failing because of: > > configure:18839: ccache clang-13 -c -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla-Werror=unguarded-availability-new -Wendif-labels -Wmissing-format-attribute -Wcast-function-type -Wformat-security-fno-strict-aliasing -fwrapv -Wno-unused-command-line-argument -Wno-compound-token-split-by-macro -g -O1-ggdb -g3 -fno-omit-frame-pointer -Wall -Wextra -Wno-unused-parameter -Wno-sign-compare -Wno-missing-field-initializers-Wno-array-bounds -std=c99 -Wc11-extensions -Werror=c11-extensions -D_GNU_SOURCE -I/usr/include/libxml2 -I/usr/lib/x86_64-linux-gnu/perl/5.36/CORE conftest.c >&5 > In file included from conftest.c:170: > In file included from /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/perl.h:5777: > /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/thread.h:386:8: error: '_Thread_local' is a C11 extension [-Werror,-Wc11-extensions] > extern PERL_THREAD_LOCAL void *PL_current_context; > ^ > /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/config.h:5154:27: note: expanded from macro 'PERL_THREAD_LOCAL' > #define PERL_THREAD_LOCAL _Thread_local /**/ > ^ > 1 error generated. > > > I.e. perl's headers use C11 features, which unsurprisingly doesn't work when > using -Wc11-extensions -Werror=c11-extensions. Like Postgres, Perl only requires C99, so any newer features such as _Thread_local are conditional on compiler support (probed by Configure). We're not actively testing the fallbacks at the moment, but I'll look at adding a CI job with the appropriate -Werror flags to make sure it doesn't break in future. > For now I worked around this by disabling perl for mylodon, but that's > obviously not a great fix. An option would be to build a custom perl with the same -Werror flags mylodon uses for Postgres (via the -Accflags option to Configure), and then building Postgres against that. > perl 5.36 also causes a bunch of warnings locally, where I obviously don't > use -Wc11-extensions -Werror=c11-extensions: > > -Wdeclaration-after-statement produces a few copies of: > [1767/2259 42 78%] Compiling C object src/pl/plperl/plperl.so.p/meson-generated_.._SPI.c.o > In file included from /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/perl.h:7242, > from ../../../../home/andres/src/postgresql/src/pl/plperl/plperl.h:82, > from ../../../../home/andres/src/postgresql/src/pl/plperl/SPI.xs:15: > /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/inline.h: In function ‘Perl_cop_file_avn’: > /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/inline.h:3489:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] > 3489 | const char *file = CopFILE(cop); > | ^~~~~ > > I don't know how much longer we can rely on headers being > -Wdeclaration-after-statement clean, my impression is that people don't have a > lot of patience for C89isms anymore. Perl's C99 policy (https://perldoc.perl.org/perlhacktips#C99) explicitly permits mixed declarations and code, so I don't think that's likely to change. > And -Wshadow=compatible-local triggers the following, very verbose, warning: > > [1767/2259 42 78%] Compiling C object src/pl/plperl/plperl.so.p/meson-generated_.._SPI.c.o > ... > In file included from /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/perl.h:4155: > /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/sv_inline.h: In function ‘Perl_newSV_type’: > /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/handy.h:97:35: warning: > declaration of ‘p_’ shadows a previous local [-Wshadow=compatible-local] > 97 | # define MUTABLE_PTR(p) ({ void *p_ = (p); p_; }) > | ^~ > /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/sv.h:1394:54: note: in definition of macro ‘SvSTASH_set’ > 1394 | (((XPVMG*) SvANY(sv))->xmg_stash = (val)); } STMT_END > | ^~~ > /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/handy.h:105:32: note: in expansion of macro ‘MUTABLE_PTR’ > 105 | #define MUTABLE_HV(p) ((HV *)MUTABLE_PTR(p)) > | ^~~~~~~~~~~ > /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/sv_inline.h:487:29: note: in expansion of macro ‘MUTABLE_HV’ > 487 | SvSTASH_set(io, MUTABLE_HV(SvREFCNT_inc(GvHV(iogv)))); > | ^~~~~~~~~~ > /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/handy.h:107:32: note: in expansion of macro ‘MUTABLE_PTR’ > 107 | #define MUTABLE_SV(p) ((SV *)MUTABLE_PTR(p)) > | ^~~~~~~~~~~ > /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/sv.h:346:59: note: in expansion of macro ‘MUTABLE_SV’ > 346 | #define SvREFCNT_inc(sv) Perl_SvREFCNT_inc(MUTABLE_SV(sv)) > | ^~~~~~~~~~ > /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/sv_inline.h:487:40: note: in expansion of macro ‘SvREFCNT_inc’ > 487 | SvSTASH_set(io, MUTABLE_HV(SvREFCNT_inc(GvHV(iogv)))); > | ^~~~~~~~~~~~ > /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/handy.h:97:35: note: shadowed declaration is here > 97 | # define MUTABLE_PTR(p) ({ void *p_ = (p); p_; }) > | ^~ > /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/sv.h:1394:54: note: in definition of macro ‘SvSTASH_set’ > 1394 | (((XPVMG*) SvANY(sv))->xmg_stash = (val)); } STMT_END > | ^~~ > /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/handy.h:105:32: note: in expansion of macro ‘MUTABLE_PTR’ > 105 | #define MUTABLE_HV(p) ((HV *)MUTABLE_PTR(p)) > | ^~~~~~~~~~~ > /usr/lib/x86_64-linux-gnu/perl/5.36/CORE/sv_inline.h:487:29: note: in expansion of macro ‘MUTABLE_HV’ > 487 | SvSTASH_set(io, MUTABLE_HV(SvREFCNT_inc(GvHV(iogv)))); > | ^~~~~~~~~~ > > I suspect the shadowing issue might get fixed if we report it, there've been a > bunch of fixes around that not too long ago. This one might be a bit tricky to fix. The root cause is the MUTABLE_PTR macro, which is meant to allow casting between different pointer types without accientally losing constness, which (when GCC brace groups are supported) is defined as: # define MUTABLE_PTR(p) ({ void *p_ = (p); p_; }) And then we have: #define MUTABLE_xV(p) ((xV *)MUTABLE_PTR(p)) etc. for all the different value types (AV, GV, HV, SV, etc.) In the above case, the SvREFCNT_inc() inside the MUTABLE_HV() expands to something that contains a MUTABLE_SV() call, causing the inner `p_` variable to shadow the outer one. > I wonder if we should try to use -isystem for a bunch of external > dependencies. That way we can keep the more aggressive warnings with a lower > likelihood of conflicting with stuff outside of our control. That is worth considering, at least if the above can't easily be fixed, or if we run across more dependencies with similar problems. > Greetings, > > Andres Freund - ilmari
Re: perl 5.36, C99, -Wdeclaration-after-statement -Wshadow=compatible-local
От
Peter Eisentraut
Дата:
On 01.11.22 19:01, Andres Freund wrote: > I don't know how much longer we can rely on headers being > -Wdeclaration-after-statement clean, my impression is that people don't have a > lot of patience for C89isms anymore. > I wonder if we should try to use -isystem for a bunch of external > dependencies. That way we can keep the more aggressive warnings with a lower > likelihood of conflicting with stuff outside of our control. Python has the same issues. There are a few other Python-embedding projects that use -Wdeclaration-after-statement and complain if the Python headers violate it. But it's getting tedious. -isystem would be a better solution.
Hi, On 2022-11-01 17:00:27 -0400, Peter Eisentraut wrote: > On 01.11.22 19:01, Andres Freund wrote: > > I don't know how much longer we can rely on headers being > > -Wdeclaration-after-statement clean, my impression is that people don't have a > > lot of patience for C89isms anymore. > > > I wonder if we should try to use -isystem for a bunch of external > > dependencies. That way we can keep the more aggressive warnings with a lower > > likelihood of conflicting with stuff outside of our control. > > Python has the same issues. There are a few other Python-embedding projects > that use -Wdeclaration-after-statement and complain if the Python headers > violate it. But it's getting tedious. -isystem would be a better solution. Which dependencies should we convert to -isystem? And I assume we should do so with meson and autoconf? It's easy with meson, it provides a function to change a dependency to use -isystem without knowing how the compiler spells that. I guess with autoconf we'd have to check if the compiler understands -isystem? The other alternative would be to drop -Wdeclaration-after-statement :) Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On 2022-11-01 17:00:27 -0400, Peter Eisentraut wrote:
>> Python has the same issues. There are a few other Python-embedding projects
>> that use -Wdeclaration-after-statement and complain if the Python headers
>> violate it. But it's getting tedious. -isystem would be a better solution.
> Which dependencies should we convert to -isystem?
Color me confused about what's being discussed here. I see nothing
in the gcc manual suggesting that -isystem has any effect on warning
levels?
regards, tom lane
Hi,
On 2022-11-02 19:57:45 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2022-11-01 17:00:27 -0400, Peter Eisentraut wrote:
> >> Python has the same issues. There are a few other Python-embedding projects
> >> that use -Wdeclaration-after-statement and complain if the Python headers
> >> violate it. But it's getting tedious. -isystem would be a better solution.
>
> > Which dependencies should we convert to -isystem?
>
> Color me confused about what's being discussed here. I see nothing
> in the gcc manual suggesting that -isystem has any effect on warning
> levels?
It's only indirectly explained :(
The -isystem and -idirafter options also mark the directory as a system directory, so that it gets the same
specialtreatment that is applied to
the standard system directories.
and then https://gcc.gnu.org/onlinedocs/cpp/System-Headers.html
Greetings,
Andres Freund
Hi, On 2022-11-02 17:03:34 -0700, Andres Freund wrote: > On 2022-11-02 19:57:45 -0400, Tom Lane wrote: > > Andres Freund <andres@anarazel.de> writes: > > > On 2022-11-01 17:00:27 -0400, Peter Eisentraut wrote: > > >> Python has the same issues. There are a few other Python-embedding projects > > >> that use -Wdeclaration-after-statement and complain if the Python headers > > >> violate it. But it's getting tedious. -isystem would be a better solution. > > > > > Which dependencies should we convert to -isystem? > > > > Color me confused about what's being discussed here. I see nothing > > in the gcc manual suggesting that -isystem has any effect on warning > > levels? > > It's only indirectly explained :( > > The -isystem and -idirafter options also mark the directory as a system directory, so that it gets the samespecial treatment that is applied to > the standard system directories. > > and then https://gcc.gnu.org/onlinedocs/cpp/System-Headers.html The attached *prototype* patch is a slightly different spin on the idea of using -isystem: It adds a #pragma GCC system_header to plperl.h if supported by the compiler. That also avoids warnings from within plperl and subsidiary headers. I don't really have an opinion about whether using the pragma or -isystem is preferrable. I chose the pragma because it makes it easier to grep for headers where we chose to do this. I added the pragma detection only to the meson build, but if others think this is a good way to go, I'll do the necessary autoconf wrangling as well. In the compiler test, I chose to not check whether -Werror=unknown-pragmas is supported - it appears to be an old gcc flag, and the worst outcome is that HAVE_PRAGMA_SYSTEM_HEADER isn't defined. We could alternatively define HAVE_PRAGMA_SYSTEM_HEADER or such based on __GNUC__ being defined. Greetings, Andres Freund
Вложения
Andres Freund <andres@anarazel.de> writes:
> The attached *prototype* patch is a slightly different spin on the idea of
> using -isystem: It adds a
> #pragma GCC system_header
> to plperl.h if supported by the compiler. That also avoids warnings from
> within plperl and subsidiary headers.
> I don't really have an opinion about whether using the pragma or -isystem is
> preferrable. I chose the pragma because it makes it easier to grep for headers
> where we chose to do this.
This seems like a reasonable answer. It feels quite a bit less magic
in the way that it suppresses warnings than -isystem, and also less
likely to have unexpected side-effects (I have a nasty feeling that
-isystem is more magic on macOS than elsewhere). So far it seems
like only the Perl headers have much of an issue, though I can
foresee Python coming along soon.
> In the compiler test, I chose to not check whether -Werror=unknown-pragmas is
> supported - it appears to be an old gcc flag, and the worst outcome is that
> HAVE_PRAGMA_SYSTEM_HEADER isn't defined.
> We could alternatively define HAVE_PRAGMA_SYSTEM_HEADER or such based on
> __GNUC__ being defined.
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.
regards, tom lane
On 2022-12-28 13:43:27 -0500, Tom Lane wrote: > > In the compiler test, I chose to not check whether -Werror=unknown-pragmas is > > supported - it appears to be an old gcc flag, and the worst outcome is that > > HAVE_PRAGMA_SYSTEM_HEADER isn't defined. > > We could alternatively define HAVE_PRAGMA_SYSTEM_HEADER or such based on > > __GNUC__ being defined. > > 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?
Andres Freund <andres@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.
regards, tom lane
Hi, On 2022-12-28 19:05:35 -0500, Tom Lane wrote: > Andres Freund <andres@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
Вложения
Andres Freund <andres@anarazel.de> writes:
> 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.
We might find that some GCC-impostor compilers have trouble with it,
but if so we can adjust the #ifdef here.
Getting nitpicky, I suggest calling it "HAVE_PRAGMA_GCC_SYSTEM_HEADER"
to align better with what you actually have to write. Also:
+ * Newer versions the perl headers trigger a lot of warnings with our compiler
"Newer versions of ..." please. Otherwise LGTM.
> 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.
+1 to both points.
regards, tom lane
Hi, On 2022-12-29 13:51:37 -0500, Tom Lane wrote: > We might find that some GCC-impostor compilers have trouble with it, > but if so we can adjust the #ifdef here. Yea. I suspect it's widely enough used that any compiler claiming to be gcc compatible has it, but ... > Getting nitpicky, I suggest calling it "HAVE_PRAGMA_GCC_SYSTEM_HEADER" > to align better with what you actually have to write. Makes sense. > + * Newer versions the perl headers trigger a lot of warnings with our compiler > > "Newer versions of ..." please. Otherwise LGTM. Oops. > > 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. > > +1 to both points. Pushed to HEAD. Greetings, Andres Freund
Hi, On 2022-12-29 13:40:13 -0800, Andres Freund wrote: > > > 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. > > > > +1 to both points. > > Pushed to HEAD. I haven't seen any problems in HEAD, so I'm working on backpatching. Greetings, Andres Freund
On 2023-01-02 15:46:36 -0800, Andres Freund wrote: > On 2022-12-29 13:40:13 -0800, Andres Freund wrote: > > > > 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. > > > > > > +1 to both points. > > > > Pushed to HEAD. > > I haven't seen any problems in HEAD, so I'm working on backpatching. And done.
Andres Freund <andres@anarazel.de> writes:
> On 2023-01-02 15:46:36 -0800, Andres Freund wrote:
>> I haven't seen any problems in HEAD, so I'm working on backpatching.
> And done.
It occurs to me that we should now be able to drop configure's
probe for -Wno-compound-token-split-by-macro, since that was only
needed to suppress warnings in the Perl headers. Won't save much
of course, but every test we can get rid of is worth doing IMO.
regards, tom lane
I wrote:
> It occurs to me that we should now be able to drop configure's
> probe for -Wno-compound-token-split-by-macro, since that was only
> needed to suppress warnings in the Perl headers.
... or not. A bit of experimentation says that they still come out,
apparently because the warnings are triggered by *use* of relevant
Perl macros not by their *definitions*. Oh well.
regards, tom lane