[cfe-commits] r157666 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp test/Sema/switch.c test/SemaCXX/warn-unique-enum.cpp

[Matthieu Monrocq]

On Wed, May 30, 2012 at 8:23 PM, Jordan Rose <jordan_rose at apple.com> wrote:

>
> On May 30, 2012, at 10:43 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Tue, May 29, 2012 at 6:01 PM, Richard Trieu <rtrieu at google.com> wrote:
>
>> Author: rtrieu
>> Date: Tue May 29 20:01:11 2012
>> New Revision: 157666
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=157666&view=rev
>> Log:
>> Add new -Wunique-enum which will warn on enums which all elements have the
>> same value and were initialized with literals.
>
>
> This currently produces a false positive on some anonymous-enum'd
> constants in Diagnostic.h:635:
>
>   *enum* {
>     */// MaxArguments - The maximum number of arguments we can hold. We currently*    */// only support up to 10 arguments (%0-%9).  A single diagnostic with more*    */// than that almost certainly has to be simplified anyway.*    MaxArguments = 10,
>
>     */// MaxRanges - The maximum number of ranges we can hold.*    MaxRanges = 10,
>
>     */// MaxFixItHints - The maximum number of ranges we can hold.*    MaxFixItHints = 10
>   };
> Perhaps we should ignore this warning if the enum is unnamed? Or specifically if it's both unnamed and has no instances (so it could still fire on "enum { x, y = 0 } a, b;")
>
> Since this warning is on by default, this breaks a self-host Clang -Werror build.
>
> Mac OS X frameworks use anonymous enums for constants in order to have the
> store type be word-sized:
>
> enum {
>    NSCaseInsensitiveSearch <https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSString_Class/Reference/NSString.html#//apple_ref/doc/c_ref/NSCaseInsensitiveSearch> = 1,
>    NSLiteralSearch <https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSString_Class/Reference/NSString.html#//apple_ref/doc/c_ref/NSLiteralSearch> = 2,
>    NSBackwardsSearch <https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSString_Class/Reference/NSString.html#//apple_ref/doc/c_ref/NSBackwardsSearch> = 4,
>    NSAnchoredSearch <https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSString_Class/Reference/NSString.html#//apple_ref/doc/c_ref/NSAnchoredSearch> = 8,
>    NSNumericSearch <https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSString_Class/Reference/NSString.html#//apple_ref/doc/c_ref/NSNumericSearch> = 64,
>    NSDiacriticInsensitiveSearch <https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSString_Class/Reference/NSString.html#//apple_ref/doc/c_ref/NSDiacriticInsensitiveSearch> = 128,
>    NSWidthInsensitiveSearch <https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSString_Class/Reference/NSString.html#//apple_ref/doc/c_ref/NSWidthInsensitiveSearch> = 256,
>    NSForcedOrderingSearch <https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSString_Class/Reference/NSString.html#//apple_ref/doc/c_ref/NSForcedOrderingSearch> = 512,
>    NSRegularExpressionSearch <https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSString_Class/Reference/NSString.html#//apple_ref/doc/c_ref/NSRegularExpressionSearch> = 1024
> };
>
> typedef NSUInteger NSStringCompareOptions;
>
> It's not likely that all options here will be the same, but then I would
> have said the same about enums in general. I don't have a strong opinion,
> but it's possible we should just change Diagnostic.h to use separate enums.
> (How common is it to group semi-related constants in an enum?)
>
> Jordan
>
>
I am not too C savvy, but I seem to recall that C is not as lenient as C++
and does not treat "int const" as being a compile-time constant, while
#define and enums really are; so I would expect to see more anonymous enums
in C (because #define is evil, isn't it ?)...

I agree it could be split in multiple enums, but having this warning on by
default in C might provoke more issues than it does on C++ code bases.

-- Matthieu.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120530/73966a06/attachment.html>