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.html )

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@148640 91177308-0d34-0410-b5e6-96231b3b80d8

7 files changed, 31 insertions, 12 deletions

diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index 9c03580413..ca0ce99c2c 100644
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/include/clang/Basic/DiagnosticGroups.td
@@ -178,7 +178,8 @@ def InvalidOffsetof : DiagGroup<"invalid-offsetof">;
 def : DiagGroup<"strict-prototypes">;
 def StrictSelector : DiagGroup<"strict-selector-match">;
 def MethodDuplicate : DiagGroup<"duplicate-method-match">;
-def SwitchEnum     : DiagGroup<"switch-enum">;
+def SwitchEnumRedundantDefault : DiagGroup<"switch-enum-redundant-default">;
+def SwitchEnum     : DiagGroup<"switch-enum", [SwitchEnumRedundantDefault]>;
 def Switch         : DiagGroup<"switch", [SwitchEnum]>;
 def Trigraphs      : DiagGroup<"trigraphs">;
 
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index f685c9f2b7..3ffd580b83 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4946,19 +4946,22 @@ def warn_case_empty_range : Warning<"empty case range specified">;
 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">,
+  InGroup<SwitchEnumRedundantDefault>;
 
 def warn_not_in_enum : Warning<"case value not in enumerated type %0">,
-  InGroup<DiagGroup<"switch-enum"> >; 
+  InGroup<SwitchEnum>; 
 def err_typecheck_statement_requires_scalar : Error<
   "statement requires expression of scalar type (%0 invalid)">;
 def err_typecheck_statement_requires_integer : Error<
diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
index a53e397243..0be3e43b5a 100644
--- a/lib/Sema/SemaStmt.cpp
+++ b/lib/Sema/SemaStmt.cpp
@@ -924,11 +924,17 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
 
         if (RI == CaseRanges.end() || EI->first < RI->first) {
           hasCasesNotInSwitch = true;
-          if (!TheDefaultStmt)
-            UnhandledNames.push_back(EI->second->getDeclName());
+          UnhandledNames.push_back(EI->second->getDeclName());
         }
       }
 
+      if (TheDefaultStmt) {
+        if (UnhandledNames.size() == 0)
+          Diag(TheDefaultStmt->getDefaultLoc(), diag::warn_unreachable_default);
+        else
+          UnhandledNames.clear();
+      }
+
       // Produce a nice diagnostic if multiple values aren't handled.
       switch (UnhandledNames.size()) {
       case 0: break;
diff --git a/test/Sema/switch.c b/test/Sema/switch.c
index ef2e4c5d9b..63ffed18e3 100644
--- a/test/Sema/switch.c
+++ b/test/Sema/switch.c
@@ -288,3 +288,12 @@ void test17(int x) {
   case 0: return;
   }
 }
+
+int test18() {
+  enum { A, B } a;
+  switch (a) {
+  case A: return 0;
+  case B: return 1;
+  default: return 2; // expected-warning {{default is unreachable as all enumeration values are accounted for}}
+  }
+}
diff --git a/test/Sema/warn-unreachable.c b/test/Sema/warn-unreachable.c
index 3ad53c707b..46a680f0ef 100644
--- a/test/Sema/warn-unreachable.c
+++ b/test/Sema/warn-unreachable.c
@@ -1,4 +1,4 @@
-// RUN: %clang %s -fsyntax-only -Xclang -verify -fblocks -Wunreachable-code -Wno-unused-value
+// RUN: %clang %s -fsyntax-only -Xclang -verify -fblocks -Wunreachable-code -Wno-unused-value -Wno-switch-enum-redundant-default
 
 int halt() __attribute__((noreturn));
 int live();
diff --git a/test/SemaCXX/gnu-case-ranges.cpp b/test/SemaCXX/gnu-case-ranges.cpp
index c1c18a8948..94100d2e83 100644
--- a/test/SemaCXX/gnu-case-ranges.cpp
+++ b/test/SemaCXX/gnu-case-ranges.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify -Wno-switch-enum-redundant-default %s
 
 enum E {
     one,
diff --git a/test/SemaCXX/return-noreturn.cpp b/test/SemaCXX/return-noreturn.cpp
index e06ba403ef..4b1f82dc6a 100644
--- a/test/SemaCXX/return-noreturn.cpp
+++ b/test/SemaCXX/return-noreturn.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 %s -fsyntax-only -verify -Wreturn-type -Wmissing-noreturn -Wno-unreachable-code
-// RUN: %clang_cc1 %s -fsyntax-only -std=c++11 -verify -Wreturn-type -Wmissing-noreturn -Wno-unreachable-code
+// RUN: %clang_cc1 %s -fsyntax-only -verify -Wreturn-type -Wmissing-noreturn -Wno-unreachable-code -Wno-switch-enum-redundant-default
+// RUN: %clang_cc1 %s -fsyntax-only -std=c++11 -verify -Wreturn-type -Wmissing-noreturn -Wno-unreachable-code -Wno-switch-enum-redundant-default
 
 // A destructor may be marked noreturn and should still influence the CFG.
 void pr6884_abort() __attribute__((noreturn));