Author: alexfh Date: Fri Sep 28 17:24:03 2012 New Revision: 164858 URL: http://llvm.org/viewvc/llvm-project?rev=164858&view=rev Log: 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 Added: cfe/trunk/test/SemaCXX/switch-implicit-fallthrough-macro.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/include/clang/Basic/TokenKinds.h cfe/trunk/include/clang/Lex/MacroInfo.h cfe/trunk/include/clang/Lex/Token.h cfe/trunk/lib/Lex/MacroInfo.cpp cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=164858&r1=164857&r2=164858&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Sep 28 17:24:03 2012 @@ -5667,7 +5667,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< Modified: cfe/trunk/include/clang/Basic/TokenKinds.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TokenKinds.h?rev=164858&r1=164857&r2=164858&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/TokenKinds.h (original) +++ cfe/trunk/include/clang/Basic/TokenKinds.h Fri Sep 28 17:24:03 2012 @@ -63,6 +63,31 @@ /// 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 Modified: cfe/trunk/include/clang/Lex/MacroInfo.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/MacroInfo.h?rev=164858&r1=164857&r2=164858&view=diff ============================================================================== --- cfe/trunk/include/clang/Lex/MacroInfo.h (original) +++ cfe/trunk/include/clang/Lex/MacroInfo.h Fri Sep 28 17:24:03 2012 @@ -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) Modified: cfe/trunk/include/clang/Lex/Token.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Token.h?rev=164858&r1=164857&r2=164858&view=diff ============================================================================== --- cfe/trunk/include/clang/Lex/Token.h (original) +++ cfe/trunk/include/clang/Lex/Token.h Fri Sep 28 17:24:03 2012 @@ -90,26 +90,18 @@ /// \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 Modified: cfe/trunk/lib/Lex/MacroInfo.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/MacroInfo.cpp?rev=164858&r1=164857&r2=164858&view=diff ============================================================================== --- cfe/trunk/lib/Lex/MacroInfo.cpp (original) +++ cfe/trunk/lib/Lex/MacroInfo.cpp Fri Sep 28 17:24:03 2012 @@ -58,6 +58,18 @@ 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; Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=164858&r1=164857&r2=164858&view=diff ============================================================================== --- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original) +++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Fri Sep 28 17:24:03 2012 @@ -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 @@ 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 @@ 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) << Added: cfe/trunk/test/SemaCXX/switch-implicit-fallthrough-macro.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/switch-implicit-fallthrough-macro.cpp?rev=164858&view=auto ============================================================================== --- cfe/trunk/test/SemaCXX/switch-implicit-fallthrough-macro.cpp (added) +++ cfe/trunk/test/SemaCXX/switch-implicit-fallthrough-macro.cpp Fri Sep 28 17:24:03 2012 @@ -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}} +} _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
