Compatibility macro detection for the -Wimplicit-fallthrough diagnostic.

Summary:
When issuing a diagnostic message for the -Wimplicit-fallthrough diagnostics, always try to find the latest macro, defined at the point of fallthrough, which is immediately expanded to "[[clang::fallthrough]]", and use it's name instead of the actual sequence.

Known issues: 
  * uses PP.getSpelling() to compare macro definition with a string (anyone can suggest a convenient way to fill a token array, or maybe lex it in runtime?);
  * this can be generalized and used in other similar cases, any ideas where it should reside then?

Reviewers: doug.gregor, rsmith

Reviewed By: rsmith

CC: cfe-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D50

llvm-svn: 164858

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -5667,7 +5667,7 @@ def warn_unannotated_fallthrough_per_function : Warning<
   "unannotated fall-through between switch labels in partly-annotated "
   "function">, InGroup<ImplicitFallthroughPerFunction>, DefaultIgnore;
 def note_insert_fallthrough_fixit : Note<
-  "insert '[[clang::fallthrough]];' to silence this warning">;
+  "insert '%0;' to silence this warning">;
 def note_insert_break_fixit : Note<
   "insert 'break;' to avoid fall-through">;
 def err_fallthrough_attr_wrong_target : Error<

clang/include/clang/Basic/TokenKinds.h

@@ -63,6 +63,31 @@ const char *getTokenName(enum TokenKind Kind);
 /// Preprocessor::getSpelling().
 const char *getTokenSimpleSpelling(enum TokenKind Kind);
 
+/// \brief Return true if this is a raw identifier or an identifier kind.
+inline bool isAnyIdentifier(TokenKind K) {
+  return (K == tok::identifier) || (K == tok::raw_identifier);
+}
+
+/// \brief Return true if this is a "literal" kind, like a numeric
+/// constant, string, etc.
+inline bool isLiteral(TokenKind K) {
+  return (K == tok::numeric_constant) || (K == tok::char_constant) ||
+         (K == tok::wide_char_constant) || (K == tok::utf16_char_constant) ||
+         (K == tok::utf32_char_constant) || (K == tok::string_literal) ||
+         (K == tok::wide_string_literal) || (K == tok::utf8_string_literal) ||
+         (K == tok::utf16_string_literal) || (K == tok::utf32_string_literal) ||
+         (K == tok::angle_string_literal);
+}
+
+/// \brief Return true if this is any of tok::annot_* kinds.
+inline bool isAnnotation(TokenKind K) {
+#define ANNOTATION(NAME) \
+  if (K == tok::annot_##NAME) \
+    return true;
+#include "clang/Basic/TokenKinds.def"
+  return false;
+}
+
 }  // end namespace tok
 }  // end namespace clang
 

clang/include/clang/Lex/MacroInfo.h

@@ -159,6 +159,11 @@ public:
   /// \brief Get previous definition of the macro with the same name.
   MacroInfo *getPreviousDefinition() { return PreviousDefinition; }
 
