Improve -Wunreachable-code to provide a means to indicate code is intentionally marke...

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.

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

diff --git a/include/clang/Analysis/Analyses/ReachableCode.h b/include/clang/Analysis/Analyses/ReachableCode.h
index a328ea2c8f95bdcb0d8e35bd138d00c015c07063..90a6d014f58dffad44711656fb51056651688df6 100644 (file)
--- a/include/clang/Analysis/Analyses/ReachableCode.h
+++ b/include/clang/Analysis/Analyses/ReachableCode.h
@@ -50,7 +50,9 @@ class Callback {
 public:
   virtual ~Callback() {}
   virtual void HandleUnreachable(UnreachableKind UK,
-                                 SourceLocation L, SourceRange R1,
+                                 SourceLocation L,
+                                 SourceRange ConditionVal,
+                                 SourceRange R1,
                                  SourceRange R2) = 0;
 };
 

diff --git a/include/clang/Analysis/CFG.h b/include/clang/Analysis/CFG.h
index c05d440ef3cb0eea729da0fb8e79039cbd49f621..123d1443a2b09a63c0c51e95cadcf374fb16ed24 100644 (file)
--- a/include/clang/Analysis/CFG.h
+++ b/include/clang/Analysis/CFG.h
@@ -623,10 +623,10 @@ public:
   CFGTerminator getTerminator() { return Terminator; }
   const CFGTerminator getTerminator() const { return Terminator; }
 
-  Stmt *getTerminatorCondition();
+  Stmt *getTerminatorCondition(bool StripParens = true);
 
