[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