pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Cc: pgsql-hackers@postgresql.org, Tomas Vondra <tomas.vondra@postgresql.org>
Cc: Peter Smith <smithpb2250@gmail.com>
Subject: Re: shadow variables - pg15 edition
Date: Mon, 10 Oct 2022 12:06:22 -0400
Message-ID: <4139736.1665417982@sss.pgh.pa.us> (raw)
In-Reply-To: <CAApHDvrEdL0bK7775NXLL2q8Y3hs5O-WY2KY5U8noX=bhaWiWA@mail.gmail.com>
References: <20220830054441.GF31833@telsasoft.com>
	<CAApHDvq4dg7M+_imHZJ3bxNRW53EBkDuo6jxmSnMtrtX_Br3fQ@mail.gmail.com>
	<20221004023012.GT7745@telsasoft.com>
	<CAApHDvoRsphHHznoBAsPaQEDvFka7W0ONm8mwXx6x6Zs0h+FmA@mail.gmail.com>
	<CAApHDvqWGMdB_pATeUqE=JCtNqNxObPOJ00jFEa2_sZ20j_Wvg@mail.gmail.com>
	<2321227.1664979561@sss.pgh.pa.us>
	<CAApHDvr9pu5bWasbxCJjLyqVsxeVkWkwCRW8yHGGTFMM=63uBA@mail.gmail.com>
	<20221005214052.c4tkudawyp5wxt3c@awork3.anarazel.de>
	<CAApHDvpCCXcw1LSPn0OWvMW=R+eh-d5=P=3FiKsipDhihMW72w@mail.gmail.com>
	<CAApHDvpFy3Bve8q+gKTFygGd+vr4oUb7mf2ogDhju_bp=7r88g@mail.gmail.com>
	<20221006003920.6xlqaoccxwisza5k@awork3.anarazel.de>
	<CAApHDvrCYoAGz9Wn=baR8xY01QJiWGNd6Nc_MTOqSoDT0pSP7A@mail.gmail.com>
	<CAApHDvrEdL0bK7775NXLL2q8Y3hs5O-WY2KY5U8noX=bhaWiWA@mail.gmail.com>

David Rowley <dgrowleyml@gmail.com> writes:
> On Fri, 7 Oct 2022 at 13:24, David Rowley <dgrowleyml@gmail.com> wrote:
>> Since I just committed the patch to fix the final warnings, I think we
>> should go ahead and commit the patch you wrote to add
>> -Wshadow=compatible-local to the standard build flags. I don't mind
>> doing this.

> Pushed.

The buildfarm's showing a few instances of this warning, which seem
to indicate that not all versions of the Perl headers are clean:

 fairywren     | 2022-10-10 09:03:50 | C:/Perl64/lib/CORE/cop.h:612:13: warning: declaration of 'av' shadows a previous local [-Wshadow=compatible-local]
 fairywren     | 2022-10-10 09:03:50 | C:/Perl64/lib/CORE/cop.h:612:13: warning: declaration of 'av' shadows a previous local [-Wshadow=compatible-local]
 fairywren     | 2022-10-10 09:03:50 | C:/Perl64/lib/CORE/cop.h:612:13: warning: declaration of 'av' shadows a previous local [-Wshadow=compatible-local]
 fairywren     | 2022-10-10 09:03:50 | C:/Perl64/lib/CORE/cop.h:612:13: warning: declaration of 'av' shadows a previous local [-Wshadow=compatible-local]
 fairywren     | 2022-10-10 09:03:50 | C:/Perl64/lib/CORE/cop.h:612:13: warning: declaration of 'av' shadows a previous local [-Wshadow=compatible-local]
 fairywren     | 2022-10-10 09:03:50 | C:/Perl64/lib/CORE/cop.h:612:13: warning: declaration of 'av' shadows a previous local [-Wshadow=compatible-local]
 snakefly      | 2022-10-10 08:21:05 | Util.c:457:14: warning: declaration of 'cv' shadows a parameter [-Wshadow=compatible-local]

Before you ask:

fairywren: perl 5.24.3
snakefly: perl 5.16.3

which are a little old, but not *that* old.

Scraping the configure logs also shows that only half of the buildfarm
(exactly 50 out of 100 reporting animals) knows -Wshadow=compatible-local,
which suggests that we might see more of these if they all did.  On the
other hand, animals with newer compilers probably also have newer Perl
installations, so assuming that the Perl crew have kept this clean
recently, maybe not.

Not sure if this is problematic enough to justify removing the switch.
A plausible alternative is to have a few animals with known-clean Perl
installations add the switch manually (and use -Werror), so that we find
out about violations without having warnings in the face of developers
who can't fix them.  I'm willing to wait to see if anyone complains of
such warnings, though.

			regards, tom lane





view thread (56+ messages)  latest in thread

Message-ID: <4139736.1665417982@sss.pgh.pa.us>
Permalink:  ../4139736.1665417982@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/4139736.1665417982@sss.pgh.pa.us

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, dgrowleyml@gmail.com, andres@anarazel.de, pryzby@telsasoft.com, tomas.vondra@postgresql.org, smithpb2250@gmail.com
  Subject: Re: shadow variables - pg15 edition
  In-Reply-To: <4139736.1665417982@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox