[cfe-dev] -Wunique-enum; is it worth it?
Chandler Carruth
chandlerc at google.com
Wed Sep 12 14:59:23 PDT 2012
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>
More information about the cfe-dev
mailing list