[cfe-commits] r148640 - in /cfe/trunk: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaStmt.cpp test/Sema/switch.c test/Sema/warn-unreachable.c test/SemaCXX/gnu-case-ranges.cpp test/SemaCXX/return-n

[David Blaikie]

On Mon, Jan 23, 2012 at 7:21 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Sat, January 21, 2012 18:12, David Blaikie wrote:
>> Author: dblaikie
>> Date: Sat Jan 21 12:12:07 2012
>> New Revision: 148640
>>
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=148640&view=rev
>> Log:
>> Add -Wswitch-enum-redundant-default.
>>
>>
>> This warning acts as the complement to the main -Wswitch-enum warning (which
>> warns whenever a switch over enum without a default doesn't cover all values
>> of the enum) & has been an an-doc coding convention in LLVM and Clang in my
>> experience. The purpose is to ensure there's never a "dead" default in a
>> switch-over-enum because this would hide future -Wswitch-enum errors.
>>
>> The name warning has a separate flag name so it can be disabled but it's
>> grouped under -Wswitch-enum & is on-by-default because of this.
>>
>> The existing violations of this rule in test cases have had the warning
>> disabled & I've added a specific test for the new behavior (many negative
>> cases already exist in the same test file - and none regressed - so I didn't
>> add more).
>>
>> Reviewed by Ted Kremenek (
>> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20120116/051690.ht
>> ml )
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/lib/Sema/SemaStmt.cpp cfe/trunk/test/Sema/switch.c
>> cfe/trunk/test/Sema/warn-unreachable.c
>> cfe/trunk/test/SemaCXX/gnu-case-ranges.cpp
>> cfe/trunk/test/SemaCXX/return-noreturn.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticS
>> emaKinds.td?rev=148640&r1=148639&r2=148640&view=diff
>> =============================================================================
>> =
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sat Jan 21 12:12:07
>> 2012
>> @@ -4946,19 +4946,22 @@
>> def warn_missing_case_for_condition : Warning<"no case matching constant switch
>> condition '%0'">; def warn_missing_case1 : Warning<"enumeration value %0 not
>> handled in switch">, -  InGroup<DiagGroup<"switch-enum"> >;
>> +  InGroup<SwitchEnum>;
>> def warn_missing_case2 : Warning< "enumeration values %0 and %1 not handled in
>> switch">, -  InGroup<DiagGroup<"switch-enum"> >;
>> +  InGroup<SwitchEnum>;
>> def warn_missing_case3 : Warning< "enumeration values %0, %1, and %2 not
>> handled in switch">, -  InGroup<DiagGroup<"switch-enum"> >;
>> +  InGroup<SwitchEnum>;
>> def warn_missing_cases : Warning< "%0 enumeration values not handled in switch:
>> %1, %2, %3...">,
>> -  InGroup<DiagGroup<"switch-enum"> >;
>> +  InGroup<SwitchEnum>;
>> +def warn_unreachable_default : Warning<
>> +  "default is unreachable as all enumeration values are accounted for">,
>
> This diagnostic is not technically correct; an expression of enumeration type
> can have a value which is not a named member of that enumeration. How about
> something like 'default label in switch which covers all enumeration values'?
> For the same reason, I'm slightly hesitant about the name of this warning
> flag.

Agreed - committed as r148783. I also renamed the option to
-Wcovered-switch-default (it doesn't have the nice style of having its
common group/parent (-Wswitch) as a prefix, but hopefully this is OK)
- open to other suggestions, though.

- David