From e61e5625e7d8a44f0f142f5f2faa912601b447f5 Mon Sep 17 00:00:00 2001 From: Alexander Kornienko Date: Fri, 28 Sep 2012 22:24:03 +0000 Subject: [PATCH] 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 | 2 +- clang/include/clang/Basic/TokenKinds.h | 25 ++++ clang/include/clang/Lex/MacroInfo.h | 5 + clang/include/clang/Lex/Token.h | 18 +-- clang/lib/Lex/MacroInfo.cpp | 12 ++ clang/lib/Sema/AnalysisBasedWarnings.cpp | 67 +++++++++- .../SemaCXX/switch-implicit-fallthrough-macro.cpp | 139 +++++++++++++++++++++ 7 files changed, 253 insertions(+), 15 deletions(-) create mode 100644 clang/test/SemaCXX/switch-implicit-fallthrough-macro.cpp diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 57c6907..a527c84 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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, 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< diff --git a/clang/include/clang/Basic/TokenKinds.h b/clang/include/clang/Basic/TokenKinds.h index 478add8..e850971 100644 --- a/clang/include/clang/Basic/TokenKinds.h +++ b/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 diff --git a/clang/include/clang/Lex/MacroInfo.h b/clang/include/clang/Lex/MacroInfo.h index ba3c30d..db6cdef 100644 --- a/clang/include/clang/Lex/MacroInfo.h +++ b/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) diff --git a/clang/include/clang/Lex/Token.h b/clang/include/clang/Lex/Token.h index 9c5a023..50b86c8 100644 --- a/clang/include/clang/Lex/Token.h +++ b/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 diff --git a/clang/lib/Lex/MacroInfo.cpp b/clang/lib/Lex/MacroInfo.cpp index 7a4964f..ffe31f2 100644 --- a/clang/lib/Lex/MacroInfo.cpp +++ b/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; diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index bc25c0a..ec500a0 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/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 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 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 { 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(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) << diff --git a/clang/test/SemaCXX/switch-implicit-fallthrough-macro.cpp b/clang/test/SemaCXX/switch-implicit-fallthrough-macro.cpp new file mode 100644 index 0000000..add212f --- /dev/null +++ b/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 +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}} +} -- 2.7.4