Author: alexfh Date: Fri Jun 17 04:25:24 2016 New Revision: 272993 URL: http://llvm.org/viewvc/llvm-project?rev=272993&view=rev Log: [clang-tidy] readability-identifier-naming - Support for Macros
Summary: Added support for macro definitions. -- 1. Added a pre-processor callback to catch macro definitions 2. Changed the type of the failure map so that macros and declarations can share the same map 3. Added extra tests to ensure fix-ups work using the new map 4. Added fix-ups for type aliases in variable and function declarations as part of adding the new tests Reviewers: alexfh Subscribers: Eugene.Zelenko, cfe-commits Patch by James Reynolds! Differential Revision: http://reviews.llvm.org/D21020 Modified: clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.h clang-tools-extra/trunk/docs/ReleaseNotes.rst clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp Modified: clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp?rev=272993&r1=272992&r2=272993&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp Fri Jun 17 04:25:24 2016 @@ -12,11 +12,53 @@ #include "llvm/Support/Debug.h" #include "llvm/Support/Format.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/PPCallbacks.h" +#include "clang/Lex/Preprocessor.h" +#include "llvm/ADT/DenseMapInfo.h" #define DEBUG_TYPE "clang-tidy" using namespace clang::ast_matchers; +namespace llvm { +/// Specialisation of DenseMapInfo to allow NamingCheckId objects in DenseMaps +template <> +struct DenseMapInfo< + clang::tidy::readability::IdentifierNamingCheck::NamingCheckId> { + using NamingCheckId = + clang::tidy::readability::IdentifierNamingCheck::NamingCheckId; + + static inline NamingCheckId getEmptyKey() { + return NamingCheckId( + clang::SourceLocation::getFromRawEncoding(static_cast<unsigned>(-1)), + "EMPTY"); + } + + static inline NamingCheckId getTombstoneKey() { + return NamingCheckId( + clang::SourceLocation::getFromRawEncoding(static_cast<unsigned>(-2)), + "TOMBSTONE"); + } + + static unsigned getHashValue(NamingCheckId Val) { + assert(Val != getEmptyKey() && "Cannot hash the empty key!"); + assert(Val != getTombstoneKey() && "Cannot hash the tombstone key!"); + + std::hash<NamingCheckId::second_type> SecondHash; + return Val.first.getRawEncoding() + SecondHash(Val.second); + } + + static bool isEqual(NamingCheckId LHS, NamingCheckId RHS) { + if (RHS == getEmptyKey()) + return LHS == getEmptyKey(); + if (RHS == getTombstoneKey()) + return LHS == getTombstoneKey(); + return LHS == RHS; + } +}; +} // namespace llvm + namespace clang { namespace tidy { namespace readability { @@ -66,6 +108,7 @@ namespace readability { m(TemplateTemplateParameter) \ m(TemplateParameter) \ m(TypeAlias) \ + m(MacroDefinition) \ enum StyleKind { #define ENUMERATE(v) SK_ ## v, @@ -84,6 +127,33 @@ static StringRef const StyleNames[] = { #undef NAMING_KEYS // clang-format on +namespace { +/// Callback supplies macros to IdentifierNamingCheck::checkMacro +class IdentifierNamingCheckPPCallbacks : public PPCallbacks { +public: + IdentifierNamingCheckPPCallbacks(Preprocessor *PP, + IdentifierNamingCheck *Check) + : PP(PP), Check(Check) {} + + /// MacroDefined calls checkMacro for macros in the main file + void MacroDefined(const Token &MacroNameTok, + const MacroDirective *MD) override { + Check->checkMacro(PP->getSourceManager(), MacroNameTok, MD->getMacroInfo()); + } + + /// MacroExpands calls expandMacro for macros in the main file + void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD, + SourceRange /*Range*/, + const MacroArgs * /*Args*/) override { + Check->expandMacro(MacroNameTok, MD.getMacroInfo()); + } + +private: + Preprocessor *PP; + IdentifierNamingCheck *Check; +}; +} // namespace + IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context) { @@ -146,6 +216,12 @@ void IdentifierNamingCheck::registerMatc Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this); } +void IdentifierNamingCheck::registerPPCallbacks(CompilerInstance &Compiler) { + Compiler.getPreprocessor().addPPCallbacks( + llvm::make_unique<IdentifierNamingCheckPPCallbacks>( + &Compiler.getPreprocessor(), this)); +} + static bool matchesStyle(StringRef Name, IdentifierNamingCheck::NamingStyle Style) { static llvm::Regex Matchers[] = { @@ -506,8 +582,8 @@ static StyleKind findStyleKind( } static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures, - const NamedDecl *Decl, SourceRange Range, - const SourceManager *SM) { + const IdentifierNamingCheck::NamingCheckId &Decl, + SourceRange Range) { // Do nothing if the provided range is invalid. if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid()) return; @@ -522,6 +598,14 @@ static void addUsage(IdentifierNamingChe !Range.getEnd().isMacroID(); } +/// Convenience method when the usage to be added is a NamedDecl +static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures, + const NamedDecl *Decl, SourceRange Range) { + return addUsage(Failures, IdentifierNamingCheck::NamingCheckId( + Decl->getLocation(), Decl->getNameAsString()), + Range); +} + void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) { if (const auto *Decl = Result.Nodes.getNodeAs<CXXConstructorDecl>("classRef")) { @@ -529,7 +613,7 @@ void IdentifierNamingCheck::check(const return; addUsage(NamingCheckFailures, Decl->getParent(), - Decl->getNameInfo().getSourceRange(), Result.SourceManager); + Decl->getNameInfo().getSourceRange()); return; } @@ -545,8 +629,7 @@ void IdentifierNamingCheck::check(const // we want instead to replace the next token, that will be the identifier. Range.setBegin(CharSourceRange::getTokenRange(Range).getEnd()); - addUsage(NamingCheckFailures, Decl->getParent(), Range, - Result.SourceManager); + addUsage(NamingCheckFailures, Decl->getParent(), Range); return; } @@ -563,8 +646,7 @@ void IdentifierNamingCheck::check(const } if (Decl) { - addUsage(NamingCheckFailures, Decl, Loc->getSourceRange(), - Result.SourceManager); + addUsage(NamingCheckFailures, Decl, Loc->getSourceRange()); return; } @@ -574,8 +656,7 @@ void IdentifierNamingCheck::check(const SourceRange Range(Ref.getTemplateNameLoc(), Ref.getTemplateNameLoc()); if (const auto *ClassDecl = dyn_cast<TemplateDecl>(Decl)) { - addUsage(NamingCheckFailures, ClassDecl->getTemplatedDecl(), Range, - Result.SourceManager); + addUsage(NamingCheckFailures, ClassDecl->getTemplatedDecl(), Range); return; } } @@ -583,7 +664,7 @@ void IdentifierNamingCheck::check(const if (const auto &Ref = Loc->getAs<DependentTemplateSpecializationTypeLoc>()) { addUsage(NamingCheckFailures, Ref.getTypePtr()->getAsTagDecl(), - Loc->getSourceRange(), Result.SourceManager); + Loc->getSourceRange()); return; } } @@ -592,8 +673,7 @@ void IdentifierNamingCheck::check(const Result.Nodes.getNodeAs<NestedNameSpecifierLoc>("nestedNameLoc")) { if (NestedNameSpecifier *Spec = Loc->getNestedNameSpecifier()) { if (NamespaceDecl *Decl = Spec->getAsNamespace()) { - addUsage(NamingCheckFailures, Decl, Loc->getLocalSourceRange(), - Result.SourceManager); + addUsage(NamingCheckFailures, Decl, Loc->getLocalSourceRange()); return; } } @@ -602,15 +682,14 @@ void IdentifierNamingCheck::check(const if (const auto *Decl = Result.Nodes.getNodeAs<UsingDecl>("using")) { for (const auto &Shadow : Decl->shadows()) { addUsage(NamingCheckFailures, Shadow->getTargetDecl(), - Decl->getNameInfo().getSourceRange(), Result.SourceManager); + Decl->getNameInfo().getSourceRange()); } return; } if (const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>("declRef")) { SourceRange Range = DeclRef->getNameInfo().getSourceRange(); - addUsage(NamingCheckFailures, DeclRef->getDecl(), Range, - Result.SourceManager); + addUsage(NamingCheckFailures, DeclRef->getDecl(), Range); return; } @@ -618,6 +697,33 @@ void IdentifierNamingCheck::check(const if (!Decl->getIdentifier() || Decl->getName().empty() || Decl->isImplicit()) return; + // Fix type aliases in value declarations + if (const auto *Value = Result.Nodes.getNodeAs<ValueDecl>("decl")) { + if (const auto *Typedef = + Value->getType().getTypePtr()->getAs<TypedefType>()) { + addUsage(NamingCheckFailures, Typedef->getDecl(), + Value->getSourceRange()); + } + } + + // Fix type aliases in function declarations + if (const auto *Value = Result.Nodes.getNodeAs<FunctionDecl>("decl")) { + if (const auto *Typedef = + Value->getReturnType().getTypePtr()->getAs<TypedefType>()) { + addUsage(NamingCheckFailures, Typedef->getDecl(), + Value->getSourceRange()); + } + for (unsigned i = 0; i < Value->getNumParams(); ++i) { + if (const auto *Typedef = Value->parameters()[i] + ->getType() + .getTypePtr() + ->getAs<TypedefType>()) { + addUsage(NamingCheckFailures, Typedef->getDecl(), + Value->getSourceRange()); + } + } + } + // Ignore ClassTemplateSpecializationDecl which are creating duplicate // replacements with CXXRecordDecl if (isa<ClassTemplateSpecializationDecl>(Decl)) @@ -644,29 +750,74 @@ void IdentifierNamingCheck::check(const KindName.c_str(), Name)); } } else { - NamingCheckFailure &Failure = NamingCheckFailures[Decl]; + NamingCheckFailure &Failure = NamingCheckFailures[NamingCheckId( + Decl->getLocation(), Decl->getNameAsString())]; SourceRange Range = DeclarationNameInfo(Decl->getDeclName(), Decl->getLocation()) .getSourceRange(); Failure.Fixup = std::move(Fixup); Failure.KindName = std::move(KindName); - addUsage(NamingCheckFailures, Decl, Range, Result.SourceManager); + addUsage(NamingCheckFailures, Decl, Range); } } } +void IdentifierNamingCheck::checkMacro(SourceManager &SourceMgr, + const Token &MacroNameTok, + const MacroInfo *MI) { + StringRef Name = MacroNameTok.getIdentifierInfo()->getName(); + NamingStyle Style = NamingStyles[SK_MacroDefinition]; + if (matchesStyle(Name, Style)) + return; + + std::string KindName = + fixupWithCase(StyleNames[SK_MacroDefinition], CT_LowerCase); + std::replace(KindName.begin(), KindName.end(), '_', ' '); + + std::string Fixup = fixupWithStyle(Name, Style); + if (StringRef(Fixup).equals(Name)) { + if (!IgnoreFailedSplit) { + DEBUG( + llvm::dbgs() << MacroNameTok.getLocation().printToString(SourceMgr) + << llvm::format(": unable to split words for %s '%s'\n", + KindName.c_str(), Name)); + } + } else { + NamingCheckId ID(MI->getDefinitionLoc(), Name); + NamingCheckFailure &Failure = NamingCheckFailures[ID]; + SourceRange Range(MacroNameTok.getLocation(), MacroNameTok.getEndLoc()); + + Failure.Fixup = std::move(Fixup); + Failure.KindName = std::move(KindName); + addUsage(NamingCheckFailures, ID, Range); + } +} + +void IdentifierNamingCheck::expandMacro(const Token &MacroNameTok, + const MacroInfo *MI) { + StringRef Name = MacroNameTok.getIdentifierInfo()->getName(); + NamingCheckId ID(MI->getDefinitionLoc(), Name); + + auto Failure = NamingCheckFailures.find(ID); + if (Failure == NamingCheckFailures.end()) + return; + + SourceRange Range(MacroNameTok.getLocation(), MacroNameTok.getEndLoc()); + addUsage(NamingCheckFailures, ID, Range); +} + void IdentifierNamingCheck::onEndOfTranslationUnit() { for (const auto &Pair : NamingCheckFailures) { - const NamedDecl &Decl = *Pair.first; + const NamingCheckId &Decl = Pair.first; const NamingCheckFailure &Failure = Pair.second; if (Failure.KindName.empty()) continue; if (Failure.ShouldFix) { - auto Diag = diag(Decl.getLocation(), "invalid case style for %0 '%1'") - << Failure.KindName << Decl.getName(); + auto Diag = diag(Decl.first, "invalid case style for %0 '%1'") + << Failure.KindName << Decl.second; for (const auto &Loc : Failure.RawUsageLocs) { // We assume that the identifier name is made of one token only. This is Modified: clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.h?rev=272993&r1=272992&r2=272993&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.h (original) +++ clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.h Fri Jun 17 04:25:24 2016 @@ -13,6 +13,9 @@ #include "../ClangTidy.h" namespace clang { + +class MacroInfo; + namespace tidy { namespace readability { @@ -36,6 +39,7 @@ public: void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void registerPPCallbacks(CompilerInstance &Compiler) override; void onEndOfTranslationUnit() override; enum CaseType { @@ -64,7 +68,7 @@ public: /// \brief Holds an identifier name check failure, tracking the kind of the /// identifer, its possible fixup and the starting locations of all the - /// idenfiier usages. + /// identifier usages. struct NamingCheckFailure { std::string KindName; std::string Fixup; @@ -81,9 +85,19 @@ public: NamingCheckFailure() : ShouldFix(true) {} }; - typedef llvm::DenseMap<const NamedDecl *, NamingCheckFailure> + + typedef std::pair<SourceLocation, std::string> NamingCheckId; + + typedef llvm::DenseMap<NamingCheckId, NamingCheckFailure> NamingCheckFailureMap; + /// Check Macros for style violations. + void checkMacro(SourceManager &sourceMgr, const Token &MacroNameTok, + const MacroInfo *MI); + + /// Add a usage of a macro if it already has a violation. + void expandMacro(const Token &MacroNameTok, const MacroInfo *MI); + private: std::vector<NamingStyle> NamingStyles; bool IgnoreFailedSplit; Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=272993&r1=272992&r2=272993&view=diff ============================================================================== --- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original) +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Fri Jun 17 04:25:24 2016 @@ -249,6 +249,11 @@ identified. The improvements since the Warns about defaulted constructors and assignment operators that are actually deleted. +- Updated `readability-identifier-naming-check + <http://clang.llvm.org/extra/clang-tidy/checks/readability-identifier-naming.html>`_ + + Added support for enforcing the case of macro statements. + - New `readability-redundant-control-flow <http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-control-flow.html>`_ check Modified: clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp?rev=272993&r1=272992&r2=272993&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp Fri Jun 17 04:25:24 2016 @@ -61,6 +61,7 @@ // RUN: {key: readability-identifier-naming.VariableCase, value: lower_case}, \ // RUN: {key: readability-identifier-naming.VirtualMethodCase, value: UPPER_CASE}, \ // RUN: {key: readability-identifier-naming.VirtualMethodPrefix, value: 'v_'}, \ +// RUN: {key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE}, \ // RUN: {key: readability-identifier-naming.TypeAliasCase, value: lower_case}, \ // RUN: {key: readability-identifier-naming.TypeAliasSuffix, value: '_t'}, \ // RUN: {key: readability-identifier-naming.IgnoreFailedSplit, value: 0} \ @@ -307,6 +308,12 @@ typedef THIS___Structure struct_type; // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for typedef 'struct_type' // CHECK-FIXES: {{^}}typedef this_structure struct_type_t;{{$}} +struct_type GlobalTypedefTestFunction(struct_type a_argument1) { +// CHECK-FIXES: {{^}}struct_type_t GlobalTypedefTestFunction(struct_type_t a_argument1) { + struct_type typedef_test_1; +// CHECK-FIXES: {{^}} struct_type_t typedef_test_1; +} + using my_struct_type = THIS___Structure; // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for type alias 'my_struct_type' // CHECK-FIXES: {{^}}using my_struct_type_t = this_structure;{{$}} @@ -329,5 +336,11 @@ static void static_Function() { // CHECK-FIXES: {{^}} using ::foo_ns::inline_namespace::ce_function;{{$}} } +#define MY_TEST_Macro(X) X() +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for macro definition 'MY_TEST_Macro' +// CHECK-FIXES: {{^}}#define MY_TEST_MACRO(X) X() + +void MY_TEST_Macro(function) {} +// CHECK-FIXES: {{^}}void MY_TEST_MACRO(function) {} } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
