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

5 files changed, 251 insertions, 28 deletions

diff --git a/lib/Analysis/CFG.cpp b/lib/Analysis/CFG.cpp
index 61c2a8709d..9a50578c4f 100644
--- a/lib/Analysis/CFG.cpp
+++ b/lib/Analysis/CFG.cpp
@@ -1070,9 +1070,6 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc) {
     case Stmt::LambdaExprClass:
       return VisitLambdaExpr(cast<LambdaExpr>(S), asc);
 
-    case Stmt::AttributedStmtClass:
-      return Visit(cast<AttributedStmt>(S)->getSubStmt(), asc);
-
     case Stmt::MemberExprClass:
       return VisitMemberExpr(cast<MemberExpr>(S), asc);
 
diff --git a/lib/Parse/ParseDeclCXX.cpp b/lib/Parse/ParseDeclCXX.cpp
index 3f06919bf6..2a64141353 100644
--- a/lib/Parse/ParseDeclCXX.cpp
+++ b/lib/Parse/ParseDeclCXX.cpp
@@ -2870,28 +2870,30 @@ void Parser::ParseCXX11AttributeSpecifier(ParsedAttributes &attrs,
     }
 
     bool AttrParsed = false;
-    // No scoped names are supported; ideally we could put all non-standard
-    // attributes into namespaces.
-    if (!ScopeName) {
-      switch (AttributeList::getKind(AttrName)) {
-      // No arguments
-      case AttributeList::AT_carries_dependency:
-      case AttributeList::AT_noreturn: {
-        if (Tok.is(tok::l_paren)) {
-          Diag(Tok.getLocation(), diag::err_cxx11_attribute_forbids_arguments)
-            << AttrName->getName();
-          break;
-        }
-
-        attrs.addNew(AttrName, AttrLoc, 0, AttrLoc, 0,
-                     SourceLocation(), 0, 0, false, true);
-        AttrParsed = true;
+    switch (AttributeList::getKind(AttrName, ScopeName)) {
+    // No arguments
+    case AttributeList::AT_carries_dependency:
+    // FIXME: implement generic support of attributes with C++11 syntax
+    // see Parse/ParseDecl.cpp: ParseGNUAttributes
+    case AttributeList::AT_clang___fallthrough:
+    case AttributeList::AT_noreturn: {
+      if (Tok.is(tok::l_paren)) {
+        Diag(Tok.getLocation(), diag::err_cxx11_attribute_forbids_arguments)
+          << AttrName->getName();
         break;
       }
 
-      // Silence warnings
-      default: break;
-      }
+      attrs.addNew(AttrName,
+                   SourceRange(ScopeLoc.isValid() ? ScopeLoc : AttrLoc,
+                               AttrLoc),
+                   ScopeName, ScopeLoc, 0,
+                   SourceLocation(), 0, 0, false, true);
+      AttrParsed = true;
+      break;
+    }
+
+    // Silence warnings
+    default: break;
     }
 
     // Skip the entire parameter clause, if any
diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp
index c3b802e4db..6e93030382 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 @@ static bool DiagnoseUninitializedUse(Sema &S, const VarDecl *VD,
   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 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
       .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 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
     }
   }
 
+  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;
diff --git a/lib/Sema/AttributeList.cpp b/lib/Sema/AttributeList.cpp
index 101e0384fa..8e7029350a 100644
--- a/lib/Sema/AttributeList.cpp
+++ b/lib/Sema/AttributeList.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/ADT/SmallString.h"
 using namespace clang;
 
 size_t AttributeList::allocated_size() const {
@@ -99,7 +100,8 @@ AttributePool::createIntegerAttribute(ASTContext &C, IdentifierInfo *Name,
 
 #include "clang/Sema/AttrParsedAttrKinds.inc"
 
-AttributeList::Kind AttributeList::getKind(const IdentifierInfo *Name) {
+AttributeList::Kind AttributeList::getKind(const IdentifierInfo *Name,
+                                           const IdentifierInfo *ScopeName) {
   StringRef AttrName = Name->getName();
 
   // Normalize the attribute name, __foo__ becomes foo.
@@ -107,5 +109,10 @@ AttributeList::Kind AttributeList::getKind(const IdentifierInfo *Name) {
       AttrName.size() >= 4)
     AttrName = AttrName.substr(2, AttrName.size() - 4);
 
+  // FIXME: implement attribute namespacing correctly.
+  SmallString<64> Buf;
+  if (ScopeName)
+    AttrName = ((Buf += ScopeName->getName()) += "___") += AttrName;
+
   return ::getAttrKind(AttrName);
 }
diff --git a/lib/Sema/SemaStmtAttr.cpp b/lib/Sema/SemaStmtAttr.cpp
index 21c329758e..912d7c6482 100644
--- a/lib/Sema/SemaStmtAttr.cpp
+++ b/lib/Sema/SemaStmtAttr.cpp
@@ -15,20 +15,46 @@
 #include "TargetAttributesSema.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/Sema/DelayedDiagnostic.h"
 #include "clang/Sema/Lookup.h"
+#include "clang/Sema/ScopeInfo.h"
 #include "llvm/ADT/StringExtras.h"
+
 using namespace clang;
 using namespace sema;
 
+static Attr *handleFallThroughAttr(Sema &S, Stmt *St, const AttributeList &A,
+                                   SourceRange Range) {
+  if (!isa<NullStmt>(St)) {
+    S.Diag(A.getRange().getBegin(), diag::err_fallthrough_attr_wrong_target)
+        << St->getLocStart();
+    if (isa<SwitchCase>(St)) {
+      SourceLocation L = Lexer::getLocForEndOfToken(Range.getEnd(), 0,
+                                  S.getSourceManager(), S.getLangOpts());
+      S.Diag(L, diag::note_fallthrough_insert_semi_fixit)
+          << FixItHint::CreateInsertion(L, ";");
+    }
+    return 0;
+  }
+  if (S.getCurFunction()->SwitchStack.empty()) {
+    S.Diag(A.getRange().getBegin(), diag::err_fallthrough_attr_outside_switch);
+    return 0;
+  }
+  return ::new (S.Context) FallThroughAttr(A.getRange(), S.Context);
+}
+
 
-static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const AttributeList &A) {
+static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const AttributeList &A,
+                                  SourceRange Range) {
   switch (A.getKind()) {
+  case AttributeList::AT_clang___fallthrough:
+    return handleFallThroughAttr(S, St, A, Range);
   default:
     // if we're here, then we parsed an attribute, but didn't recognize it as a
     // statement attribute => it is declaration attribute
-    S.Diag(A.getRange().getBegin(), diag::warn_attribute_invalid_on_stmt) <<
-      A.getName()->getName();
+    S.Diag(A.getRange().getBegin(), diag::warn_attribute_invalid_on_stmt)
+        << A.getName()->getName() << St->getLocStart();
     return 0;
   }
 }
@@ -37,7 +63,7 @@ StmtResult Sema::ProcessStmtAttributes(Stmt *S, AttributeList *AttrList,
                                        SourceRange Range) {
   AttrVec Attrs;
   for (const AttributeList* l = AttrList; l; l = l->getNext()) {
-    if (Attr *a = ProcessStmtAttribute(*this, S, *l))
+    if (Attr *a = ProcessStmtAttribute(*this, S, *l, Range))
       Attrs.push_back(a);
   }