Author: alexfh Date: Wed Feb 6 20:17:19 2013 New Revision: 174575 URL: http://llvm.org/viewvc/llvm-project?rev=174575&view=rev Log: -Wimplicit-fallthrough: fixed two cases where "fallthrough annotation in unreachable code" was issued incorrectly.
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. Reviewers: rsmith Reviewed By: rsmith CC: cfe-commits Differential Revision: http://llvm-reviews.chandlerc.com/D374 Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp cfe/trunk/test/SemaCXX/switch-implicit-fallthrough.cpp Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=174575&r1=174574&r2=174575&view=diff ============================================================================== --- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original) +++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Wed Feb 6 20:17:19 2013 @@ -708,6 +708,17 @@ namespace { ReachableBlocks.insert(&Cfg->getEntry()); BlockQueue.push_back(&Cfg->getEntry()); + // Mark all case blocks reachable to avoid problems with switching on + // constants, covered enums, etc. + // 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<SwitchCase>(L) && ReachableBlocks.insert(B)) + BlockQueue.push_back(B); + } + while (!BlockQueue.empty()) { const CFGBlock *P = BlockQueue.front(); BlockQueue.pop_front(); @@ -747,14 +758,16 @@ namespace { 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. } Modified: cfe/trunk/test/SemaCXX/switch-implicit-fallthrough.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/switch-implicit-fallthrough.cpp?rev=174575&r1=174574&r2=174575&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/switch-implicit-fallthrough.cpp (original) +++ cfe/trunk/test/SemaCXX/switch-implicit-fallthrough.cpp Wed Feb 6 20:17:19 2013 @@ -179,38 +179,56 @@ void fallthrough_cfgblock_with_null_succ 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 + case 9: 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 + case 5: n += static_cast<int>(p); - break; + [[clang::fallthrough]]; // no warning here default: - break; + n += 1; + 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}} _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
