This revision was automatically updated to reflect the committed changes.
jhen marked an inline comment as done.
Closed by commit rL284992: [clang-tidy] Fix identifier naming in macro args. 
(authored by jhen).

Changed prior to commit:
  https://reviews.llvm.org/D25450?vs=75430&id=75610#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D25450

Files:
  clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp

Index: clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp
===================================================================
--- clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming.cpp
@@ -95,14 +95,41 @@
 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'
Index: clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
===================================================================
--- clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/trunk/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;
+
+  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 @@
 
   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

Reply via email to