MIME-Version: 1.0
References: <CAHut+Pve+Dyq_piJPG0Ykhn7-gnBsXqge2HQt-baB4i9TJAfnA@mail.gmail.com>
In-Reply-To: <CAHut+Pve+Dyq_piJPG0Ykhn7-gnBsXqge2HQt-baB4i9TJAfnA@mail.gmail.com>
From: David Rowley <dgrowleyml@gmail.com>
Date: Thu, 12 Sep 2024 12:58:19 +1200
Message-ID: <CAApHDvpkr2TdZss4amJGXh0cxVzU+DHpphnmgE89Qazb58Vc6w@mail.gmail.com>
Subject: Re: Remove shadowed declaration warnings
To: Peter Smith <smithpb2250@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: <https://www.postgresql.org/message-id/CAApHDvpkr2TdZss4amJGXh0cxVzU%2BDHpphnmgE89Qazb58Vc6w%40mail.gmail.com>
Precedence: bulk

On Thu, 12 Sept 2024 at 12:33, Peter Smith <smithpb2250@gmail.com> wrote:
> I normally build the code with warnings enabled (specifically,
> -Wshadow) which exposes many "shadowed" declarations.
>
> It would be better to reduce warnings wherever it's easy to do so,
> because if we always see/ignore lots of warnings then sooner or later
> something important may escape attention. In any case, just removing
> these warnings sometimes makes the code more readable by removing any
> ambiguity.

0fe954c28 did add -Wshadow=compatible-local to the standard set of
complication flags.  I felt it was diminishing returns after that, but
-Wshadow=local would be the next step before going full -Wshadow.

There was justification for -Wshadow=compatible-local because there
has been > 1 bug (see af7d270dd) fixed that would have been found if
we'd had that sooner. Have we ever had any bugs that would have been
highlighted by -Wshadow but not -Wshadow=compatible-local? I'd be
curious to know if you do go through this process of weeding these out
if you do find any bugs as a result.

I also wonder if we could ever get this to a stable point. I didn't
take the time to understand what 388e80132 did. Is that going to
protect us from getting warnings where fixing them is beyond our
control for full -Wshadow?

David