Horton

Re: shadow variables - pg15 edition

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: David Rowley <dgrowleyml@gmail.com>, Justin Pryzby <pryzby@telsasoft.com>, pgsql-hackers@postgresql.org, Tomas Vondra <tomas.vondra@postgresql.org>, Peter Smith <smithpb2250@gmail.com>
Date: 2022-10-10T16:27:32Z
Lists: pgsql-hackers 
Hi,

On 2022-10-10 12:06:22 -0400, Tom Lane wrote:
> 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.

I think it's not just newness - only gcc has compatible-local, even very new
clang doesn't.


This was fixed ~6 years ago in perl:

commit f2b9631d5d19d2b71c1776e1193173d13f3620bf
Author: David Mitchell <davem@iabyn.com>
Date:   2016-05-23 14:43:56 +0100

    CX_POP_SAVEARRAY(): use more distinctive var name

    Under -Wshadow, CX_POP_SAVEARRAY's local var 'av' can generate this
    warning:

        warning: declaration shadows a local variable [-Wshadow]

    So rename it to cx_pop_savearay_av to reduce the risk of a clash.

    (See http://nntp.perl.org/group/perl.perl5.porters/236444)


> 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.

Given the age of affected perl instances I suspect there'll not be a lot of
developers affected, and the number of warnings is reasonably small too. It'd
likely hurt more developers to not see the warnings locally, given that such
shadowing often causes bugs.

Greetings,

Andres Freund

Commits

  1. Fix shadow variable in postgres.c

  2. Add -Wshadow=compatible-local to the standard compilation flags

  3. Fix final warnings produced by -Wshadow=compatible-local

  4. Fix final compiler warning produced by -Wshadow=compatible-local

  5. Add optional parameter to PG_TRY() macros

  6. Small refactor to get rid of -Wshadow=compatible-local warning

  7. More -Wshadow=compatible-local warning fixes

  8. Further -Wshadow=compatible-local warning fixes

  9. Further reduce warnings with -Wshadow=compatible-local

  10. Reduce warnings with -Wshadow=compatible-local builds

  11. Remove shadowed local variables that are new in v15