[clang-tools-extra] r284992 - [clang-tidy] Fix identifier naming in macro args.
Jason Henline via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 24 10:20:32 PDT 2016
Author: jhen
Date: Mon Oct 24 12:20:32 2016
New Revision: 284992
URL: http://llvm.org/viewvc/llvm-project?rev=284992&view=rev
Log:
[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
Modified:
clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
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=284992&r1=284991&r2=284992&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp Mon Oct 24 12:20:32 2016
@@ -612,27 +612,57 @@ static StyleKind findStyleKind(
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;
+
+ if (!Failure.ShouldFix)
return;
- Failure.ShouldFix = Failure.ShouldFix && !Range.getBegin().isMacroID() &&
- !Range.getEnd().isMacroID();
+ // 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 @@ void IdentifierNamingCheck::check(const
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;
}
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=284992&r1=284991&r2=284992&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 Oct 24 12:20:32 2016
@@ -95,14 +95,41 @@ SYSTEM_MACRO(var1);
USER_MACRO(var2);
// NO warnings or fixes expected as var2 is declared in a macro expansion
+#define BLA int FOO_bar
+BLA;
+// NO warnings or fixes expected as FOO_bar is from macro expansion
+
+int global0;
+#define USE_NUMBERED_GLOBAL(number) auto use_global##number = global##number
+USE_NUMBERED_GLOBAL(0);
+// NO warnings or fixes expected as global0 is pieced together in a macro
+// expansion.
+
+int global1;
+#define USE_NUMBERED_BAL(prefix, number) \
+ auto use_##prefix##bal##number = prefix##bal##number
+USE_NUMBERED_BAL(glo, 1);
+// NO warnings or fixes expected as global1 is pieced together in a macro
+// expansion.
+
+int global2;
+#define USE_RECONSTRUCTED(glo, bal) auto use_##glo##bal = glo##bal
+USE_RECONSTRUCTED(glo, bal2);
+// NO warnings or fixes expected as global2 is pieced together in a macro
+// expansion.
+
int global;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for global variable 'global'
+// CHECK-FIXES: {{^}}int g_global;{{$}}
#define USE_IN_MACRO(m) auto use_##m = m
USE_IN_MACRO(global);
-// NO warnings or fixes expected as global is used in a macro expansion
-#define BLA int FOO_bar
-BLA;
-// NO warnings or fixes expected as FOO_bar is from macro expansion
+int global3;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for global variable 'global3'
+// CHECK-FIXES: {{^}}int g_global3;{{$}}
+#define ADD_TO_SELF(m) (m) + (m)
+int g_twice_global3 = ADD_TO_SELF(global3);
+// CHECK-FIXES: {{^}}int g_twice_global3 = ADD_TO_SELF(g_global3);{{$}}
enum my_enumeration {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for enum 'my_enumeration'
More information about the cfe-commits
mailing list