+  /// \brief Find macro definition active in the specified source location. If
+  /// this macro was not defined there, return NULL.
+  const MacroInfo *findDefinitionAtLoc(SourceLocation L,
+                                       SourceManager &SM) const;
+
   /// \brief Get length in characters of the macro definition.
   unsigned getDefinitionLength(SourceManager &SM) const {
     if (IsDefinitionLengthCached)

clang/include/clang/Lex/Token.h

@@ -90,26 +90,18 @@ public:
   /// \brief Return true if this is a raw identifier (when lexing
   /// in raw mode) or a non-keyword identifier (when lexing in non-raw mode).
   bool isAnyIdentifier() const {
-    return is(tok::identifier) || is(tok::raw_identifier);
+    return tok::isAnyIdentifier(getKind());
   }
 
-  /// isLiteral - Return true if this is a "literal", like a numeric
+  /// \brief Return true if this is a "literal", like a numeric
   /// constant, string, etc.
   bool isLiteral() const {
-    return is(tok::numeric_constant) || is(tok::char_constant) ||
-           is(tok::wide_char_constant) || is(tok::utf16_char_constant) ||
-           is(tok::utf32_char_constant) || is(tok::string_literal) ||
-           is(tok::wide_string_literal) || is(tok::utf8_string_literal) ||
-           is(tok::utf16_string_literal) || is(tok::utf32_string_literal) ||
-           is(tok::angle_string_literal);
+    return tok::isLiteral(getKind());
   }
 
+  /// \brief Return true if this is any of tok::annot_* kind tokens.
   bool isAnnotation() const {
-#define ANNOTATION(NAME) \
-    if (is(tok::annot_##NAME)) \
-      return true;
-#include "clang/Basic/TokenKinds.def"
-    return false;
+    return tok::isAnnotation(getKind());
   }
 
   /// \brief Return a source location identifier for the specified

clang/lib/Lex/MacroInfo.cpp

@@ -58,6 +58,18 @@ MacroInfo::MacroInfo(const MacroInfo &MI, llvm::BumpPtrAllocator &PPAllocator)
   setArgumentList(MI.ArgumentList, MI.NumArguments, PPAllocator);
 }
 
+const MacroInfo *MacroInfo::findDefinitionAtLoc(SourceLocation L,
+                                                SourceManager &SM) const {
+  assert(L.isValid() && "SourceLocation is invalid.");
+  for (const MacroInfo *MI = this; MI; MI = MI->PreviousDefinition) {
+    if (MI->Location.isInvalid() ||  // For macros defined on the command line.
+        SM.isBeforeInTranslationUnit(MI->Location, L))
+      return (MI->UndefLocation.isInvalid() ||
+              SM.isBeforeInTranslationUnit(L, MI->UndefLocation)) ? MI : NULL;
+  }
+  return NULL;
+}
+
 unsigned MacroInfo::getDefinitionLengthSlow(SourceManager &SM) const {
   assert(!IsDefinitionLengthCached);
   IsDefinitionLengthCached = true;

clang/lib/Sema/AnalysisBasedWarnings.cpp

@@ -36,6 +36,7 @@
 #include "clang/Analysis/Analyses/ThreadSafety.h"
 #include "clang/Analysis/CFGStmtMap.h"
 #include "clang/Analysis/Analyses/UninitializedValues.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableMap.h"
@@ -678,6 +679,61 @@ static bool DiagnoseUninitializedUse(Sema &S, const VarDecl *VD,
   return true;
 }
 
+/// \brief Stores token information for comparing actual tokens with
+/// predefined values. Only handles simple tokens and identifiers.
+class TokenValue {
+  tok::TokenKind Kind;
+  IdentifierInfo *II;
+
+public:
+  TokenValue(tok::TokenKind Kind) : Kind(Kind), II(0) {
+    assert(Kind != tok::raw_identifier && "Raw identifiers are not supported.");
+    assert(Kind != tok::identifier &&
+           "Identifiers should be created by TokenValue(IdentifierInfo *)");
+    assert(!tok::isLiteral(Kind) && "Literals are not supported.");
+    assert(!tok::isAnnotation(Kind) && "Annotations are not supported.");
+  }
+  TokenValue(IdentifierInfo *II) : Kind(tok::identifier), II(II) {}
+  bool operator==(const Token &Tok) const {
+    return Tok.getKind() == Kind &&
+        (!II || II == Tok.getIdentifierInfo());
+  }
+};
+
+/// \brief Compares macro tokens with a specified token value sequence.
+static bool MacroDefinitionEquals(const MacroInfo *MI,
+                                  llvm::ArrayRef<TokenValue> Tokens) {
+  return Tokens.size() == MI->getNumTokens() &&
+      std::equal(Tokens.begin(), Tokens.end(), MI->tokens_begin());
+}
+
+static std::string GetSuitableSpelling(Preprocessor &PP, SourceLocation L,
+                                       llvm::ArrayRef<TokenValue> Tokens,
+                                       const char *Spelling) {
+  SourceManager &SM = PP.getSourceManager();
+  SourceLocation BestLocation;
+  std::string BestSpelling = Spelling;
+  for (Preprocessor::macro_iterator I = PP.macro_begin(), E = PP.macro_end();
+       I != E; ++I) {
+    if (!I->second->isObjectLike())
+      continue;
+    const MacroInfo *MI = I->second->findDefinitionAtLoc(L, SM);
+    if (!MI)
+      continue;
+    if (!MacroDefinitionEquals(MI, Tokens))
+      continue;
+    SourceLocation Location = I->second->getDefinitionLoc();
+    // Choose the macro defined latest.
+    if (BestLocation.isInvalid() ||
+        (Location.isValid() &&
+         SM.isBeforeInTranslationUnit(BestLocation, Location))) {
+      BestLocation = Location;
+      BestSpelling = I->first->getName();
+    }
+  }
+  return BestSpelling;
+}
+
 namespace {
   class FallthroughMapper : public RecursiveASTVisitor<FallthroughMapper> {
   public:
@@ -852,8 +908,17 @@ static void DiagnoseSwitchLabelsFallthrough(Sema &S, AnalysisDeclContext &AC,
       if (S.getLangOpts().CPlusPlus0x) {
         const Stmt *Term = B.getTerminator();
         if (!(B.empty() && Term && isa<BreakStmt>(Term))) {
+          Preprocessor &PP = S.getPreprocessor();
+          TokenValue Tokens[] = {
+            tok::l_square, tok::l_square, PP.getIdentifierInfo("clang"),
+            tok::coloncolon, PP.getIdentifierInfo("fallthrough"),
+            tok::r_square, tok::r_square
+          };
+          std::string AnnotationSpelling = GetSuitableSpelling(
+              PP, L, Tokens, "[[clang::fallthrough]]");
           S.Diag(L, diag::note_insert_fallthrough_fixit) <<
-            FixItHint::CreateInsertion(L, "[[clang::fallthrough]]; ");
+              AnnotationSpelling <<
+              FixItHint::CreateInsertion(L, AnnotationSpelling + "; ");
         }
       }
       S.Diag(L, diag::note_insert_break_fixit) <<

clang/test/SemaCXX/switch-implicit-fallthrough-macro.cpp

@@ -0,0 +1,139 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough -DCOMMAND_LINE_FALLTHROUGH=[[clang::fallthrough]] %s
+
+int fallthrough_compatibility_macro_from_command_line(int n) {
+  switch (n) {
+    case 0:
+      n = n * 10;
+    case 1: // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert 'COMMAND_LINE_FALLTHROUGH;' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}}
+      ;
+  }
+  return n;
+}
+
+#ifdef __clang__
+#if __has_feature(cxx_attributes) && __has_warning("-Wimplicit-fallthrough")
+#define COMPATIBILITY_FALLTHROUGH   [ [ /* test */  clang /* test */ \
+    ::  fallthrough  ]  ]    // testing whitespace and comments in macro definition
+#endif
+#endif
+
+#ifndef COMPATIBILITY_FALLTHROUGH
+#define COMPATIBILITY_FALLTHROUGH do { } while (0)
+#endif
+
+int fallthrough_compatibility_macro_from_source(int n) {
+  switch (n) {
+    case 0:
+      n = n * 20;
+    case 1: // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert 'COMPATIBILITY_FALLTHROUGH;' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}}
+      ;
+  }
+  return n;
+}
+
+// Deeper macro substitution
+#define M1 [[clang::fallthrough]]
+#ifdef __clang__
+#define M2 M1
+#else
+#define M2
+#endif
+
+#define WRONG_MACRO1 clang::fallthrough
+#define WRONG_MACRO2 [[clang::fallthrough]
+#define WRONG_MACRO3 [[clang::fall through]]
+#define WRONG_MACRO4 [[clang::fallthrough]]]
+
+int fallthrough_compatibility_macro_in_macro(int n) {
+  switch (n) {
+    case 0:
+      n = n * 20;
+    case 1: // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert 'M1;' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}}
+                                                                          // there was an idea that this ^ should be M2
+      ;
+  }
+  return n;
+}
+
+#undef M1
+#undef M2
+#undef COMPATIBILITY_FALLTHROUGH
+#undef COMMAND_LINE_FALLTHROUGH
+
+int fallthrough_compatibility_macro_undefined(int n) {
+  switch (n) {
+    case 0:
+      n = n * 20;
+    case 1: // 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}}
+      ;
+  }
+#define TOO_LATE [[clang::fallthrough]]
+  return n;
+}
+#undef TOO_LATE
+
+#define MACRO_WITH_HISTORY 11111111
+#undef MACRO_WITH_HISTORY
+#define MACRO_WITH_HISTORY [[clang::fallthrough]]
+#undef MACRO_WITH_HISTORY
+#define MACRO_WITH_HISTORY 2222222
+
+int fallthrough_compatibility_macro_history(int n) {
+  switch (n) {
+    case 0:
+      n = n * 20;
+#undef MACRO_WITH_HISTORY
+    case 1: // 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}}
+      ;
+#define MACRO_WITH_HISTORY [[clang::fallthrough]]
+  }
+  return n;
+}
+
+#undef MACRO_WITH_HISTORY
+#define MACRO_WITH_HISTORY 11111111
+#undef MACRO_WITH_HISTORY
+#define MACRO_WITH_HISTORY [[clang::fallthrough]]
+#undef MACRO_WITH_HISTORY
+#define MACRO_WITH_HISTORY 2222222
+#undef MACRO_WITH_HISTORY
+
+int fallthrough_compatibility_macro_history2(int n) {
+  switch (n) {
+    case 0:
+      n = n * 20;
+#define MACRO_WITH_HISTORY [[clang::fallthrough]]
+    case 1: // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert 'MACRO_WITH_HISTORY;' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}}
+      ;
+#undef MACRO_WITH_HISTORY
+#define MACRO_WITH_HISTORY 3333333
+#undef MACRO_WITH_HISTORY
+#define MACRO_WITH_HISTORY 4444444
+#undef MACRO_WITH_HISTORY
+#define MACRO_WITH_HISTORY 5555555
+  }
+  return n;
+}
+
+template<const int N>
+int fallthrough_compatibility_macro_history_template(int n) {
+  switch (N * n) {
+    case 0:
+      n = n * 20;
+#define MACRO_WITH_HISTORY2 [[clang::fallthrough]]
+    case 1: // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert 'MACRO_WITH_HISTORY2;' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}}
+      ;
+#undef MACRO_WITH_HISTORY2
+#define MACRO_WITH_HISTORY2 3333333
+  }
+  return n;
+}
+
+#undef MACRO_WITH_HISTORY2
+#define MACRO_WITH_HISTORY2 4444444
+#undef MACRO_WITH_HISTORY2
+#define MACRO_WITH_HISTORY2 5555555
+
+void f() {
+  fallthrough_compatibility_macro_history_template<1>(0); // expected-note{{in instantiation of function template specialization 'fallthrough_compatibility_macro_history_template<1>' requested here}}
+}