Further refine -Wunreachable-code groups so that -Wno-unreachable-code-break doesn't turn off all unreachable code warnings.

Also relax unreachable 'break' and 'return' to not check for being
preceded by a call to 'noreturn'.  That turns out to not be so
interesting in practice.

llvm-svn: 204000

clang/include/clang/Basic/DiagnosticGroups.td

@@ -431,13 +431,12 @@ def DuplicateArgDecl : DiagGroup<"duplicate-method-arg">;
 //  under separate flags.
 //
 def UnreachableCode : DiagGroup<"unreachable-code">;
-def UnreachableCodeBreak : DiagGroup<"unreachable-code-break",
-                            [UnreachableCode]>;
-def UnreachableCodeTrivialReturn : DiagGroup<"unreachable-code-trivial-return",
-                                    [UnreachableCode]>;
+def UnreachableCodeBreak : DiagGroup<"unreachable-code-break">;
+def UnreachableCodeReturn : DiagGroup<"unreachable-code-return">;
 def UnreachableCodeAggressive : DiagGroup<"unreachable-code-aggressive",
-                                    [UnreachableCodeBreak,
-                                     UnreachableCodeTrivialReturn]>;
+                                    [UnreachableCode,
+                                     UnreachableCodeBreak,
+                                     UnreachableCodeReturn]>;
 
 // Aggregation warning settings.
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -367,9 +367,9 @@ def warn_unreachable : Warning<
 def warn_unreachable_break : Warning<
   "'break' will never be executed">,
   InGroup<UnreachableCodeBreak>, DefaultIgnore;
-def warn_unreachable_trivial_return : Warning<
+def warn_unreachable_return : Warning<
   "'return' will never be executed">,
-  InGroup<UnreachableCodeTrivialReturn>, DefaultIgnore;
+  InGroup<UnreachableCodeReturn>, DefaultIgnore;
 
 /// Built-in functions.
 def ext_implicit_lib_function_decl : ExtWarn<

clang/lib/Analysis/ReachableCode.cpp

@@ -136,8 +136,7 @@ static bool isTrivialExpression(const Expr *Ex) {
          isEnumConstant(Ex);
 }
 
