Hi doug.gregor, rsmith,

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?

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

Files:
  tools/clang/test/SemaCXX/switch-implicit-fallthrough-macro.cpp
  tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
  tools/clang/include/clang/Lex/MacroInfo.h
  tools/clang/lib/Sema/AnalysisBasedWarnings.cpp
  tools/clang/lib/Lex/MacroInfo.cpp
Index: tools/clang/test/SemaCXX/switch-implicit-fallthrough-macro.cpp
===================================================================
--- tools/clang/test/SemaCXX/switch-implicit-fallthrough-macro.cpp
+++ tools/clang/test/SemaCXX/switch-implicit-fallthrough-macro.cpp
@@ -0,0 +1,133 @@
+// 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   [[   clang  ::  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
+
+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}}
+}
Index: tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5651,7 +5651,7 @@
   "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<
Index: tools/clang/include/clang/Lex/MacroInfo.h
===================================================================
--- tools/clang/include/clang/Lex/MacroInfo.h
+++ tools/clang/include/clang/Lex/MacroInfo.h
@@ -159,6 +159,11 @@
   /// \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)
Index: tools/clang/lib/Sema/AnalysisBasedWarnings.cpp
===================================================================
--- tools/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ tools/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -678,6 +678,46 @@
   return true;
 }
 
+static bool MacroDefinitionEquals(const MacroInfo *MI, const char *Definition,
+                                  Preprocessor &PP) {
+  SmallVector<char, 1024> Buf;
+  const char *P = Definition;
+  for (MacroInfo::tokens_iterator it = MI->tokens_begin();
+       it != MI->tokens_end(); ++it) {
+    StringRef S = PP.getSpelling(*it, Buf);
+    if (StringRef(P, S.size()) != S)
+      return false;
+    P += S.size();
+  }
+  return *P == '\0';
+}
+
+static std::string GetSuitableSpelling(Preprocessor &PP, SourceLocation L,
+                                       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, Spelling, PP))
+      continue;
+    SourceLocation Location = I->second->getDefinitionLoc();
+    // Choose the macro defined latest.
+    if (BestLocation.isInvalid() ||
+        (SM.getFileEntryForID(SM.getFileID(Location)) &&
+         SM.isBeforeInTranslationUnit(BestLocation, Location))) {
+      BestLocation = Location;
+      BestSpelling = I->first->getName();
+    }
+  }
+  return BestSpelling;
+}
+
 namespace {
   class FallthroughMapper : public RecursiveASTVisitor<FallthroughMapper> {
   public:
@@ -852,8 +892,11 @@
       if (S.getLangOpts().CPlusPlus0x) {
         const Stmt *Term = B.getTerminator();
         if (!(B.empty() && Term && isa<BreakStmt>(Term))) {
+          std::string AnnotationSpelling = GetSuitableSpelling(
+              S.getPreprocessor(), L, "[[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) <<
Index: tools/clang/lib/Lex/MacroInfo.cpp
===================================================================
--- tools/clang/lib/Lex/MacroInfo.cpp
+++ tools/clang/lib/Lex/MacroInfo.cpp
@@ -58,6 +58,17 @@
   setArgumentList(MI.ArgumentList, MI.NumArguments, PPAllocator);
 }
 
+const MacroInfo *MacroInfo::findDefinitionAtLoc(SourceLocation L,
+                                                SourceManager &SM) const {
+  for (const MacroInfo *MI = this; MI; MI = MI->PreviousDefinition) {
+    if (!SM.getFileEntryForID(SM.getFileID(MI->Location)) ||
+        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;
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to