Author: Carlos Galvez Date: 2025-04-03T09:28:34+02:00 New Revision: 6333fa5160fbde4bd2cf6afe8856695c13ab621f
URL: https://github.com/llvm/llvm-project/commit/6333fa5160fbde4bd2cf6afe8856695c13ab621f DIFF: https://github.com/llvm/llvm-project/commit/6333fa5160fbde4bd2cf6afe8856695c13ab621f.diff LOG: [clang-tidy] Fix broken HeaderFilterRegex when read from config file (#133582) 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.gal...@zenseact.com> Added: clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/.clang-tidy clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/foo.cpp clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/foo.h clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/subfolder/.clang-tidy clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/subfolder/bar.cpp clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/inheritance/subfolder/bar.h clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/simple/.clang-tidy clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/simple/foo.cpp clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config-file/simple/foo.h Modified: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h clang-tools-extra/clang-tidy/ClangTidyOptions.cpp clang-tools-extra/docs/ReleaseNotes.rst Removed: ################################################################################ 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 6cb8d572d3a78..6c1f05009df98 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -94,6 +94,9 @@ Improvements to clang-tidy - Improved :program:`clang-tidy- diff .py` script. Add the `-warnings-as-errors` argument to treat warnings as errors. +- 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); }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
