[clang-tools-extra] r272993 - [clang-tidy] readability-identifier-naming - Support for Macros
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 17 02:25:24 PDT 2016
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) {}
}
}
More information about the cfe-commits
mailing list