llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Carlos Galvez (carlosgalvezp) <details> <summary>Changes</summary> 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, #<!-- -->121969, #<!-- -->133453 --- Full diff: https://github.com/llvm/llvm-project/pull/133582.diff 12 Files Affected: - (modified) clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp (+19-17) - (modified) clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h (+4) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3) - (added) clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/inheritance/.clang-tidy (+1) - (added) clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/inheritance/foo.cpp (+3) - (added) clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/inheritance/foo.h (+1) - (added) clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/inheritance/subfolder/.clang-tidy (+2) - (added) clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/inheritance/subfolder/bar.cpp (+6) - (added) clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/inheritance/subfolder/bar.h (+1) - (added) clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/simple/.clang-tidy (+1) - (added) clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/simple/foo.cpp (+3) - (added) clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/simple/foo.h (+1) ``````````diff 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/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/inheritance/.clang-tidy b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/inheritance/.clang-tidy new file mode 100644 index 0000000000000..f4210353f94de --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/inheritance/.clang-tidy @@ -0,0 +1 @@ +HeaderFilterRegex: '.*' diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/inheritance/foo.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/inheritance/foo.cpp new file mode 100644 index 0000000000000..79622418d6dff --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/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/inheritance/foo.h b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/inheritance/foo.h new file mode 100644 index 0000000000000..f61d4c2923b50 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/inheritance/foo.h @@ -0,0 +1 @@ +struct X { X(int); }; diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/inheritance/subfolder/.clang-tidy b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/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/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/inheritance/subfolder/bar.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/inheritance/subfolder/bar.cpp new file mode 100644 index 0000000000000..7576b1af74bfa --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/inheritance/subfolder/bar.cpp @@ -0,0 +1,6 @@ +// 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/inheritance/subfolder/bar.h b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/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/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/simple/.clang-tidy b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/simple/.clang-tidy new file mode 100644 index 0000000000000..f4210353f94de --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/simple/.clang-tidy @@ -0,0 +1 @@ +HeaderFilterRegex: '.*' diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/simple/foo.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/simple/foo.cpp new file mode 100644 index 0000000000000..79622418d6dff --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/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/simple/foo.h b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/simple/foo.h new file mode 100644 index 0000000000000..f61d4c2923b50 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/header-filter-from-config/simple/foo.h @@ -0,0 +1 @@ +struct X { X(int); }; `````````` </details> https://github.com/llvm/llvm-project/pull/133582 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
