Re: perl 5.36, C99, -Wdeclaration-after-statement -Wshadow=compatible-local

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, pgsql-hackers(at)postgresql(dot)org, David Rowley <dgrowleyml(at)gmail(dot)com>, Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org>
Subject: Re: perl 5.36, C99, -Wdeclaration-after-statement -Wshadow=compatible-local
Date: 2022-12-29 18:42:36
Message-ID: 20221229184236.3v7bvsumefpslq7h@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2022-12-28 19:05:35 -0500, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)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

Attachment Content-Type Size
v2-0001-perl-Hide-warnings-inside-perl.h-when-using-gcc-c.patch text/x-diff 3.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-12-29 18:45:49 Re: BUG #17717: Regression in vacuumdb (15 is slower than 10/11 and possible memory issue)
Previous Message Nathan Bossart 2022-12-29 18:26:57 Re: BUG #17717: Regression in vacuumdb (15 is slower than 10/11 and possible memory issue)