This is an archive of the discontinued LLVM Phabricator instance.

Fix unreachable code false positive, vardecl in switch
ClosedPublic

Authored by danielmarjamaki on Sep 26 2016, 1:04 AM.

Details

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) {}
}

Diff Detail

Event Timeline

danielmarjamaki retitled this revision from to Fix unreachable code false positive, vardecl in switch.
danielmarjamaki updated this object.
danielmarjamaki set the repository for this revision to rL LLVM.

Minor updates of testcase

xazax.hun added inline comments.Sep 27 2016, 4:36 AM
lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
195

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

danielmarjamaki removed rL LLVM as the repository for this revision.

Use !isa<DeclStmt>. Suggestion by Gabor.

danielmarjamaki marked an inline comment as done.Sep 28 2016, 1:24 AM
danielmarjamaki added inline comments.
lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
195

yes I agree.

danielmarjamaki marked an inline comment as done.Sep 28 2016, 1:25 AM
xazax.hun accepted this revision.Sep 28 2016, 2:11 AM
xazax.hun edited edge metadata.

LGTM!

This revision is now accepted and ready to land.Sep 28 2016, 2:11 AM
This revision was automatically updated to reflect the committed changes.

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;
  }
}

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.