[PATCH] D21020: [clang-tidy] readability-identifier-naming - Support for Macros

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.

================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:142
@@ +141,3 @@
+    SourceManager &SM(PP->getSourceManager());
+    if (!SM.isInMainFile(MacroNameTok.getLocation()))
+      return;
----------------
I'm not sure the check currently refuses to fix stuff defined in headers. It probably relies on -header-filter to limit its scope. It's intrinsically dangerous (since we should see all translation units using the entity to correctly rename it), but we should let users do this, in case they know what to do.

================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:152
@@ +151,3 @@
+                    SourceRange Range, const MacroArgs *Args) override {
+    (void)Range;
+    (void)Args;
----------------
No need to mute -Wunused _this_ way. Just comment out parameter names in the declaration.

================
Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:814
@@ +813,3 @@
+  SourceRange Range =
+      SourceRange(MacroNameTok.getLocation(), MacroNameTok.getEndLoc());
+  addUsage(NamingCheckFailures, ID, Range);
----------------
Remove ` = SourceRange`. Just `SourceRange Range(...);` is enough.

================
Comment at: clang-tidy/readability/IdentifierNamingCheck.h:86
@@ +85,3 @@
+
+  struct NamingCheckId : std::pair<SourceLocation, std::string> {
+    typedef std::pair<SourceLocation, std::string> Parent;
----------------
Maybe just typedef?

================
Comment at: clang-tidy/readability/IdentifierNamingCheck.h:88
@@ +87,3 @@
+    typedef std::pair<SourceLocation, std::string> Parent;
+    using Parent::Parent;
+  };
----------------
Delegating constructors don't work in VS2013, which LLVM should still support.


http://reviews.llvm.org/D21020