[llvm-branch-commits] [clang-tools-extra] [clang-tidy] Normalize windows line filter paths (#217038) (PR #221500)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sat Sep 5 15:21:46 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Chris Apple (cjappl)
<details>
<summary>Changes</summary>
Fixes #<!-- -->221459
Back port of PR: #<!-- -->217038
Original issue: #<!-- -->112038
This is a cherry-pick of the original commit, but the release note had to be moved to the RST file (on main it is markdown).
No changes to source or test were needed in the cherry-pick
---
`git | clang-tidy-diff.py` emits line-filter paths with forward slashes, while Windows compilation databases use a mix of forwards and backwards slashes. These paths are then silently rejected (and no report is emitted).
Reproduce on windows via:
```
git.exe diff -U0 main -- . | python clang-tidy-diff.py -p1 -path .\build"
```
Git will produce a path like:
```
my/source/file.cpp
```
The compilation database has files with entries like this:
```
"file": "C:\\dev\\repo\\my\\source\\file.cpp",
"output": "my\\source\\CMakeFiles\\source.dir\\file.cpp.obj"
```
Clang tidy tries to match these paths, but fails on the slash comparison.
Do a few things to fix this:
* Normalize filter paths with `llvm::sys::path::convert_to_slash()` before matching the compilation database. This is a no-op on posix, but converts windows forward slashes to backwards.
* Normalize `Filter.Name` paths when parsed
* Write a couple more unit tests for windows specific paths that expose this behavior
* Enable another UNSUPPORTED test for windows clang-tidy-diff (and make it run in a nested directory to exercise the issue
Fixes https://github.com/llvm/llvm-project/issues/112038.
---
Full diff: https://github.com/llvm/llvm-project/pull/221500.diff
7 Files Affected:
- (modified) clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp (+4-1)
- (modified) clang-tools-extra/clang-tidy/ClangTidyOptions.cpp (+6-1)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
- (modified) clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp (+8-8)
- (modified) clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp (+70)
- (modified) clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp (+10)
- (modified) clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h (+10-10)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 88d0a433bc7fb..c2c96a9f3161a 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -36,6 +36,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/Path.h"
#include "llvm/Support/Regex.h"
#include <optional>
#include <tuple>
@@ -484,8 +485,10 @@ bool ClangTidyDiagnosticConsumer::passesLineFilter(StringRef FileName,
unsigned LineNumber) const {
if (Context.getGlobalOptions().LineFilter.empty())
return true;
+ const std::string NormalizedFileName =
+ llvm::sys::path::convert_to_slash(FileName);
for (const FileFilter &Filter : Context.getGlobalOptions().LineFilter) {
- if (FileName.ends_with(Filter.Name)) {
+ if (StringRef(NormalizedFileName).ends_with(Filter.Name)) {
if (Filter.LineRanges.empty())
return true;
return llvm::any_of(
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
index 0a0f392346f6d..ca2ff2fcf1716 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -531,7 +531,12 @@ std::error_code parseLineFilter(StringRef LineFilter,
clang::tidy::ClangTidyGlobalOptions &Options) {
llvm::yaml::Input Input(LineFilter);
Input >> Options.LineFilter;
- return Input.error();
+ if (const std::error_code &Error = Input.error())
+ return Error;
+
+ for (FileFilter &Filter : Options.LineFilter)
+ Filter.Name = llvm::sys::path::convert_to_slash(Filter.Name);
+ return {};
}
llvm::ErrorOr<ClangTidyOptions>
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 697f13f99f617..c58e57e1e99d4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -322,6 +322,11 @@ Improvements to clang-tidy
checks are `clang-diagnostic-*` ones. This allows using
:program:`clang-tidy` purely as a frontend to Clang's builtin warnings.
+- On Windows, :program:`clang-tidy` will now better match mixes of forwards and
+ backwards slashes when using the `--line-filter` option. This also improves
+ piping from :program:`git` to :program:`clang-tidy-diff.py`, where slashes
+ will now be automatically normalized.
+
New checks
^^^^^^^^^^
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp
index 43b7d781e6a7a..a5311adaaf190 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp
@@ -1,13 +1,13 @@
-// UNSUPPORTED: system-windows
-// RUN: sed 's/placeholder_for_f/f/' %s > %t.cpp
-// RUN: clang-tidy -checks=-*,modernize-use-override %t.cpp -- -std=c++11 | FileCheck -check-prefix=CHECK-SANITY %s
-// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -- -std=c++11 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-JMAX
-// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -quiet -- -std=c++11 2>&1 | FileCheck -check-prefix=CHECK-QUIET %s
+// RUN: mkdir -p %t.dir/src
+// RUN: sed 's/placeholder_for_f/f/' %s > %t.dir/src/test.cpp
+// RUN: clang-tidy -checks=-*,modernize-use-override %t.dir/src/test.cpp -- -std=c++11 | FileCheck -check-prefix=CHECK-SANITY %s
+// RUN: not diff -U0 %s %t.dir/src/test.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -- -std=c++11 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-JMAX
+// RUN: not diff -U0 %s %t.dir/src/test.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -quiet -- -std=c++11 2>&1 | FileCheck -check-prefix=CHECK-QUIET %s
// RUN: mkdir -p %t.dir/compilation-database-test/
-// RUN: echo '[{"directory": "%t.dir", "command": "clang++ -o test.o -std=c++11 %t.cpp", "file": "%t.cpp"}]' > %t.dir/compilation-database-test/compile_commands.json
-// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -path %t.dir/compilation-database-test 2>&1 | FileCheck -check-prefix=CHECK %s
+// RUN: echo '[{"directory": "%/t.dir", "command": "clang++ -o test.o -std=c++11 %/t.dir/src/test.cpp", "file": "%/t.dir/src/test.cpp"}]' > %t.dir/compilation-database-test/compile_commands.json
+// RUN: not diff -U0 %s %t.dir/src/test.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -path %t.dir/compilation-database-test 2>&1 | FileCheck -check-prefix=CHECK %s
-// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -j 1 -- -std=c++11 2>&1 | FileCheck %s --check-prefix=CHECK-J1
+// RUN: not diff -U0 %s %t.dir/src/test.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -j 1 -- -std=c++11 2>&1 | FileCheck %s --check-prefix=CHECK-J1
// CHECK-J1: Running clang-tidy in 1 threads...
struct A {
virtual void f() {}
diff --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp b/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
index 11c9ac9928221..39b86c7d62ef7 100644
--- a/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -111,6 +111,76 @@ TEST(ClangTidyDiagnosticConsumer, InvalidSourceLocationRangesIgnored) {
EXPECT_EQ(1ul, Errors[3].Message.Ranges.size());
}
+#ifdef _WIN32
+
+namespace {
+class LineFilterTestCheck : public ClangTidyCheck {
+public:
+ LineFilterTestCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override {
+ Finder->addMatcher(ast_matchers::varDecl().bind("var"), this);
+ }
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override {
+ diag(Result.Nodes.getNodeAs<VarDecl>("var")->getLocation(), "variable");
+ }
+};
+} // namespace
+
+TEST(ClangTidyDiagnosticConsumer, LineFilterNormalizesWindowsPaths) {
+ const auto ParsedDiagnosticsFor = [](StringRef FileName,
+ StringRef LineFilter) {
+ ClangTidyGlobalOptions GlobalOptions;
+ EXPECT_FALSE(parseLineFilter(LineFilter, GlobalOptions));
+ std::vector<ClangTidyError> Errors;
+ runCheckOnCode<LineFilterTestCheck>("int x;", &Errors, FileName, {},
+ ClangTidyOptions(), {}, GlobalOptions);
+ return Errors;
+ };
+ const auto DiagnosticsFor =
+ [](StringRef FileName, StringRef FilterName,
+ std::vector<FileFilter::LineRange> LineRanges = {}) {
+ ClangTidyGlobalOptions GlobalOptions;
+ GlobalOptions.LineFilter.push_back(
+ {FilterName.str(), std::move(LineRanges)});
+ std::vector<ClangTidyError> Errors;
+ runCheckOnCode<LineFilterTestCheck>("int x;", &Errors, FileName, {},
+ ClangTidyOptions(), {},
+ GlobalOptions);
+ return Errors;
+ };
+
+ EXPECT_EQ(1u, ParsedDiagnosticsFor(R"(C:\root\project\src\input.cc)",
+ R"([{"name":"project/src/input.cc"}])")
+ .size());
+ EXPECT_EQ(1u, ParsedDiagnosticsFor(R"(C:\root\project\src\input.cc)",
+ R"([{"name":"project\\src\\input.cc"}])")
+ .size());
+ EXPECT_EQ(1u, ParsedDiagnosticsFor("C:/root/project/src/input.cc",
+ R"([{"name":"project/src/input.cc"}])")
+ .size());
+ EXPECT_EQ(1u, ParsedDiagnosticsFor("C:/root/project/src/input.cc",
+ R"([{"name":"project\\src\\input.cc"}])")
+ .size());
+
+ EXPECT_EQ(
+ 1u, DiagnosticsFor(R"(C:\root\project\src\input.cc)", "input.cc").size());
+ EXPECT_EQ(
+ 1u,
+ DiagnosticsFor(R"(C:\root\project\src\input.cc)", "src/input.cc").size());
+ EXPECT_TRUE(
+ DiagnosticsFor(R"(C:\root\project\src\input.cc)", "other/src/input.cc")
+ .empty());
+
+ EXPECT_EQ(1u, DiagnosticsFor(R"(C:\root\project\src\input.cc)",
+ "project/src/input.cc", {{1, 1}})
+ .size());
+ EXPECT_TRUE(DiagnosticsFor(R"(C:\root\project\src\input.cc)",
+ "project/src/input.cc", {{2, 2}})
+ .empty());
+}
+#endif
+
} // namespace test
} // namespace tidy
} // namespace clang
diff --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
index 3f86f65c1ce65..d5725609f367f 100644
--- a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -75,6 +75,16 @@ TEST(ParseLineFilter, ValidFilter) {
EXPECT_EQ(1000u, Options.LineFilter[2].LineRanges[0].second);
}
+#ifdef _WIN32
+TEST(ParseLineFilter, NormalizesWindowsPathSeparators) {
+ ClangTidyGlobalOptions Options;
+ EXPECT_FALSE(
+ parseLineFilter(R"([{"name":"project\\src\\input.cc"}])", Options));
+ ASSERT_EQ(1u, Options.LineFilter.size());
+ EXPECT_EQ("project/src/input.cc", Options.LineFilter[0].Name);
+}
+#endif // _WIN32
+
TEST(ParseConfiguration, ValidConfiguration) {
llvm::ErrorOr<ClangTidyOptions> Options =
parseConfiguration(llvm::MemoryBufferRef(
diff --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h b/clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
index 195d6a6c60918..040388338bc95 100644
--- a/clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
+++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
@@ -83,19 +83,19 @@ class TestClangTidyAction : public ASTFrontendAction {
};
template <typename... CheckTypes>
-std::string
-runCheckOnCode(StringRef Code, std::vector<ClangTidyError> *Errors = nullptr,
- const Twine &Filename = "input.cc",
- ArrayRef<std::string> ExtraArgs = {},
- const ClangTidyOptions &ExtraOptions = ClangTidyOptions(),
- std::map<StringRef, StringRef> PathsToContent =
- std::map<StringRef, StringRef>()) {
+std::string runCheckOnCode(
+ StringRef Code, std::vector<ClangTidyError> *Errors = nullptr,
+ const Twine &Filename = "input.cc", ArrayRef<std::string> ExtraArgs = {},
+ const ClangTidyOptions &ExtraOptions = ClangTidyOptions(),
+ std::map<StringRef, StringRef> PathsToContent =
+ std::map<StringRef, StringRef>(),
+ const ClangTidyGlobalOptions &GlobalOptions = ClangTidyGlobalOptions()) {
static_assert(sizeof...(CheckTypes) > 0, "No checks specified");
ClangTidyOptions Options = ExtraOptions;
Options.Checks = "*";
- ClangTidyContext Context(std::make_unique<DefaultOptionsProvider>(
- ClangTidyGlobalOptions(), Options),
- false, false, false);
+ ClangTidyContext Context(
+ std::make_unique<DefaultOptionsProvider>(GlobalOptions, Options), false,
+ false, false);
ClangTidyDiagnosticConsumer DiagConsumer(Context);
auto DiagOpts = std::make_unique<DiagnosticOptions>();
DiagnosticsEngine DE(DiagnosticIDs::create(), *DiagOpts, &DiagConsumer,
``````````
</details>
https://github.com/llvm/llvm-project/pull/221500
More information about the llvm-branch-commits
mailing list