jhen created this revision.
jhen added a reviewer: alexfh.
jhen added a subscriber: cfe-commits.
Herald added a subscriber: jlebar.
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);
https://reviews.llvm.org/D25450
Files:
clang-tidy/readability/IdentifierNamingCheck.cpp
test/clang-tidy/readability-identifier-naming.cpp
Index: test/clang-tidy/readability-identifier-naming.cpp
===================================================================
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -95,15 +95,35 @@
USER_MACRO(var2);
// NO warnings or fixes expected as var2 is declared in a macro expansion
-int 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 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);
+
enum my_enumeration {
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for enum 'my_enumeration'
// CHECK-FIXES: {{^}}enum EMyEnumeration {{{$}}
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===================================================================
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -612,7 +612,7 @@
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;
@@ -623,16 +623,39 @@
if (!Failure.RawUsageLocs.insert(Range.getBegin().getRawEncoding()).second)
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;
+ if (!RangeIsEntirelyWithinMacroArgument)
+ for (SourceLocation Loc = Range.getBegin(); Loc.isValid();
+ Loc = Loc.getLocWithOffset(1)) {
+ RangeContainsMacroExpansion =
+ RangeContainsMacroExpansion || Loc.isMacroID();
+ if (Loc == Range.getEnd())
+ break;
+ }
+
+ bool RangeCanBeFixed =
+ RangeIsEntirelyWithinMacroArgument || !RangeContainsMacroExpansion;
+ Failure.ShouldFix = 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 +742,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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
