Add -Wimplicit-fallthrough warning flag, which warns on fallthrough between
cases in switch statements. Also add a [[clang::fallthrough]] attribute, which
can be used to suppress the warning in the case of intentional fallthrough.
Patch by Alexander Kornienko!
The handling of C++11 attribute namespaces in this patch is temporary, and will
be replaced with a cleaner mechanism in a subsequent patch.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@156086 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp
index c3b802e..6e93030 100644
--- a/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/lib/Sema/AnalysisBasedWarnings.cpp
@@ -27,6 +27,7 @@
#include "clang/AST/StmtCXX.h"
#include "clang/AST/EvaluatedExprVisitor.h"
#include "clang/AST/StmtVisitor.h"
+#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Analysis/AnalysisContext.h"
#include "clang/Analysis/CFG.h"
#include "clang/Analysis/Analyses/ReachableCode.h"
@@ -42,7 +43,9 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Casting.h"
#include <algorithm>
+#include <iterator>
#include <vector>
+#include <deque>
using namespace clang;
@@ -522,6 +525,188 @@
return true;
}
+namespace {
+ class FallthroughMapper : public RecursiveASTVisitor<FallthroughMapper> {
+ public:
+ FallthroughMapper(Sema &S)
+ : FoundSwitchStatements(false),
+ S(S) {
+ }
+
+ bool foundSwitchStatements() const { return FoundSwitchStatements; }
+
+ void markFallthroughVisited(const AttributedStmt *Stmt) {
+ bool Found = FallthroughStmts.erase(Stmt);
+ assert(Found);
+ }
+
+ typedef llvm::SmallPtrSet<const AttributedStmt*, 8> AttrStmts;
+
+ const AttrStmts &getFallthroughStmts() const {
+ return FallthroughStmts;
+ }
+
+ bool checkFallThroughIntoBlock(const CFGBlock &B, int &AnnotatedCnt) {
+ int UnannotatedCnt = 0;
+ AnnotatedCnt = 0;
+
+ std::deque<const CFGBlock*> BlockQueue;
+
+ std::copy(B.pred_begin(), B.pred_end(), std::back_inserter(BlockQueue));
+
+ while (!BlockQueue.empty()) {
+ const CFGBlock *P = BlockQueue.front();
+ BlockQueue.pop_front();
+
+ const Stmt *Term = P->getTerminator();
+ if (Term && isa<SwitchStmt>(Term))
+ continue; // Switch statement, good.
+
+ const SwitchCase *SW = dyn_cast_or_null<SwitchCase>(P->getLabel());
+ if (SW && SW->getSubStmt() == B.getLabel() && P->begin() == P->end())
+ continue; // Previous case label has no statements, good.
+
+ if (P->pred_begin() == P->pred_end()) { // The block is unreachable.
+ // This only catches trivially unreachable blocks.
+ for (CFGBlock::const_iterator ElIt = P->begin(), ElEnd = P->end();
+ ElIt != ElEnd; ++ElIt) {
+ if (const CFGStmt *CS = ElIt->getAs<CFGStmt>()){
+ if (const AttributedStmt *AS = asFallThroughAttr(CS->getStmt())) {
+ S.Diag(AS->getLocStart(),
+ diag::warn_fallthrough_attr_unreachable);
+ markFallthroughVisited(AS);
+ ++AnnotatedCnt;
+ }
+ // 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.
+ // break;
+ // }
+ // // <<<< This place is represented by a 'hanging' CFG block.
+ // case Y:
+ continue;
+ }
+
+ const Stmt *LastStmt = getLastStmt(*P);
+ if (const AttributedStmt *AS = asFallThroughAttr(LastStmt)) {
+ markFallthroughVisited(AS);
+ ++AnnotatedCnt;
+ continue; // Fallthrough annotation, good.
+ }
+
+ if (!LastStmt) { // This block contains no executable statements.
+ // Traverse its predecessors.
+ std::copy(P->pred_begin(), P->pred_end(),
+ std::back_inserter(BlockQueue));
+ continue;
+ }
+
+ ++UnannotatedCnt;
+ }
+ return !!UnannotatedCnt;
+ }
+
+ // RecursiveASTVisitor setup.
+ bool shouldWalkTypesOfTypeLocs() const { return false; }
+
+ bool VisitAttributedStmt(AttributedStmt *S) {
+ if (asFallThroughAttr(S))
+ FallthroughStmts.insert(S);
+ return true;
+ }
+
+ bool VisitSwitchStmt(SwitchStmt *S) {
+ FoundSwitchStatements = true;
+ return true;
+ }
+
+ private:
+
+ static const AttributedStmt *asFallThroughAttr(const Stmt *S) {
+ if (const AttributedStmt *AS = dyn_cast_or_null<AttributedStmt>(S)) {
+ if (hasSpecificAttr<FallThroughAttr>(AS->getAttrs()))
+ return AS;
+ }
+ return 0;
+ }
+
+ static const Stmt *getLastStmt(const CFGBlock &B) {
+ if (const Stmt *Term = B.getTerminator())
+ return Term;
+ for (CFGBlock::const_reverse_iterator ElemIt = B.rbegin(),
+ ElemEnd = B.rend();
+ ElemIt != ElemEnd; ++ElemIt) {
+ if (const CFGStmt *CS = ElemIt->getAs<CFGStmt>())
+ return CS->getStmt();
+ }
+ // Workaround to detect a statement thrown out by CFGBuilder:
+ // case X: {} case Y:
+ // case X: ; case Y:
+ if (const SwitchCase *SW = dyn_cast_or_null<SwitchCase>(B.getLabel()))
+ if (!isa<SwitchCase>(SW->getSubStmt()))
+ return SW->getSubStmt();
+
+ return 0;
+ }
+
+ bool FoundSwitchStatements;
+ AttrStmts FallthroughStmts;
+ Sema &S;
+ };
+}
+
+static void DiagnoseSwitchLabelsFallthrough(Sema &S, AnalysisDeclContext &AC) {
+ FallthroughMapper FM(S);
+ FM.TraverseStmt(AC.getBody());
+
+ if (!FM.foundSwitchStatements())
+ return;
+
+ CFG *Cfg = AC.getCFG();
+
+ if (!Cfg)
+ return;
+
+ int AnnotatedCnt;
+
+ for (CFG::reverse_iterator I = Cfg->rbegin(), E = Cfg->rend(); I != E; ++I) {
+ const CFGBlock &B = **I;
+ const Stmt *Label = B.getLabel();
+
+ if (!Label || !isa<SwitchCase>(Label))
+ continue;
+
+ if (!FM.checkFallThroughIntoBlock(B, AnnotatedCnt))
+ continue;
+
+ S.Diag(Label->getLocStart(), diag::warn_unannotated_fallthrough);
+
+ if (!AnnotatedCnt) {
+ SourceLocation L = Label->getLocStart();
+ if (L.isMacroID())
+ continue;
+ if (S.getLangOpts().CPlusPlus0x) {
+ S.Diag(L, diag::note_insert_fallthrough_fixit) <<
+ FixItHint::CreateInsertion(L, "[[clang::fallthrough]]; ");
+ }
+ S.Diag(L, diag::note_insert_break_fixit) <<
+ FixItHint::CreateInsertion(L, "break; ");
+ }
+ }
+
+ const FallthroughMapper::AttrStmts &Fallthroughs = FM.getFallthroughStmts();
+ for (FallthroughMapper::AttrStmts::const_iterator I = Fallthroughs.begin(),
+ E = Fallthroughs.end();
+ I != E; ++I) {
+ S.Diag((*I)->getLocStart(), diag::warn_fallthrough_attr_invalid_placement);
+ }
+
+}
+
typedef std::pair<const Expr*, bool> UninitUse;
namespace {
@@ -861,7 +1046,8 @@
.setAlwaysAdd(Stmt::CStyleCastExprClass)
.setAlwaysAdd(Stmt::DeclRefExprClass)
.setAlwaysAdd(Stmt::ImplicitCastExprClass)
- .setAlwaysAdd(Stmt::UnaryOperatorClass);
+ .setAlwaysAdd(Stmt::UnaryOperatorClass)
+ .setAlwaysAdd(Stmt::AttributedStmtClass);
}
// Construct the analysis context with the specified CFG build options.
@@ -973,6 +1159,11 @@
}
}
+ if (Diags.getDiagnosticLevel(diag::warn_unannotated_fallthrough,
+ D->getLocStart()) != DiagnosticsEngine::Ignored) {
+ DiagnoseSwitchLabelsFallthrough(S, AC);
+ }
+
// Collect statistics about the CFG if it was built.
if (S.CollectStats && AC.isCFGBuilt()) {
++NumFunctionsAnalyzed;