[clang-tidy] Skip system macros in readability-identifier-naming check

[carlosgalvezp]

Currently, the check is processing system macros. Most importantly, it tries to find .clang-tidy files associated with those files, if the option GetConfigPerFile (on by default) is active.

This is problematic for a number of reasons:

• System macros cannot be acted upon (renamed), so it's wasted work.
• When the main .cpp file includes a system header, clang-tidy tries to find the .clang-tidy file for that system header. When that system header is a 3rd-party repository, they may have their own .clang-tidy file, which may not be compatible with the current version of clang-tidy. So, clang-tidy may fail to analyze our main.cpp file, only because it includes a 3rd-party system header whose .clang-tidy file is incompatible with our clang-tidy binary.

Therefore, skip system macros in this check.

[llvmbot]

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Carlos Galvez (carlosgalvezp)

Changes

Currently, the check is processing system macros. Most importantly, it tries to find .clang-tidy files associated with those files, if the option GetConfigPerFile (on by default) is active.

This is problematic for a number of reasons:

• System macros cannot be acted upon (renamed), so it's wasted work.
• When the main .cpp file includes a system header, clang-tidy tries to find the .clang-tidy file for that system header. When that system header is a 3rd-party repository, they may have their own .clang-tidy file, which may not be compatible with the current version of clang-tidy. So, clang-tidy may fail to analyze our main.cpp file, only because it includes a 3rd-party system header whose .clang-tidy file is incompatible with our clang-tidy binary.

Therefore, skip system macros in this check.

---

Full diff: https://github.com/llvm/llvm-project/pull/132016.diff

1 Files Affected:

• (modified) clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp (+2)

diff --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
index 9104723c7f1c0..59e11ca51a0ae 100644
--- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -194,6 +194,8 @@ class RenamerClangTidyCheckPPCallbacks : public PPCallbacks {
       return;
     if (SM.isWrittenInCommandLineFile(MacroNameTok.getLocation()))
       return;
+    if (SM.isInSystemHeader(MacroNameTok.getLocation()))
+      return;
     Check->checkMacro(MacroNameTok, Info, SM);
   }
 

[carlosgalvezp]

Maybe we do want to enable processing the macro if SystemHeaders is active?

[PiotrZSL]

"Maybe we do want to enable processing the macro if SystemHeaders is active?"
We could, but also RenamerClangTidyVisitor could be improved to skip system code if not enabled.

LGTM

[carlosgalvezp]

We could, but also RenamerClangTidyVisitor could be improved to skip system code if not enabled.

I checked briefly and it does not seem to process system declarations already now, it was just the macros that were missing.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.