[PATCH] D52400: Improve -Wshadow warnings with enumerators

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D52400#1267499, @sberg wrote:

> In https://reviews.llvm.org/D52400#1266341, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D52400#1266307, @sberg wrote:
> >
> > >
> >
>
>
> [...]
>
> > Then again, this is a case where you don't get any error but you do get a silent behavioral ambiguity without the current enumerator shadow diagnostic:
> > 
> >   struct S1;
> >   struct S2;
> >   struct S3 {
> >     void S1();
> >     enum { S2 };
> >   
> >     void f(decltype(S2) s);
> >   };
> > 
> > 
> > So there are cases where this behavior can be somewhat useful.
>
> but decltype(S2) is a syntax error when S2 names a struct type, no?


Ugh, you're absolutely right.

>>> (ran into such a new -Wshadow while compiling LibreOffice)
>> 
>> Was it a frequent/annoying occurrence?
> 
> there was less than 20 cases overall. about half were "good" warnings about clashing enumerators from different non-scoped enums. the others were "unhelpful" ones about clashes with class names, two of them in stable interface code that cant be changed (so would need ugly #pragma clang warning decorations), one of them even about entities in unrelated include files, so whether a warning is emitted depends on the order in which the files happen to get included in a TU

Thanks, that's good information!

> (and in any case, "declaration shadows a variable" sounds wrong when the shadowed entity is a class type. thats why I thought something is not quite right with this new code yet)

Definitely agreed. I think I'm convinced that this should be silenced when the conflict is with a type rather than a value. I'll try to look into this next week when I'm back from WG14 meetings.


https://reviews.llvm.org/D52400