[clang-tidy] Fix identifier naming in macro args.
Summary:
clang-tidy should fix identifier naming even when the identifier is
referenced inside a macro expansion, provided that the identifier enters
the macro expansion completely within a macro argument.
For example, this will allow fixes to the naming of the identifier
'global' when it is declared and used as follows:
int global;
#define USE_IN_MACRO(m) auto use_##m = m
USE_IN_MACRO(global);
Reviewers: alexfh
Subscribers: jlebar, cfe-commits
Differential Revision: https://reviews.llvm.org/D25450
llvm-svn: 284992
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index 7c49df9..bca5d46 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -612,27 +612,57 @@
static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
const IdentifierNamingCheck::NamingCheckId &Decl,
- SourceRange Range) {
+ SourceRange Range, SourceManager *SourceMgr = nullptr) {
// Do nothing if the provided range is invalid.
if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid())
return;
+ // If we have a source manager, use it to convert to the spelling location for
+ // performing the fix. This is necessary because macros can map the same
+ // spelling location to different source locations, and we only want to fix
+ // the token once, before it is expanded by the macro.
+ SourceLocation FixLocation = Range.getBegin();
+ if (SourceMgr)
+ FixLocation = SourceMgr->getSpellingLoc(FixLocation);
+ if (FixLocation.isInvalid())
+ return;
+
// Try to insert the identifier location in the Usages map, and bail out if it
// is already in there
auto &Failure = Failures[Decl];
- if (!Failure.RawUsageLocs.insert(Range.getBegin().getRawEncoding()).second)
+ if (!Failure.RawUsageLocs.insert(FixLocation.getRawEncoding()).second)
return;
- Failure.ShouldFix = Failure.ShouldFix && !Range.getBegin().isMacroID() &&
- !Range.getEnd().isMacroID();
+ if (!Failure.ShouldFix)
+ return;
+
+ // Check if the range is entirely contained within a macro argument.
+ SourceLocation MacroArgExpansionStartForRangeBegin;
+ SourceLocation MacroArgExpansionStartForRangeEnd;
+ bool RangeIsEntirelyWithinMacroArgument =
+ SourceMgr &&
+ SourceMgr->isMacroArgExpansion(Range.getBegin(),
+ &MacroArgExpansionStartForRangeBegin) &&
+ SourceMgr->isMacroArgExpansion(Range.getEnd(),
+ &MacroArgExpansionStartForRangeEnd) &&
+ MacroArgExpansionStartForRangeBegin == MacroArgExpansionStartForRangeEnd;
+
+ // Check if the range contains any locations from a macro expansion.
+ bool RangeContainsMacroExpansion = RangeIsEntirelyWithinMacroArgument ||
+ Range.getBegin().isMacroID() ||
+ Range.getEnd().isMacroID();
+
+ bool RangeCanBeFixed =
+ RangeIsEntirelyWithinMacroArgument || !RangeContainsMacroExpansion;
+ Failure.ShouldFix = RangeCanBeFixed;
}
/// Convenience method when the usage to be added is a NamedDecl
static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
- const NamedDecl *Decl, SourceRange Range) {
+ const NamedDecl *Decl, SourceRange Range, SourceManager *SourceMgr = nullptr) {
return addUsage(Failures, IdentifierNamingCheck::NamingCheckId(
Decl->getLocation(), Decl->getNameAsString()),
- Range);
+ Range, SourceMgr);
}
void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) {
@@ -719,7 +749,7 @@
if (const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>("declRef")) {
SourceRange Range = DeclRef->getNameInfo().getSourceRange();
- addUsage(NamingCheckFailures, DeclRef->getDecl(), Range);
+ addUsage(NamingCheckFailures, DeclRef->getDecl(), Range, Result.SourceManager);
return;
}