On 9/19/19 12:19 PM, Richard Biener wrote: > On Wed, Sep 18, 2019 at 3:09 PM Bernd Edlinger > <bernd.edlin...@hotmail.de> wrote: >> >> Hi, >> >> I'm currently trying to add -Wshadow=local to the gcc build rules. >> I started with -Wshadow, but gave up that idea immediately. >> >> As you could expect the current code base has plenty of shadowed >> local variables. Most are trivial to resolve, some are less trivial. >> I am not finished yet, but it is clear that it will be a rather big >> patch. >> >> I would like to ask you if you agree that would be a desirable step, >> in improving code quality in the gcc tree. > > I wonder if -Wshadow=compatible-local is easier to achieve? >
I tried that and it does not make a big difference, while it misses for instance: * gcc/c-family/c-ada-spec.c (dump_ada_macros) unsigned char *s, shadowed by const unsigned char *s. :-/ On the other hand, -Wshadow=global is a lot more difficult, because we have lots of globals, for instance: context.h: /* The global singleton context aka "g". (the name is chosen to be easy to type in a debugger). */ extern gcc::context *g; But unfortunately Wshadow=local does not find class members shadowed by local variable, which happens for instance in wide-int With -Wshadow, I had to change this in wide-int.h: Index: gcc/wide-int.h =================================================================== --- gcc/wide-int.h (revision 275545) +++ gcc/wide-int.h (working copy) @@ -648,9 +648,9 @@ namespace wi storage_ref () {} storage_ref (const HOST_WIDE_INT *, unsigned int, unsigned int); - const HOST_WIDE_INT *val; - unsigned int len; - unsigned int precision; + const HOST_WIDE_INT *m_val; + unsigned int m_len; + unsigned int m_precision; So this change was not necessary for -Wshadow=local, although I would think that, shadowing class members is not much different from shadowing a local scope. Not sure if shadowing class members should be part of -Wshadow=local instead of -Wshadow=global. Bernd.