On Fri, Sep 20, 2019, 2:21 PM Bernd Edlinger <bernd.edlin...@hotmail.de> wrote:
> 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. > That would make sense to me. >