-  const Stmt *getTerminatorCondition() const {
-    return const_cast<CFGBlock*>(this)->getTerminatorCondition();
+  const Stmt *getTerminatorCondition(bool StripParens = true) const {
+    return const_cast<CFGBlock*>(this)->getTerminatorCondition(StripParens);
   }
 
   const Stmt *getLoopTarget() const { return LoopTarget; }

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index f7efbbd2efcbb60639803d15c69e93d50f110769..704bf1f1054e018deb9727a304832007fd614dfa 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -373,6 +373,8 @@ def warn_unreachable_return : Warning<
 def warn_unreachable_loop_increment : Warning<
   "loop will run at most once (loop increment never executed)">,
   InGroup<UnreachableCodeLoopIncrement>, DefaultIgnore;
+def note_unreachable_silence : Note<
+  "silence by adding parentheses to mark code as explicitly dead">;
 
 /// Built-in functions.
 def ext_implicit_lib_function_decl : ExtWarn<

diff --git a/lib/Analysis/CFG.cpp b/lib/Analysis/CFG.cpp
index ad5cf69cd70c60908098c9149f964c3f3ae94304..13ac92e86f216650a6cf4532e7a23be1156f0929 100644 (file)
--- a/lib/Analysis/CFG.cpp
+++ b/lib/Analysis/CFG.cpp
@@ -4112,7 +4112,7 @@ void CFGBlock::printTerminator(raw_ostream &OS,
   TPrinter.print(getTerminator());
 }
 
-Stmt *CFGBlock::getTerminatorCondition() {
+Stmt *CFGBlock::getTerminatorCondition(bool StripParens) {
   Stmt *Terminator = this->Terminator;
   if (!Terminator)
     return NULL;
@@ -4171,6 +4171,9 @@ Stmt *CFGBlock::getTerminatorCondition() {
       return Terminator;
   }
 
+  if (!StripParens)
+    return E;
+
   return E ? E->IgnoreParens() : NULL;
 }
 

diff --git a/lib/Analysis/ReachableCode.cpp b/lib/Analysis/ReachableCode.cpp
index 53e03de64ed482d88374fd894f1362908d9d40ed..7c8ea39dc56f61219cc7b93ce0fd6ed622c0d6d2 100644 (file)
--- a/lib/Analysis/ReachableCode.cpp
+++ b/lib/Analysis/ReachableCode.cpp
@@ -133,10 +133,17 @@ static bool isConfigurationValue(const ValueDecl *D, Preprocessor &PP);
 /// 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 @@ static bool isConfigurationValue(const Stmt *S,
     }
     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 @@ static bool isConfigurationValue(const Stmt *S,
       // 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 @@ static bool shouldTreatSuccessorsAsReachable(const CFGBlock *B,
     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 @@ namespace {
     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 @@ void DeadCodeScan::reportDeadCode(const CFGBlock *B,
     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 @@ void DeadCodeScan::reportDeadCode(const CFGBlock *B,
       }
 
       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);
 }
 
 //===----------------------------------------------------------------------===//

diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp
index efaf966562cf1ce3bf81a200c880ced40b3594f0..75d7060e2d8e1b982d21ec85368f0fd97a2a0eaf 100644 (file)
--- a/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/lib/Sema/AnalysisBasedWarnings.cpp
@@ -66,7 +66,9 @@ namespace {
     UnreachableCodeHandler(Sema &s) : S(s) {}
 
     void HandleUnreachable(reachable_code::UnreachableKind UK,
-                           SourceLocation L, SourceRange R1,
+                           SourceLocation L,
+                           SourceRange SilenceableCondVal,
+                           SourceRange R1,
                            SourceRange R2) override {
       unsigned diag = diag::warn_unreachable;
       switch (UK) {
@@ -84,6 +86,17 @@ namespace {
       }
 
       S.Diag(L, diag) << R1 << R2;
+      
+      SourceLocation Open = SilenceableCondVal.getBegin();
+      if (Open.isValid()) {
+        SourceLocation Close = SilenceableCondVal.getEnd();        
+        Close = S.PP.getLocForEndOfToken(Close);
+        if (Close.isValid()) {
+          S.Diag(Open, diag::note_unreachable_silence)
+            << FixItHint::CreateInsertion(Open, "/* DISABLES CODE */ (")
+            << FixItHint::CreateInsertion(Close, ")");
+        }
+      }
     }
   };
 }

diff --git a/test/Sema/warn-unreachable.c b/test/Sema/warn-unreachable.c
index 9cd87a7343b0072dc4d42676becf3017874defe0..0d22965957372565fa6befd101631c7d1da5696c 100644 (file)
--- a/test/Sema/warn-unreachable.c
+++ b/test/Sema/warn-unreachable.c
@@ -257,7 +257,7 @@ int test_config_constant(int x) {
     calledFun(); // no-warning
     return 1;
   }
-  if (!1) {
+  if (!1) { // expected-note {{silence by adding parentheses to mark code as explicitly dead}}
     calledFun(); // expected-warning {{will never be executed}}
     return 1;
   }
@@ -268,7 +268,7 @@ int test_config_constant(int x) {
   if (x > 10)
     return CONFIG_CONSTANT ? calledFun() : calledFun(); // no-warning
   else
-    return 1 ?
+    return 1 ? // expected-note {{silence by adding parentheses to mark code as explicitly dead}}
       calledFun() :
       calledFun(); // expected-warning {{will never be executed}}
 }
@@ -365,3 +365,34 @@ int test_break_preceded_by_noreturn_SUPPRESSED(int i) {
 }
 
 #pragma clang diagnostic pop
+
+// Test "silencing" with parentheses.
+void test_with_paren_silencing(int x) {
+  if (0) calledFun(); // expected-warning {{will never be executed}} expected-note {{silence by adding parentheses to mark code as explicitly dead}}
+  if ((0)) calledFun(); // no-warning
+
+  if (1) // expected-note {{silence by adding parentheses to mark code as explicitly dead}}
+    calledFun();
+  else
+    calledFun(); // expected-warning {{will never be executed}}
+
+  if ((1))
+    calledFun();
+  else
+    calledFun(); // no-warning
+  
+  if (!1) // expected-note {{silence by adding parentheses to mark code as explicitly dead}}
+    calledFun(); // expected-warning {{code will never be executed}}
+  else
+    calledFun();
+  
+  if ((!1))
+    calledFun(); // no-warning
+  else
+    calledFun();
+  
+  if (!(1))
+    calledFun(); // no-warning
+  else
+    calledFun();
+}