On Wed, May 16, 2012 at 11:42 AM, Douglas Gregor <[email protected]> wrote:
> > On May 15, 2012, at 4:10 PM, Richard Trieu <[email protected]> wrote: > > > Add -Wunique-enum which will warn on enums with at least 2 elements such > that all elements are the same value. This will catch enums such as: > > > > enum A { > > FIRST = 1, > > SECOND = 1 > > }; > > Why do we need this warning? Personally, I can't recall having seen this > be the source of any bugs, and it's scope is so narrow that it doesn't seem > worth adding. > Bugs of this type would be difficult to discover. In general, most testing is positive testing, such as: A test = FIRST: assert(test == FIRST); Less common is negative testing: A test = FIRST; assert(test != SECOND); Also, narrowing the scope of the warning to just enum values created from literal values should make this a more precise warning. > > As for the actual patch… > > +def warn_identical_enum_values : Warning< > + "all elements of %select{anonymous enum|%1}0 have value %2">, > + InGroup<DiagGroup<"unique-enum">>, DefaultIgnore; > + > > Default-ignore warnings are always a red flag. If this warning is > important, it should be on by default. If it's not important, we shouldn't > add it. > > I can certainly have this on by default if that's your preference. The change to also check for literal values should greatly reduce the number of false positives. > + // Keep track of whther all the elements have the same value. > + bool AllElementsEqual = true; > + llvm::APSInt LastVal; > > Typo "whther". > > + // Keep track of whether every enum element is the same value. > + if (AllElementsEqual && i > 0) { > + if (InitVal.getBitWidth() > LastVal.getBitWidth()) > + AllElementsEqual = InitVal == > LastVal.extend(InitVal.getBitWidth()); > + else if (InitVal.getBitWidth() < LastVal.getBitWidth()) > + AllElementsEqual = InitVal.extend(LastVal.getBitWidth()) == > LastVal; > + else > + AllElementsEqual = InitVal == LastVal; > + } > + LastVal = InitVal; > + > > If you did this in the loop below, where we adjust the initializer values > to all have the same size/# of bits, it would be easier. > > - Doug > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
