[cfe-dev] -Wunique-enum; is it worth it?

[Chandler Carruth]

On Wed, Sep 12, 2012 at 2:30 PM, Richard Trieu <rtrieu at google.com> wrote:

> On Tue, Sep 11, 2012 at 3:26 PM, Ted Kremenek <kremenek at apple.com> wrote:
>
>> I've started receiving feedback about -Wunique-enum from some users and
>> I'm wondering if it is worth it.  Can we discuss what was the motivation
>> behind this warning and what kind of bugs it was meant to catch?
>>
>> One problem I see right now is that it doesn't handle enums that
>> represent bit masks, e.g.:
>>
>>   MaskA = 0x1,
>>   MaskB = 0x1
>>
>> and so on.  There are legitimate cases where declaring a bunch of
>> constants in an enum is okay, and it is fine for them to have the same
>> value.  Has this warning caught any real bugs?
>>
>> In general, I think it is fine to give warnings a new try and then
>> evaluate them.  I'm leaning towards removing this warning since I don't see
>> what value it brings, but I wanted to know what the motivation was behind
>> this warning before pressing on doing that.
>>
>
> The motivating issue is to catch issues that arise when enums have the
> same value.  Users may assume that enums are unique, leading to making
> unfounded assumptions about the enum.  One way that this may manifest is
> during testing of enum values:
>
> enum Foo { A = 0, B = 0 };
> Foo f = B;
> if (f == A)
>   ActOnA();  // Runs since A == B
> else if (f == B)
>   ActOnB();  // Never runs.
>
> There were several things added to reduce false positives.  Anonymous
> enums are ignored.  All the enum constants must be initialized with a
> literal.  This does not trigger if only some of the enum constants have the
> same value (i.e. if there's 3 constants with one value, and 2 of another).
>  A fix-it hint is also included to show to silence this warning.
>

Richard, I know you also caught a number of very real bugs with this
warning. Roughly how many? How many times did you have to silence the
warning?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120912/9a0e1b0c/attachment.html>