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