[llvm-branch-commits] [clang-tools-extra] release/20.x: [clang-tidy] Fix broken HeaderFilterRegex when read from config file (#133582) (PR #134215)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Fri Apr 11 09:45:14 PDT 2025
https://github.com/llvmbot updated https://github.com/llvm/llvm-project/pull/134215
>From 19c2e1c12d47cd6f64db16eb15dd916bf1b76984 Mon Sep 17 00:00:00 2001
From: Carlos Galvez <carlosgalvezp at gmail.com>
Date: Thu, 3 Apr 2025 09:28:34 +0200
Subject: [PATCH] [clang-tidy] Fix broken HeaderFilterRegex when read from
config file (#133582)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
PR https://github.com/llvm/llvm-project/pull/91400 broke the usage of
HeaderFilterRegex via config file, because it is now created at a
different point in the execution and leads to a different value.
The result of that is that using HeaderFilterRegex only in the config
file does NOT work, in other words clang-tidy stops triggering warnings
on header files, thereby losing a lot of coverage.
This patch reverts the logic so that the header filter is created upon
calling the getHeaderFilter() function.
Additionally, this patch adds 2 unit tests to prevent regressions in the
future:
- One of them, "simple", tests the most basic use case with a single
top-level .clang-tidy file.
- The second one, "inheritance", demonstrates that the subfolder only
gets warnings from headers within it, and not from parent headers.
Fixes #118009
Fixes #121969
Fixes #133453
Co-authored-by: Carlos Gálvez <carlos.galvez at zenseact.com>
(cherry picked from commit 6333fa5160fbde4bd2cf6afe8856695c13ab621f)
---
.../ClangTidyDiagnosticConsumer.cpp | 36 ++++++++++---------
.../clang-tidy/ClangTidyDiagnosticConsumer.h | 4 +++
.../clang-tidy/ClangTidyOptions.cpp | 4 +--
clang-tools-extra/docs/ReleaseNotes.rst | 3 ++
.../inheritance/.clang-tidy | 1 +
.../inheritance/foo.cpp | 3 ++
.../inheritance/foo.h | 1 +
.../inheritance/subfolder/.clang-tidy | 2 ++
.../inheritance/subfolder/bar.cpp | 8 +++++
.../inheritance/subfolder/bar.h | 1 +
.../simple/.clang-tidy | 1 +
.../simple/foo.cpp | 3 ++
.../simple/foo.h | 1 +
13 files changed, 49 insertions(+), 19 deletions(-)
create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/.clang-tidy
create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/foo.cpp
create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/foo.h
create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/subfolder/.clang-tidy
create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/subfolder/bar.cpp
create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/subfolder/bar.h
create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/simple/.clang-tidy
create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/simple/foo.cpp
create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/simple/foo.h
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 4c75b42270114..71e852545203e 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -311,18 +311,7 @@ ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(
: Context(Ctx), ExternalDiagEngine(ExternalDiagEngine),
RemoveIncompatibleErrors(RemoveIncompatibleErrors),
GetFixesFromNotes(GetFixesFromNotes),
- EnableNolintBlocks(EnableNolintBlocks) {
-
- if (Context.getOptions().HeaderFilterRegex &&
- !Context.getOptions().HeaderFilterRegex->empty())
- HeaderFilter =
- std::make_unique<llvm::Regex>(*Context.getOptions().HeaderFilterRegex);
-
- if (Context.getOptions().ExcludeHeaderFilterRegex &&
- !Context.getOptions().ExcludeHeaderFilterRegex->empty())
- ExcludeHeaderFilter = std::make_unique<llvm::Regex>(
- *Context.getOptions().ExcludeHeaderFilterRegex);
-}
+ EnableNolintBlocks(EnableNolintBlocks) {}
void ClangTidyDiagnosticConsumer::finalizeLastError() {
if (!Errors.empty()) {
@@ -571,17 +560,30 @@ void ClangTidyDiagnosticConsumer::checkFilters(SourceLocation Location,
}
StringRef FileName(File->getName());
- LastErrorRelatesToUserCode =
- LastErrorRelatesToUserCode || Sources.isInMainFile(Location) ||
- (HeaderFilter &&
- (HeaderFilter->match(FileName) &&
- !(ExcludeHeaderFilter && ExcludeHeaderFilter->match(FileName))));
+ LastErrorRelatesToUserCode = LastErrorRelatesToUserCode ||
+ Sources.isInMainFile(Location) ||
+ (getHeaderFilter()->match(FileName) &&
+ !getExcludeHeaderFilter()->match(FileName));
unsigned LineNumber = Sources.getExpansionLineNumber(Location);
LastErrorPassesLineFilter =
LastErrorPassesLineFilter || passesLineFilter(FileName, LineNumber);
}
+llvm::Regex *ClangTidyDiagnosticConsumer::getHeaderFilter() {
+ if (!HeaderFilter)
+ HeaderFilter =
+ std::make_unique<llvm::Regex>(*Context.getOptions().HeaderFilterRegex);
+ return HeaderFilter.get();
+}
+
+llvm::Regex *ClangTidyDiagnosticConsumer::getExcludeHeaderFilter() {
+ if (!ExcludeHeaderFilter)
+ ExcludeHeaderFilter = std::make_unique<llvm::Regex>(
+ *Context.getOptions().ExcludeHeaderFilterRegex);
+ return ExcludeHeaderFilter.get();
+}
+
void ClangTidyDiagnosticConsumer::removeIncompatibleErrors() {
// Each error is modelled as the set of intervals in which it applies
// replacements. To detect overlapping replacements, we use a sweep line
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
index ff42f96a0477b..d6cf6a2b2731e 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -302,6 +302,10 @@ class ClangTidyDiagnosticConsumer : public DiagnosticConsumer {
/// context.
llvm::Regex *getHeaderFilter();
+ /// Returns the \c ExcludeHeaderFilter constructed for the options set in the
+ /// context.
+ llvm::Regex *getExcludeHeaderFilter();
+
/// Updates \c LastErrorRelatesToUserCode and LastErrorPassesLineFilter
/// according to the diagnostic \p Location.
void checkFilters(SourceLocation Location, const SourceManager &Sources);
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
index 8bac6f161fa05..dd1d86882f5d4 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -194,8 +194,8 @@ ClangTidyOptions ClangTidyOptions::getDefaults() {
Options.WarningsAsErrors = "";
Options.HeaderFileExtensions = {"", "h", "hh", "hpp", "hxx"};
Options.ImplementationFileExtensions = {"c", "cc", "cpp", "cxx"};
- Options.HeaderFilterRegex = std::nullopt;
- Options.ExcludeHeaderFilterRegex = std::nullopt;
+ Options.HeaderFilterRegex = "";
+ Options.ExcludeHeaderFilterRegex = "";
Options.SystemHeaders = false;
Options.FormatStyle = "none";
Options.User = std::nullopt;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 316ac1743ccb7..2ab597eb37048 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -187,6 +187,9 @@ Improvements to clang-tidy
:doc:`readability-redundant-access-specifiers <clang-tidy/checks/readability/redundant-access-specifiers>`, CheckFirstDeclaration
:doc:`readability-redundant-casting <clang-tidy/checks/readability/redundant-casting>`, IgnoreTypeAliases
+- Fixed bug in :program:`clang-tidy` by which `HeaderFilterRegex` did not take
+ effect when passed via the `.clang-tidy` file.
+
New checks
^^^^^^^^^^
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/.clang-tidy b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/.clang-tidy
new file mode 100644
index 0000000000000..f4210353f94de
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/.clang-tidy
@@ -0,0 +1 @@
+HeaderFilterRegex: '.*'
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/foo.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/foo.cpp
new file mode 100644
index 0000000000000..5828c2cafaf7d
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/foo.cpp
@@ -0,0 +1,3 @@
+// RUN: clang-tidy -checks=-*,google-explicit-constructor %s 2>&1 | FileCheck %s
+#include "foo.h"
+// CHECK: foo.h:1:12: warning: single-argument constructors must be marked explicit
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/foo.h b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/foo.h
new file mode 100644
index 0000000000000..f61d4c2923b50
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/foo.h
@@ -0,0 +1 @@
+struct X { X(int); };
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/subfolder/.clang-tidy b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/subfolder/.clang-tidy
new file mode 100644
index 0000000000000..96706c1428047
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/subfolder/.clang-tidy
@@ -0,0 +1,2 @@
+InheritParentConfig: true
+HeaderFilterRegex: 'subfolder/.*'
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/subfolder/bar.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/subfolder/bar.cpp
new file mode 100644
index 0000000000000..229ba52e2695a
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/subfolder/bar.cpp
@@ -0,0 +1,8 @@
+// shell is required for the "dirname" command
+// REQUIRES: shell
+// RUN: clang-tidy -checks=-*,google-explicit-constructor %s -- -I "$(dirname %S)" 2>&1 | FileCheck %s
+#include "foo.h"
+// CHECK-NOT: foo.h:1:12: warning: single-argument constructors must be marked explicit
+
+#include "bar.h"
+// CHECK: bar.h:1:13: warning: single-argument constructors must be marked explicit
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/subfolder/bar.h b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/subfolder/bar.h
new file mode 100644
index 0000000000000..ee12d00d334dd
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/subfolder/bar.h
@@ -0,0 +1 @@
+struct XX { XX(int); };
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/simple/.clang-tidy b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/simple/.clang-tidy
new file mode 100644
index 0000000000000..f4210353f94de
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/simple/.clang-tidy
@@ -0,0 +1 @@
+HeaderFilterRegex: '.*'
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/simple/foo.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/simple/foo.cpp
new file mode 100644
index 0000000000000..5828c2cafaf7d
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/simple/foo.cpp
@@ -0,0 +1,3 @@
+// RUN: clang-tidy -checks=-*,google-explicit-constructor %s 2>&1 | FileCheck %s
+#include "foo.h"
+// CHECK: foo.h:1:12: warning: single-argument constructors must be marked explicit
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/simple/foo.h b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/simple/foo.h
new file mode 100644
index 0000000000000..f61d4c2923b50
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/simple/foo.h
@@ -0,0 +1 @@
+struct X { X(int); };
More information about the llvm-branch-commits
mailing list