Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Re: Cleanup shadows variable warnings, round 1
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: 
 <CAApHDvp1AnEsoqi7jHAn-p9ejqwYx=-21Zw7d2HrmSU2vry7Hg@mail.gmail.com>
Date: Thu, 23 Apr 2026 14:34:03 +0800
Cc: =?utf-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de>,
 Postgres hackers <pgsql-hackers@lists.postgresql.org>,
 Peter Eisentraut <peter@eisentraut.org>,
 Peter Smith <smithpb2250@gmail.com>
Content-Transfer-Encoding: quoted-printable
Message-Id: <27BB52C6-74C9-462A-8A6C-9DC3531E2146@gmail.com>
References: <aeeADDomU6yAjCOb@alvherre.pgsql>
 <8B830174-7794-411F-B936-37DDB509EC5D@gmail.com>
 <CAApHDvp1AnEsoqi7jHAn-p9ejqwYx=-21Zw7d2HrmSU2vry7Hg@mail.gmail.com>
To: David Rowley <dgrowleyml@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/27BB52C6-74C9-462A-8A6C-9DC3531E2146%40gmail.com>
Precedence: bulk


> On Apr 23, 2026, at 13:19, David Rowley <dgrowleyml@gmail.com> wrote:
>=20
> On Wed, 22 Apr 2026 at 17:14, Chao Li <li.evan.chao@gmail.com> wrote:
>> The attached new v1 patch fixes the v19-only shadow warnings. There =
are not many. I strictly limited it to warnings newly introduced in v19, =
without touching any pre-existing ones, even where an old occurrence is =
very close to a new one.
>=20
> Thank you. I pushed those after some adjustments.
>=20
> While doing that, I did think more on if we should do more of this for
> v20. I keep thinking back to the times when I've had to write 6
> different versions of a patch to back patch to 6 different branches.
> It's rarely that bad, but it sure does make you swear when the 6th
> "git am" fails, especially when you find out that it was for a very
> trivial thing, such as a spelling mistake fix. You really have to
> fight off the temptation of complacency after the first 3 or so failed
> git ams.
>=20
> A worse category of problems that this particular set of patches could
> cause is no conflict when we want one. I personally always write bug
> fixes for master and back-patch them, but if anyone were to work
> forward to newer versions, then imagine someone adding some code to a
> function that does something with a local variable that's shadowed
> globally. If they forward patch that to a version where the local
> variable has been renamed, everything compiles and might appear to
> work, but it's now the global that's being changed when the new code
> was meant to change the local. Maybe no committers work that way, but
> if they do, it's a real risk.
>=20
> IMO, without any references to recent bugs that have been fixed due to
> shadowing, then I can't see beyond the fact that this might be more
> likely to cause bugs than to prevent them. As I recall, we were about
> borderline on doing -Wshadow=3Dcompatible-local.  At least for
> non-compatible variables, I'd expect you'd get a warning or error
> during compilation. For the record, I got motivated for Justin's work
> on the compatible-local due to af7d270dd. I removed a shadowed
> variable which was incorrect. In my view, Justin Pryzby's proposal to
> do something about this was well timed. I'm not seeing the same thing
> happen here. Maybe I missed it?
>=20
> David

Hi David,

Thank you very much for accepting this v19-only patch.

I helped prepare back-patch diff files for [1] today, from v10 to v18. =
It was only a tiny change, but I still ended up with 3 diff files across =
9 branches, which was quite painful. I can understand that, as a =
committer, you probably run into that kind of pain regularly, and would =
prefer to avoid adding more of it.

I=E2=80=99ll hold off on the rest of this cleanup unless there is a =
concrete reason to revisit it in the future.

[1] https://postgr.es/21E668C0-CEAE-44F8-B585-319F31883AFE@gmail.com

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/