[PATCH] D113450: [clang-tidy] Fix llvm-header-guard so that it works with Windows paths

Salman Javed via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 9 04:52:03 PST 2021


salman-javed-nz updated this revision to Diff 385780.
salman-javed-nz added a comment.

Unit tests:

- Renamed Samba to SMB
- Added test for UNC paths


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113450/new/

https://reviews.llvm.org/D113450

Files:
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.cpp
  clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp


Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -9,8 +9,6 @@
 namespace tidy {
 namespace test {
 
-// FIXME: It seems this might be incompatible to dos path. Investigating.
-#if !defined(_WIN32)
 static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
                                        Optional<StringRef> ExpectedWarning) {
   std::vector<ClangTidyError> Errors;
@@ -220,8 +218,47 @@
             runHeaderGuardCheck(
                 "", "/llvm-project/clang-tools-extra/clangd/foo.h",
                 StringRef("header is missing header guard")));
-}
+
+#ifdef WIN32
+  // Check interaction with Windows-style path separators (\).
+  EXPECT_EQ(
+      "#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FOO_H\n"
+      "#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FOO_H\n"
+      "\n"
+      "\n"
+      "#endif\n",
+      runHeaderGuardCheck("", "llvm-project\\clang-tools-extra\\clangd\\foo.h",
+                          StringRef("header is missing header guard")));
+
+  EXPECT_EQ("#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FOO_H\n"
+            "#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FOO_H\n"
+            "\n"
+            "\n"
+            "#endif\n",
+            runHeaderGuardCheck(
+                "", "C:\\llvm-project\\clang-tools-extra\\clangd\\foo.h",
+                StringRef("header is missing header guard")));
+
+  EXPECT_EQ("#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FOO_H\n"
+            "#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FOO_H\n"
+            "\n"
+            "\n"
+            "#endif\n",
+            runHeaderGuardCheck(
+                "",
+                "\\\\SMBShare\\llvm-project\\clang-tools-extra\\clangd\\foo.h",
+                StringRef("header is missing header guard")));
+
+  EXPECT_EQ("#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FOO_H\n"
+            "#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FOO_H\n"
+            "\n"
+            "\n"
+            "#endif\n",
+            runHeaderGuardCheck(
+                "", "\\\\?\\C:\\llvm-project\\clang-tools-extra\\clangd\\foo.h",
+                StringRef("header is missing header guard")));
 #endif
+}
 
 } // namespace test
 } // namespace tidy
Index: clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.cpp
+++ clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "HeaderGuardCheck.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Path.h"
 
 namespace clang {
 namespace tidy {
@@ -21,6 +22,10 @@
                                                  StringRef OldGuard) {
   std::string Guard = tooling::getAbsolutePath(Filename);
 
+  // When running under Windows, need to convert the path separators from
+  // `\` to `/`.
+  Guard = llvm::sys::path::convert_to_slash(Guard);
+
   // Sanitize the path. There are some rules for compatibility with the historic
   // style in include/llvm and include/clang which we want to preserve.
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D113450.385780.patch
Type: text/x-patch
Size: 3222 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211109/b9878ba2/attachment-0001.bin>


More information about the cfe-commits mailing list