-static bool isTrivialReturnOrDoWhile(const CFGBlock *B, const Stmt *S,
-                                     reachable_code::UnreachableKind &UK) {
+static bool isTrivialDoWhile(const CFGBlock *B, const Stmt *S) {
   // Check if the block ends with a do...while() and see if 'S' is the
   // condition.
   if (const Stmt *Term = B->getTerminator()) {
@@ -146,7 +145,10 @@ static bool isTrivialReturnOrDoWhile(const CFGBlock *B, const Stmt *S,
       return Cond == S && isTrivialExpression(Cond);
     }
   }
+  return false;
+}
 
+static bool isTrivialReturn(const CFGBlock *B, const Stmt *S) {
   // Look to see if the block ends with a 'return', and see if 'S'
   // is a substatement.  The 'return' may not be the last element in
   // the block because of destructors.
@@ -156,30 +158,11 @@ static bool isTrivialReturnOrDoWhile(const CFGBlock *B, const Stmt *S,
       if (const ReturnStmt *RS = dyn_cast<ReturnStmt>(CS->getStmt())) {
         // Determine if we need to lock at the body of the block
         // before the dead return.
-        bool LookAtBody = false;
-        if (RS == S) {
-          LookAtBody = true;
-          UK = reachable_code::UK_TrivialReturn;
-        }
-        else {
-          const Expr *RE = RS->getRetValue();
-          if (RE) {
-            RE = stripExprSugar(RE->IgnoreParenCasts());
-            if (RE == S) {
-              UK = reachable_code::UK_TrivialReturn;
-              LookAtBody = isTrivialExpression(RE);
-            }
-          }
-        }
-
-        if (LookAtBody) {
-          // More than one predecessor?  Restrict the heuristic
-          // to looking at return statements directly dominated
-          // by a noreturn call.
-          if (B->pred_size() != 1)
-            return false;
-
-          return bodyEndsWithNoReturn(*B->pred_begin());
+        if (RS == S)
+          return true;
+        if (const Expr *RE = RS->getRetValue()) {
+          RE = stripExprSugar(RE->IgnoreParenCasts());
+          return RE == S && isTrivialExpression(RE);
         }
       }
       break;
@@ -606,15 +589,25 @@ void DeadCodeScan::reportDeadCode(const CFGBlock *B,
   // The kind of unreachable code found.
   reachable_code::UnreachableKind UK = reachable_code::UK_Other;
 
-  // Suppress idiomatic cases of calling a noreturn function just
-  // before executing a 'break'.  If there is other code after the 'break'
-  // in the block then don't suppress the warning.
-  if (isBreakPrecededByNoReturn(B, S, UK))
-    return;
+  do {
+    // Suppress idiomatic cases of calling a noreturn function just
+    // before executing a 'break'.  If there is other code after the 'break'
+    // in the block then don't suppress the warning.
+    if (isa<BreakStmt>(S)) {
+      UK = reachable_code::UK_Break;
+      break;
+    }
 
-  // Suppress trivial 'return' statements that are dead.
-  if (UK == reachable_code::UK_Other && isTrivialReturnOrDoWhile(B, S, UK))
-    return;
+    if (isTrivialDoWhile(B, S))
+      return;
+
+    // Suppress trivial 'return' statements that are dead.
+    if (isTrivialReturn(B, S)) {
+      UK = reachable_code::UK_TrivialReturn;
+      break;
+    }
+
+  } while(false);
 
   SourceRange R1, R2;
   SourceLocation Loc = GetUnreachableLoc(S, R1, R2);

clang/lib/Sema/AnalysisBasedWarnings.cpp

@@ -74,7 +74,7 @@ namespace {
           diag = diag::warn_unreachable_break;
           break;
         case reachable_code::UK_TrivialReturn:
-          diag = diag::warn_unreachable_trivial_return;
+          diag = diag::warn_unreachable_return;
           break;
         case reachable_code::UK_Other:
           break;
@@ -1665,6 +1665,11 @@ clang::sema::AnalysisBasedWarnings::Policy::Policy() {
   enableConsumedAnalysis = 0;
 }
 
+static unsigned isEnabled(DiagnosticsEngine &D, unsigned diag) {
+  return (unsigned) D.getDiagnosticLevel(diag, SourceLocation()) !=
+                    DiagnosticsEngine::Ignored;
+}
+
 clang::sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s)
   : S(s),
     NumFunctionsAnalyzed(0),
@@ -1676,16 +1681,20 @@ clang::sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s)
     MaxUninitAnalysisVariablesPerFunction(0),
     NumUninitAnalysisBlockVisits(0),
     MaxUninitAnalysisBlockVisitsPerFunction(0) {
+
+  using namespace diag;
   DiagnosticsEngine &D = S.getDiagnostics();
-  DefaultPolicy.enableCheckUnreachable = (unsigned)
-    (D.getDiagnosticLevel(diag::warn_unreachable, SourceLocation()) !=
-        DiagnosticsEngine::Ignored);
-  DefaultPolicy.enableThreadSafetyAnalysis = (unsigned)
-    (D.getDiagnosticLevel(diag::warn_double_lock, SourceLocation()) !=
-     DiagnosticsEngine::Ignored);
-  DefaultPolicy.enableConsumedAnalysis = (unsigned)
-    (D.getDiagnosticLevel(diag::warn_use_in_invalid_state, SourceLocation()) !=
-     DiagnosticsEngine::Ignored);
+
+  DefaultPolicy.enableCheckUnreachable =
+    isEnabled(D, warn_unreachable) ||
+    isEnabled(D, warn_unreachable_break) ||
+    isEnabled(D, warn_unreachable_return);
+
+  DefaultPolicy.enableThreadSafetyAnalysis =
+    isEnabled(D, warn_double_lock);
+
+  DefaultPolicy.enableConsumedAnalysis =
+    isEnabled(D, warn_use_in_invalid_state);
 }
 
 static void flushDiagnostics(Sema &S, sema::FunctionScopeInfo *fscope) {

clang/test/Sema/warn-unreachable.c

@@ -151,10 +151,10 @@ int test_break_preceded_by_noreturn(int i) {
   switch (i) {
     case 1:
       raze();
-      break; // no-warning
+      break; // expected-warning {{'break' will never be executed}}
     case 2:
       raze();
-      break; // no-warning
+      break; // expected-warning {{'break' will never be executed}}
       warn_here(); // expected-warning {{will never be executed}}
     case 3:
       return 1;
@@ -194,19 +194,17 @@ void unreachable_in_default(MyEnum e) {
 // Don't warn about trivial dead returns.
 int trivial_dead_return() {
   raze();
-  // Use the '()' to test that we unwrap such stuff
-  // when looking for dead code.
-  return ((0)); // no-warning
+  return ((0)); // expected-warning {{'return' will never be executed}}
 }
 
 void trivial_dead_return_void() {
   raze();
-  return; // no-warning
+  return; // expected-warning {{'return' will never be executed}}
 }
 
 MyEnum trival_dead_return_enum() {
   raze();
-  return Value1; // no-warning
+  return Value1; // expected-warning {{'return' will never be executed}}
 }
 
 MyEnum trivial_dead_return_enum_2(int x) {
@@ -222,12 +220,12 @@ MyEnum trivial_dead_return_enum_2(int x) {
 
 const char *trivial_dead_return_cstr() {
   raze();
-  return ""; // no-warning
+  return ""; // expected-warning {{return' will never be executed}}
 }
 
 char trivial_dead_return_char() {
   raze();
-  return ' '; // no-warning
+  return ' '; // expected-warning {{return' will never be executed}}
 }
 
 MyEnum nontrivial_dead_return_enum_2(int x) {
@@ -325,3 +323,45 @@ int test_do_while_nontrivial_cond(int x) {
   return x;
 }
 
+// Diagnostic control: -Wunreachable-code-return.
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunreachable-code-return"
+
+void trivial_dead_return_void_SUPPRESSED() {
+  raze();
+  return; // no-warning
+}
+
+MyEnum trival_dead_return_enum_SUPPRESSED() {
+  raze();
+  return Value1; // no-warning
+}
+
+#pragma clang diagnostic pop
+
+// Diagnostic control: -Wunreachable-code-break.
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunreachable-code-break"
+
+int test_break_preceded_by_noreturn_SUPPRESSED(int i) {
+  switch (i) {
+    case 1:
+      raze();
+      break; // no-warning
+    case 2:
+      raze();
+      break; // no-warning
+      warn_here(); // expected-warning {{will never be executed}}
+    case 3:
+      return 1;
+      break; // no-warning
+    default:
+      break;
+      break; // no-warning
+  }
+  return i;
+}
+
+#pragma clang diagnostic pop

clang/test/SemaCXX/warn-unreachable.cpp

@@ -142,7 +142,7 @@ typedef basic_string<char> string;
 
 std::string testStr() {
   raze();
-  return ""; // no-warning
+  return ""; // expected-warning {{'return' will never be executed}}
 }
 
 std::string testStrWarn(const char *s) {
@@ -152,7 +152,7 @@ std::string testStrWarn(const char *s) {
 
 bool testBool() {
   raze();
-  return true; // no-warning
+  return true; // expected-warning {{'return' will never be executed}}
 }
 
 static const bool ConditionVar = 1;