commit ec3bbf4933d70fd1c907f883acd45da4ab1f10ee
author Ted Kremenek <kremenek@apple.com> Sat Mar 29 00:35:20 2014 +0000
committer Ted Kremenek <kremenek@apple.com> Sat Mar 29 00:35:20 2014 +0000
tree dcd8e6bb16ecfd1b1d0da0b1e308b6bf44956d63
parent fdf496cb4807647276ad43f9c4ae5aa30bad299c

Improve -Wunreachable-code to provide a means to indicate code is intentionally marked dead via if((0)).

Taking a hint from -Wparentheses, use an extra '()' as a sigil that
a dead condition is intentionally dead.  For example:

  if ((0)) { dead }

When this sigil is found, do not emit a dead code warning.  When the
analysis sees:

  if (0)

it suggests inserting '()' as a Fix-It.

llvm-svn: 205069

clang/lib/Analysis/ReachableCode.cpp

diff --git a/clang/lib/Analysis/ReachableCode.cpp b/clang/lib/Analysis/ReachableCode.cpp
index 53e03de..7c8ea39 100644
--- a/clang/lib/Analysis/ReachableCode.cpp
+++ b/clang/lib/Analysis/ReachableCode.cpp
@@ -133,10 +133,17 @@
 /// those blocks.
 static bool isConfigurationValue(const Stmt *S,
                                  Preprocessor &PP,
-                                 bool IncludeIntegers = true) {
+                                 SourceRange *SilenceableCondVal = nullptr,
+                                 bool IncludeIntegers = true,
+                                 bool WrappedInParens = false) {
   if (!S)
     return false;
 
+  // Special case looking for the sigil '()' around an integer literal.
+  if (const ParenExpr *PE = dyn_cast<ParenExpr>(S))
+    return isConfigurationValue(PE->getSubExpr(), PP, SilenceableCondVal, 
+                                IncludeIntegers, true);
+
   if (const Expr *Ex = dyn_cast<Expr>(S))
     S = Ex->IgnoreParenCasts();
 
@@ -148,9 +155,15 @@
     }
     case Stmt::DeclRefExprClass:
       return isConfigurationValue(cast<DeclRefExpr>(S)->getDecl(), PP);
-    case Stmt::IntegerLiteralClass:
-      return IncludeIntegers ? isExpandedFromConfigurationMacro(S, PP)
-                             : false;
+    case Stmt::IntegerLiteralClass: {
+      const IntegerLiteral *E = cast<IntegerLiteral>(S);
+      if (IncludeIntegers) {
+        if (SilenceableCondVal && !SilenceableCondVal->getBegin().isValid())
+          *SilenceableCondVal = E->getSourceRange();
+        return WrappedInParens || isExpandedFromConfigurationMacro(E, PP);
+      }
+      return false;
+    }
     case Stmt::MemberExprClass:
       return isConfigurationValue(cast<MemberExpr>(S)->getMemberDecl(), PP);
     case Stmt::ObjCBoolLiteralExprClass:
@@ -163,13 +176,18 @@
       // values if they are used in a logical or comparison operator
       // (not arithmetic).
       IncludeIntegers &= (B->isLogicalOp() || B->isComparisonOp());
-      return isConfigurationValue(B->getLHS(), PP, IncludeIntegers) ||
-             isConfigurationValue(B->getRHS(), PP, IncludeIntegers);
+      return isConfigurationValue(B->getLHS(), PP, SilenceableCondVal,
+                                  IncludeIntegers) ||
+             isConfigurationValue(B->getRHS(), PP, SilenceableCondVal,
+                                  IncludeIntegers);
     }
     case Stmt::UnaryOperatorClass: {
       const UnaryOperator *UO = cast<UnaryOperator>(S);
+      if (SilenceableCondVal) 
+        *SilenceableCondVal = UO->getSourceRange();      
       return UO->getOpcode() == UO_LNot &&
-             isConfigurationValue(UO->getSubExpr(), PP);
+             isConfigurationValue(UO->getSubExpr(), PP, SilenceableCondVal,
+                                  IncludeIntegers, WrappedInParens);
     }
     default:
       return false;
@@ -204,11 +222,13 @@
     if (isa<SwitchStmt>(Term))
       return true;
     // Specially handle '||' and '&&'.
-    if (isa<BinaryOperator>(Term))
+    if (isa<BinaryOperator>(Term)) {
       return isConfigurationValue(Term, PP);
+    }
   }
 
-  return isConfigurationValue(B->getTerminatorCondition(), PP);
+  const Stmt *Cond = B->getTerminatorCondition(/* stripParens */ false);
+  return isConfigurationValue(Cond, PP);
 }
 
 static unsigned scanFromBlock(const CFGBlock *Start,
@@ -314,9 +334,9 @@
     const Stmt *findDeadCode(const CFGBlock *Block);
 
     void reportDeadCode(const CFGBlock *B,
-    const Stmt *S,
-    clang::reachable_code::Callback &CB);
-    };
+                        const Stmt *S,
+                        clang::reachable_code::Callback &CB);
+  };
 }
 
 void DeadCodeScan::enqueue(const CFGBlock *block) {
@@ -529,6 +549,8 @@
     UK = reachable_code::UK_Return;
   }
 
+  SourceRange SilenceableCondVal;
+
   if (UK == reachable_code::UK_Other) {
     // Check if the dead code is part of the "loop target" of
     // a for/for-range loop.  This is the block that contains
@@ -544,14 +566,26 @@
       }
 
       CB.HandleUnreachable(reachable_code::UK_Loop_Increment,
-                           Loc, SourceRange(Loc, Loc), R2);
+                           Loc, SourceRange(), SourceRange(Loc, Loc), R2);
       return;
     }
+    
+    // Check if the dead block has a predecessor whose branch has
+    // a configuration value that *could* be modified to
+    // silence the warning.
+    CFGBlock::const_pred_iterator PI = B->pred_begin();
+    if (PI != B->pred_end()) {
+      if (const CFGBlock *PredBlock = PI->getPossiblyUnreachableBlock()) {
+        const Stmt *TermCond =
+            PredBlock->getTerminatorCondition(/* strip parens */ false);
+        isConfigurationValue(TermCond, PP, &SilenceableCondVal);
+      }
+    }
   }
 
   SourceRange R1, R2;
   SourceLocation Loc = GetUnreachableLoc(S, R1, R2);
-  CB.HandleUnreachable(UK, Loc, R1, R2);
+  CB.HandleUnreachable(UK, Loc, SilenceableCondVal, R1, R2);
 }
 
 //===----------------------------------------------------------------------===//