Start breaking -Wunreachable-code up into different diagnostic groups.

Recent work on -Wunreachable-code has focused on suppressing uninteresting
unreachable code that center around "configuration values", but
there are still some set of cases that are sometimes interesting
or uninteresting depending on the codebase.  For example, a dead
"break" statement may not be interesting for a particular codebase,
potentially because it is auto-generated or simply because code
is written defensively.

To address these workflow differences, -Wunreachable-code is now
broken into several diagnostic groups:

-Wunreachable-code: intended to be a reasonable "default" for
most users.

and then other groups that turn on more aggressive checking:

-Wunreachable-code-break: warn about dead break statements

-Wunreachable-code-trivial-return: warn about dead return statements
that return "trivial" values (e.g., return 0).  Other return
statements that return non-trivial values are still reported
under -Wunreachable-code (this is an area subject to more refinement).

-Wunreachable-code-aggressive: supergroup that enables all these
groups.

The goal is to eventually make -Wunreachable-code good enough to
either be in -Wall or on-by-default, thus finessing these warnings
into different groups helps achieve maximum signal for more users.

TODO: the tests need to be updated to reflect this extra control
via diagnostic flags.

llvm-svn: 203994

1 files changed, 40 insertions, 22 deletions

diff --git a/clang/lib/Analysis/ReachableCode.cpp b/clang/lib/Analysis/ReachableCode.cpp
index 994f9d2a12f..09b0efd0e68 100644
--- a/clang/lib/Analysis/ReachableCode.cpp
+++ b/clang/lib/Analysis/ReachableCode.cpp
@@ -59,9 +59,14 @@ static bool bodyEndsWithNoReturn(const CFGBlock::AdjacentBlock &AB) {
   return bodyEndsWithNoReturn(Pred);
 }
 
-static bool isBreakPrecededByNoReturn(const CFGBlock *B,
-                                      const Stmt *S) {
-  if (!isa<BreakStmt>(S) || B->pred_empty())
+static bool isBreakPrecededByNoReturn(const CFGBlock *B, const Stmt *S,
+                                      reachable_code::UnreachableKind &UK) {
+  if (!isa<BreakStmt>(S))
+    return false;
+
+  UK = reachable_code::UK_Break;
+
+  if (B->pred_empty())
     return false;
 
   assert(B->empty());
@@ -131,23 +136,17 @@ static bool isTrivialExpression(const Expr *Ex) {
          isEnumConstant(Ex);
 }
 
-static bool isTrivialReturnOrDoWhile(const CFGBlock *B, const Stmt *S) {
-  const Expr *Ex = dyn_cast<Expr>(S);
-
-  if (Ex && !isTrivialExpression(Ex))
-    return false;
-
+static bool isTrivialReturnOrDoWhile(const CFGBlock *B, const Stmt *S,
+                                     reachable_code::UnreachableKind &UK) {
   // Check if the block ends with a do...while() and see if 'S' is the
   // condition.
   if (const Stmt *Term = B->getTerminator()) {
-    if (const DoStmt *DS = dyn_cast<DoStmt>(Term))
-      if (DS->getCond() == S)
-        return true;
+    if (const DoStmt *DS = dyn_cast<DoStmt>(Term)) {
+      const Expr *Cond = DS->getCond();
+      return Cond == S && isTrivialExpression(Cond);
+    }
   }
 
-  if (B->pred_size() != 1)
-    return false;
-
   // 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.
@@ -155,17 +154,33 @@ static bool isTrivialReturnOrDoWhile(const CFGBlock *B, const Stmt *S) {
        I != E; ++I) {
     if (Optional<CFGStmt> CS = I->getAs<CFGStmt>()) {
       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)
+        if (RS == S) {
           LookAtBody = true;
+          UK = reachable_code::UK_TrivialReturn;
+        }
         else {
           const Expr *RE = RS->getRetValue();
-          if (RE && stripExprSugar(RE->IgnoreParenCasts()) == Ex)
-            LookAtBody = true;
+          if (RE) {
+            RE = stripExprSugar(RE->IgnoreParenCasts());
+            if (RE == S) {
+              UK = reachable_code::UK_TrivialReturn;
+              LookAtBody = isTrivialExpression(RE);
+            }
+          }
         }
 
-        if (LookAtBody)
+        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());
+        }
       }
       break;
     }
@@ -588,19 +603,22 @@ static SourceLocation GetUnreachableLoc(const Stmt *S,
 void DeadCodeScan::reportDeadCode(const CFGBlock *B,
                                   const Stmt *S,
                                   clang::reachable_code::Callback &CB) {
+  // 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))
+  if (isBreakPrecededByNoReturn(B, S, UK))
     return;
 
   // Suppress trivial 'return' statements that are dead.
-  if (isTrivialReturnOrDoWhile(B, S))
+  if (UK == reachable_code::UK_Other && isTrivialReturnOrDoWhile(B, S, UK))
     return;
 
   SourceRange R1, R2;
   SourceLocation Loc = GetUnreachableLoc(S, R1, R2);
-  CB.HandleUnreachable(Loc, R1, R2);
+  CB.HandleUnreachable(UK, Loc, R1, R2);
 }
 
 //===----------------------------------------------------------------------===//