[clang-tools-extra] r248700 - [clang-tidy] Code factorization and cleanup in IdentifierNamingCheck
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 28 01:59:12 PDT 2015
Author: alexfh
Date: Mon Sep 28 03:59:12 2015
New Revision: 248700
URL: http://llvm.org/viewvc/llvm-project?rev=248700&view=rev
Log:
[clang-tidy] Code factorization and cleanup in IdentifierNamingCheck
This is to level the ground a little bit, in preparation for the changes in http://reviews.llvm.org/D13081.
Code factorization replaces all insertions to NamingCheckFailures map with a unique addUsage function that does the job.
There is also no more difference between the declaration and the references to a given identifier, both cases are treated as ranges in the Usage vector. There is also a check to avoid duplicated ranges to be inserted, which sometimes triggered erroneous replacements.
References can now also be added before the declaration of the identifier is actually found; this looks to be the case for example when a templated class uses its parameters to specialize its templated base class.
Patch by Beren Minor!
Differential revision: http://reviews.llvm.org/D13079
Modified:
clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.h
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=248700&r1=248699&r2=248700&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp Mon Sep 28 03:59:12 2015
@@ -21,6 +21,7 @@ namespace clang {
namespace tidy {
namespace readability {
+// clang-format off
#define NAMING_KEYS(m) \
m(Namespace) \
m(InlineNamespace) \
@@ -80,6 +81,7 @@ static StringRef const StyleNames[] = {
};
#undef NAMING_KEYS
+// clang-format on
IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
ClangTidyContext *Context)
@@ -134,10 +136,10 @@ void IdentifierNamingCheck::storeOptions
}
void IdentifierNamingCheck::registerMatchers(MatchFinder *Finder) {
-// FIXME: For now, only Decl and DeclRefExpr nodes are visited for checking and
-// replacement. There is a lot of missing cases, such as references to a class
-// name (as in 'const int CMyClass::kClassConstant = 4;'), to an enclosing
-// context (namespace, class, etc).
+ // FIXME: For now, only Decl and DeclRefExpr nodes are visited for checking
+ // and replacement. There is a lot of missing cases, such as references to a
+ // class name (as in 'const int CMyClass::kClassConstant = 4;'), to an
+ // enclosing context (namespace, class, etc).
Finder->addMatcher(namedDecl().bind("decl"), this);
Finder->addMatcher(declRefExpr().bind("declref"), this);
@@ -499,23 +501,24 @@ static StyleKind findStyleKind(
return SK_Invalid;
}
+static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
+ const NamedDecl *Decl, SourceRange Range,
+ const SourceManager *SM) {
+ auto &Failure = Failures[Decl];
+ if (!Failure.RawUsageLocs.insert(Range.getBegin().getRawEncoding()).second)
+ return;
+
+ Failure.ShouldFix = Failure.ShouldFix && SM->isInMainFile(Range.getBegin()) &&
+ SM->isInMainFile(Range.getEnd()) &&
+ !Range.getBegin().isMacroID() &&
+ !Range.getEnd().isMacroID();
+}
+
void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) {
if (const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>("declref")) {
- auto It = NamingCheckFailures.find(DeclRef->getDecl());
- if (It == NamingCheckFailures.end())
- return;
-
- NamingCheckFailure &Failure = It->second;
SourceRange Range = DeclRef->getNameInfo().getSourceRange();
-
- Failure.Usages.push_back(Range);
- Failure.ShouldFix = Failure.ShouldFix &&
- Result.SourceManager->isInMainFile(Range.getBegin()) &&
- Result.SourceManager->isInMainFile(Range.getEnd()) &&
- !Range.getBegin().isMacroID() &&
- !Range.getEnd().isMacroID();
-
- return;
+ return addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
+ Result.SourceManager);
}
if (const auto *Decl = Result.Nodes.getNodeAs<NamedDecl>("decl")) {
@@ -550,11 +553,7 @@ void IdentifierNamingCheck::check(const
Failure.Fixup = std::move(Fixup);
Failure.KindName = std::move(KindName);
- Failure.ShouldFix =
- Failure.ShouldFix &&
- Result.SourceManager->isInMainFile(Range.getBegin()) &&
- Result.SourceManager->isInMainFile(Range.getEnd()) &&
- !Range.getBegin().isMacroID() && !Range.getEnd().isMacroID();
+ addUsage(NamingCheckFailures, Decl, Range, Result.SourceManager);
}
}
}
@@ -564,19 +563,19 @@ void IdentifierNamingCheck::onEndOfTrans
const NamedDecl &Decl = *Pair.first;
const NamingCheckFailure &Failure = Pair.second;
- SourceRange DeclRange =
- DeclarationNameInfo(Decl.getDeclName(), Decl.getLocation())
- .getSourceRange();
+ if (Failure.KindName.empty())
+ continue;
+
auto Diag = diag(Decl.getLocStart(), "invalid case style for %0 '%1'")
<< Failure.KindName << Decl.getName();
-
if (Failure.ShouldFix) {
- Diag << FixItHint::CreateReplacement(
- CharSourceRange::getTokenRange(DeclRange), Failure.Fixup);
-
- for (const auto &Range : Failure.Usages) {
+ for (const auto &Loc : Failure.RawUsageLocs) {
+ // We assume that the identifier name is made of one token only. This is
+ // always the case as we ignore usages in macros that could build
+ // identifier names by combining multiple tokens.
Diag << FixItHint::CreateReplacement(
- CharSourceRange::getTokenRange(Range), Failure.Fixup);
+ SourceRange(SourceLocation::getFromRawEncoding(Loc)),
+ Failure.Fixup);
}
}
}
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=248700&r1=248699&r2=248700&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.h Mon Sep 28 03:59:12 2015
@@ -62,20 +62,32 @@ public:
}
};
-private:
- std::vector<NamingStyle> NamingStyles;
- bool IgnoreFailedSplit;
-
+ /// \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.
struct NamingCheckFailure {
std::string KindName;
std::string Fixup;
+
+ /// \brief Whether the failure should be fixed or not.
+ ///
+ /// ie: if the identifier was used or declared within a macro we won't offer
+ /// a fixup for safety reasons.
bool ShouldFix;
- std::vector<SourceRange> Usages;
+
+ /// \brief A set of all the identifier usages starting SourceLocation, in
+ /// their encoded form.
+ llvm::DenseSet<unsigned> RawUsageLocs;
NamingCheckFailure() : ShouldFix(true) {}
};
+ typedef llvm::DenseMap<const NamedDecl *, NamingCheckFailure>
+ NamingCheckFailureMap;
- llvm::DenseMap<const NamedDecl *, NamingCheckFailure> NamingCheckFailures;
+private:
+ std::vector<NamingStyle> NamingStyles;
+ bool IgnoreFailedSplit;
+ NamingCheckFailureMap NamingCheckFailures;
};
} // namespace readability
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=248700&r1=248699&r2=248700&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 Mon Sep 28 03:59:12 2015
@@ -68,6 +68,8 @@
// FIXME: name, declaration contexts, forward declarations, etc, are correctly
// FIXME: checked and renamed
+// clang-format off
+
namespace FOO_NS {
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: invalid case style for namespace 'FOO_NS' [readability-identifier-naming]
// CHECK-FIXES: {{^}}namespace foo_ns {{{$}}
More information about the cfe-commits
mailing list