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

[Richard Trieu]

On Wed, Sep 12, 2012 at 2:59 PM, Chandler Carruth <chandlerc at google.com>wrote:

> 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?
>

This warning caught around 5 bugs.  Roughly twice that amount in false
positives needed to be silenced.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120912/7867b389/attachment.html>