Re: [PATCH 0/8] Add -Wshadow=local

[Simon Marchi <simon dot marchi at ericsson dot com>]

On 2018-09-23 12:08 AM, Tom Tromey wrote:
> This series enables -Wshadow=local for gdb.
> 
> I think that shadowing is mildly confusing, and I normally try to
> avoid it.  However, for a long time, it wasn't feasible to enable any
> shadowing warnings, because gcc would warn about shadowing that was
> "uninteresting" -- the classic example being the use of a local
> variable named "index".
> 
> In this series I chose to use -Wshadow=local, to forbid shadowing
> within a function.  -Wshadow=compatible-local is another choice, but I
> think that the stated semantics:
> 
>     In C++, type compatibility here means the type of the shadowing
>     variable can be converted to that of the shadowed variable.
> 
> ... is maybe insufficient.  For example a situation where the two
> different types are both convertible to a third type could be enough
> to cause a bug.  That's a bit of a reach maybe, but it seemed to me
> that it's not such a big deal, and certainly always safer, to just
> disallow shadowing entirely.
> 
> Adding this warning found a few latent bugs in gdb.
> 
> Regression tested by the buildbot.  Thanks, Sergio, for your work
> running this -- it is a great service.
> 
> Let me know what you think.  I feel that a change like this should
> involve some feedback.

I think that's a good idea, I think variable shadowing is more likely
done by mistake than anything else.

Simon