Re: Cleanup shadows variable warnings, round 1
Chao Li <li.evan.chao@gmail.com>
From: Chao Li <li.evan.chao@gmail.com>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Álvaro Herrera <alvherre@kurilemu.de>, Postgres hackers <pgsql-hackers@lists.postgresql.org>, Peter Eisentraut <peter@eisentraut.org>, Peter Smith <smithpb2250@gmail.com>
Date: 2026-04-23T06:34:03Z
Lists: pgsql-hackers
> On Apr 23, 2026, at 13:19, David Rowley <dgrowleyml@gmail.com> wrote: > > 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. > > Thank you. I pushed those after some adjustments. > > 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. > > 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. > > 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=compatible-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? > > 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’ll 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/
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Fix new-to-v19 -Wshadow warnings
- 4f0cbc6fb5df 19 (unreleased) landed
-
Reduce scope of for-loop-local variables to avoid shadowing
- cece37c9843c 19 (unreleased) landed
-
Fix local-variable shadowing in pg_trgm's printSourceNFA().
- cdaa67565867 19 (unreleased) cited