[clang-tools-extra] c7aa358 - [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"

[Salman Javed via cfe-commits]

Author: Salman Javed
Date: 2021-11-30T12:43:35+13:00
New Revision: c7aa358798e6330593fd5cc2ff4caf6bc15ba3c9

URL: https://github.com/llvm/llvm-project/commit/c7aa358798e6330593fd5cc2ff4caf6bc15ba3c9
DIFF: https://github.com/llvm/llvm-project/commit/c7aa358798e6330593fd5cc2ff4caf6bc15ba3c9.diff

LOG: [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"

Fixes https://bugs.llvm.org/show_bug.cgi?id=48613.

llvm-header-guard is suggesting header guards with leading underscores
if the header file path begins with a '/' or similar special character.
Only reserved identifiers should begin with an underscore.

Differential Revision: https://reviews.llvm.org/D114149

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
    clang-tools-extra/clang-tidy/utils/HeaderGuard.h
    clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp b/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
index 1cae618dfd093..36bf64110a774 100644
--- a/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
+++ b/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
@@ -164,10 +164,11 @@ class HeaderGuardPPCallbacks : public PPCallbacks {
                                          StringRef CurHeaderGuard,
                                          std::vector<FixItHint> &FixIts) {
     std::string CPPVar = Check->getHeaderGuard(FileName, CurHeaderGuard);
+    CPPVar = Check->sanitizeHeaderGuard(CPPVar);
     std::string CPPVarUnder = CPPVar + '_';
 
-    // Allow a trailing underscore iff we don't have to change the endif comment
-    // too.
+    // Allow a trailing underscore if and only if we don't have to change the
+    // endif comment too.
     if (Ifndef.isValid() && CurHeaderGuard != CPPVar &&
         (CurHeaderGuard != CPPVarUnder ||
          wouldFixEndifComment(FileName, EndIf, CurHeaderGuard))) {
@@ -216,6 +217,7 @@ class HeaderGuardPPCallbacks : public PPCallbacks {
         continue;
 
       std::string CPPVar = Check->getHeaderGuard(FileName);
+      CPPVar = Check->sanitizeHeaderGuard(CPPVar);
       std::string CPPVarUnder = CPPVar + '_'; // Allow a trailing underscore.
       // If there's a macro with a name that follows the header guard convention
       // but was not recognized by the preprocessor as a header guard there must
@@ -278,6 +280,11 @@ void HeaderGuardCheck::registerPPCallbacks(const SourceManager &SM,
   PP->addPPCallbacks(std::make_unique<HeaderGuardPPCallbacks>(PP, this));
 }
 
+std::string HeaderGuardCheck::sanitizeHeaderGuard(StringRef Guard) {
+  // Only reserved identifiers are allowed to start with an '_'.
+  return Guard.drop_while([](char C) { return C == '_'; }).str();
+}
+
 bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
   return utils::isFileExtension(FileName, HeaderFileExtensions);
 }

diff  --git a/clang-tools-extra/clang-tidy/utils/HeaderGuard.h b/clang-tools-extra/clang-tidy/utils/HeaderGuard.h
index 86c0f3dc8fda2..584a09c756d49 100644
--- a/clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ b/clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -38,6 +38,9 @@ class HeaderGuardCheck : public ClangTidyCheck {
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
                            Preprocessor *ModuleExpanderPP) override;
 
+  /// Ensure that the provided header guard is a non-reserved identifier.
+  std::string sanitizeHeaderGuard(StringRef Guard);
+
   /// Returns ``true`` if the check should suggest inserting a trailing comment
   /// on the ``#endif`` of the header guard. It will use the same name as
   /// returned by ``HeaderGuardCheck::getHeaderGuard``.

diff  --git a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
index 6b312413b870d..2a3fae05f585f 100644
--- a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -219,6 +219,16 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
                 "", "/llvm-project/clang-tools-extra/clangd/foo.h",
                 StringRef("header is missing header guard")));
 
+  // Substitution of characters should not result in a header guard starting
+  // with "_".
+  EXPECT_EQ("#ifndef BAR_H\n"
+            "#define BAR_H\n"
+            "\n"
+            "\n"
+            "#endif\n",
+            runHeaderGuardCheck("", "include/--bar.h",
+                                StringRef("header is missing header guard")));
+
 #ifdef WIN32
   // Check interaction with Windows-style path separators (\).
   EXPECT_EQ(