Re: Remove shadowed declaration warnings

On 12.09.24 04:25, David Rowley wrote:
> On Thu, 12 Sept 2024 at 14:03, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I do grant that sometimes shadowing of locals can cause bugs. I don't
>> recall right now why we opted for -Wshadow=compatible-local over
>> -Wshadow=local, but we could certainly take another look at that.
>
> I don't recall if it was discussed, but certainly, it was an easier
> goal to achieve.
>
> Looks like there are currently 47 warnings with -Wshadow=local. I'm
> not quite sure what the definition of "compatible" is for this flag,
> but looking at one warning in pgbench.c:4548, I see an int shadowing a
> bool. So maybe -Wshadow=local is worthwhile.

Another thing to keep in mind with these different shadow warning
variants is that we should try to keep them consistent across compilers,
at least the common ones. The current -Wshadow=compatible-local is only
available in gcc, not in clang, so it's currently impossible to rely on
clang to write warning-free code.

Of course we have other warning flags that we use that don't exist in
all compilers, but in my experience these are either for very esoteric
cases or something that is very obviously wrong and a developer would
normally see immediately. For example, there is no warning flag in
clang that mirrors the switch "fallthrough" labeling that we have set up
with gcc. But this is not as much of a problem in practice because the
wrong code would usually misbehave in an obvious way and the issue can
be found by looking at the code with two lines of context. With the
shadow warning, the issues are much harder to find visually, and in most
cases they are not actually a problem.

The shadow warning levels in gcc and clang appear to be very differently
structured, so I'm not sure how we can improve interoperability here.