public inbox for [email protected]  

help / color / mirror / Atom feed

From: Peter Eisentraut <[email protected]>
To: David Rowley <[email protected]>
To: Tom Lane <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Remove shadowed declaration warnings
Date: Wed, 18 Sep 2024 09:53:11 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAApHDvqaa3YXqTF2cZ0ocwXvvpqEwAPHpBAO_0Sfwa6ORhYYHQ@mail.gmail.com>
References: <CAHut+Pve+Dyq_piJPG0Ykhn7-gnBsXqge2HQt-baB4i9TJAfnA@mail.gmail.com>
	<CAApHDvpkr2TdZss4amJGXh0cxVzU+DHpphnmgE89Qazb58Vc6w@mail.gmail.com>
	<[email protected]>
	<CAApHDvqaa3YXqTF2cZ0ocwXvvpqEwAPHpBAO_0Sfwa6ORhYYHQ@mail.gmail.com>

On 12.09.24 04:25, David Rowley wrote:
> On Thu, 12 Sept 2024 at 14:03, Tom Lane <[email protected]> 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.

view thread (6+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Remove shadowed declaration warnings
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox