Fix unreachable code false positive, vardecl in switch

[danielmarjamaki]

Summary

This patch fixes false positives for vardecls that are technically unreachable but they are needed.

switch (x) {
  int a;  // <- This is unreachable but needed
case 1:
  a = ...

For this code there will be Wunused-variable:

if (1+2==45) {
  int x;
}

For this code there is 'unreachable code' warning on the 'if (1)':

if (1+2==45) {
  int x;
  if (1) {}
}

[danielmarjamaki]

Minor updates of testcase

[xazax.hun]

lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp

195

Maybe I would prefer something like !isa<DeclStmt> which is a less intrusive change.

[danielmarjamaki]

Use !isa<DeclStmt>. Suggestion by Gabor.

[danielmarjamaki]

lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp

195

yes I agree.

[xazax.hun]

LGTM!

[dcoughlin]

Sorry, missed this patch.

I think it would good to add a test to make sure we do warn when the var decl has an initializer, since that will not be executed.

void varDecl(int X) {
  switch (X) {
    int A = 12; // We do want a warning here, since the variable will be uninitialized in C (This is not allowed in C++).
  case 1:
    ...
    break;
  }
}

[danielmarjamaki]

In D24905#557573, @dcoughlin wrote:

Sorry, missed this patch.

I think it would good to add a test to make sure we do warn when the var decl has an initializer, since that will not be executed.

void varDecl(int X) {
  switch (X) {
    int A = 12; // We do want a warning here, since the variable will be uninitialized in C (This is not allowed in C++).
  case 1:
    ...
    break;
  }
}

I added such test case with 283096.

lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp

if (!reachable.count((*I)->getBlockID())) {
         FindUnreachableEntryPoints(*I, reachable, visited);
     }
   }
 }
 
 // Find the Stmt* in a CFGBlock for reporting a warning
 const Stmt *UnreachableCodeChecker::getUnreachableStmt(const CFGBlock *CB) {
   for (CFGBlock::const_iterator I = CB->begin(), E = CB->end(); I != E; ++I) {
-    if (Optional<CFGStmt> S = I->getAs<CFGStmt>())
+    if (Optional<CFGStmt> S = I->getAs<CFGStmt>()) {
+      if (isa<Expr>(S->getStmt()))
         return S->getStmt();
     }
+  }
   if (const Stmt *S = CB->getTerminator())
     return S;
   else
     return nullptr;
 }
 
 // Determines if the path to this CFGBlock contained an element that infers this
 // block is a false positive. We assume that FindUnreachableEntryPoints has

test/Analysis/unreachable-code-path.c

void testInlined() {
   int cond = coin();
   if (!cond) {
     inlined(0);
     if (cond) {
       foo(5); // expected-warning {{never executed}}
     }
   }
 }
+
+// don't warn about unreachable vardecl
+void varDecl() {
+  switch (x) {
+    int a; // No warning here.
+  case 1:
+    a=1;
+    break;
+  case 2:
+    a=2;
+    break;
+  }
+}