-Wimplicit-fallthrough: fixed two cases where "fallthrough annotation in unreachable code" was issued incorrectly.

[alexfh]

Summary

-Wimplicit-fallthrough: fixed two cases where "fallthrough annotation in unreachable code" was issued incorrectly:

1. In actual unreachable code, but not immediately on a fall-through execution

path "fallthrough annotation does not directly precede switch label" is better;

2. After default: in a switch with covered enum cases. Actually, these shouldn't

be treated as unreachable code for our purpose.

[alexfh]

Treat all cases as reachable (fixes a long known problem with switching on constants).

[rsmith]

Makes sense, LGTM

test/SemaCXX/switch-implicit-fallthrough.cpp

202–205

We can drop these comments now.

lib/Sema/AnalysisBasedWarnings.cpp

public:
     }
 
     void fillReachableBlocks(CFG *Cfg) {
       assert(ReachableBlocks.empty() && "ReachableBlocks already filled");
       std::deque<const CFGBlock *> BlockQueue;
 
       ReachableBlocks.insert(&Cfg->getEntry());
       BlockQueue.push_back(&Cfg->getEntry());
+      // Mark all default: blocks as reachable, even when isAllEnumCasesCovered.
+      // These blocks can contain fall-through annotations, and we don't want to
+      // issue a warn_fallthrough_attr_unreachable for them.
+      for (CFG::iterator I = Cfg->begin(), E = Cfg->end(); I != E; ++I) {
+        const CFGBlock *B = *I;
+        const Stmt *L = B->getLabel();
+        if (L && isa<DefaultStmt>(L) && ReachableBlocks.insert(B))
+          BlockQueue.push_back(B);
+      }
+
       while (!BlockQueue.empty()) {
         const CFGBlock *P = BlockQueue.front();
         BlockQueue.pop_front();
         for (CFGBlock::const_succ_iterator I = P->succ_begin(),
                                            E = P->succ_end();
              I != E; ++I) {
           if (*I && ReachableBlocks.insert(*I))
             BlockQueue.push_back(*I);
bool checkFallThroughIntoBlock(const CFGBlock &B, int &AnnotatedCnt) {
         if (SW && SW->getSubStmt() == B.getLabel() && P->begin() == P->end())
           continue; // Previous case label has no statements, good.
 
         const LabelStmt *L = dyn_cast_or_null<LabelStmt>(P->getLabel());
         if (L && L->getSubStmt() == B.getLabel() && P->begin() == P->end())
           continue; // Case label is preceded with a normal label, good.
 
         if (!ReachableBlocks.count(P)) {
-          for (CFGBlock::const_iterator ElIt = P->begin(), ElEnd = P->end();
-               ElIt != ElEnd; ++ElIt) {
-            if (const CFGStmt *CS = ElIt->getAs<CFGStmt>()){
+          for (CFGBlock::const_reverse_iterator ElemIt = P->rbegin(),
+                                                ElemEnd = P->rend();
+               ElemIt != ElemEnd; ++ElemIt) {
+            if (const CFGStmt *CS = ElemIt->getAs<CFGStmt>()) {
               if (const AttributedStmt *AS = asFallThroughAttr(CS->getStmt())) {
                 S.Diag(AS->getLocStart(),
                        diag::warn_fallthrough_attr_unreachable);
                 markFallthroughVisited(AS);
                 ++AnnotatedCnt;
+                break;
               }
               // Don't care about other unreachable statements.
             }
           }
           // If there are no unreachable statements, this may be a special
           // case in CFG:
           // case X: {
           //    A a;  // A has a destructor.

test/SemaCXX/switch-implicit-fallthrough.cpp

 
 void fallthrough_cfgblock_with_null_successor(int x) {
   (x && "") ? (void)(0) : (void)(1);
   switch (x) {}
 }
 
 int fallthrough_position(int n) {
   switch (n) {
+      [[clang::fallthrough]];  // expected-warning{{fallthrough annotation does not directly precede switch label}}
+      n += 300;
       [[clang::fallthrough]];  // expected-warning{{fallthrough annotation in unreachable code}}
     case 221:
-      [[clang::fallthrough]]; // expected-warning{{fallthrough annotation does not directly precede switch label}}
+      [[clang::fallthrough]];  // expected-warning{{fallthrough annotation does not directly precede switch label}}
       return 1;
       [[clang::fallthrough]];  // expected-warning{{fallthrough annotation in unreachable code}}
     case 222:
-      [[clang::fallthrough]]; // expected-warning{{fallthrough annotation does not directly precede switch label}}
+      [[clang::fallthrough]];  // expected-warning{{fallthrough annotation does not directly precede switch label}}
       n += 400;
     case 223:          // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}}
       [[clang::fallthrough]]; // expected-warning{{fallthrough annotation does not directly precede switch label}}
   }
 
   // TODO: uncomment this test after CFG gets more options to deal with
   // unreachable code:
   // http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20120507/057370.html
 #if 0
   long p = static_cast<long>(n) * n;
   switch (sizeof(p)) {
     case 9:                    // this test will not work on compilers with 72-bit long
       n += static_cast<int>(p >> 32);
       [[clang::fallthrough]];  // no warning here
     case 5:                    // it is not intended to work on compilers with 40-bit long as well
       n += static_cast<int>(p);
       break;
     default:
      break;
   }
 #endif
 
   return n;
 }
 
+enum Enum {
+  Value1, Value2
+};
+
+int fallthrough_covered_enums(Enum e) {
+  int n = 0;
+  switch (e) {
+    default:
+      n += 17;
+      [[clang::fallthrough]];  // no warning here, this shouldn't be treated as unreachable code
+    case Value1:
+      n += 19;
+      break;
+    case Value2:
+      n += 21;
+      break;
+  }
+  return n;
+}
+
 int fallthrough_targets(int n) {
   [[clang::fallthrough]]; // expected-error{{fallthrough annotation is outside switch statement}}
 
   [[clang::fallthrough]]  // expected-error{{fallthrough attribute is only allowed on empty statements}}
   switch (n) {
     case 121:
       n += 400;
       [[clang::fallthrough]]; // no warning here, correct target