Re: Using -Wshadow

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
> I wouldn't object to something that catches shadowings of parameters
> or local variables, but I think the flag as defined is not very
> useful.

On closer examination, that seems like a prescient comment. There are
about 1100 distinct warnings enabled by this flag. Of these, about 900
are caused by shadowed names from system headers: for example, using
'index' or 'shutdown' as the name of a function parameter. Note that a
single instance of shadowing in a header file generates warnings every
time that header file is included, so the number of actual places that
need to be changed should be smaller than 1100.

There are about 200 warnings that are not shadows of something from
the system headers. A fair number of these are also not very useful
(e.g. every variable named "length" triggers a warning, since that
shadows a function defined in pg_list.h), but there are also a fair
number of legitimately-shadowed local variables.

I think this leaves us with three options:

(1) Do nothing

(2) Enable -Wshadow for GCC, fix all the instances of the warning in
the source tree.

(3) Manually scan through the list of warnings and just submit
patches for the legitimate instances of shadowing.

The problem with #2 is the large number of warnings induced by system
headers: other platforms / standard libraries may well cause
additional instances of shadowing, so it might take a little while to
track down all the spurious warnings. On the other hand, it would be
nice if we could just turn this on and then forget about it.

Any comments?

-Neil