Add option to exclude headers from clang-tidy analysis #91400
Conversation
|
@llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang-tools-extra Author: Justin Cady (justincady) ChangesThis is a renewed attempt to land @toddlipcon's D34654. The comments on After considering various options, including migrating to std::regex, I This functionality is useful for improving performance when analyzing a Full diff: https://github.com/llvm/llvm-project/pull/91400.diff 6 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index de2a3b51422a5..3cde0d2d68874 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -564,7 +564,8 @@ void ClangTidyDiagnosticConsumer::checkFilters(SourceLocation Location,
StringRef FileName(File->getName());
LastErrorRelatesToUserCode = LastErrorRelatesToUserCode ||
Sources.isInMainFile(Location) ||
- getHeaderFilter()->match(FileName);
+ (getHeaderFilter()->match(FileName) &&
+ !getExcludeHeaderFilter()->match(FileName));
unsigned LineNumber = Sources.getExpansionLineNumber(Location);
LastErrorPassesLineFilter =
@@ -578,6 +579,13 @@ llvm::Regex *ClangTidyDiagnosticConsumer::getHeaderFilter() {
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 9280eb1e1f218..a3add5d52778d 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -299,6 +299,10 @@ class ClangTidyDiagnosticConsumer : public DiagnosticConsumer {
/// context.
llvm::Regex *getHeaderFilter();
+ /// \brief 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);
@@ -313,6 +317,7 @@ class ClangTidyDiagnosticConsumer : public DiagnosticConsumer {
bool EnableNolintBlocks;
std::vector<ClangTidyError> Errors;
std::unique_ptr<llvm::Regex> HeaderFilter;
+ std::unique_ptr<llvm::Regex> ExcludeHeaderFilter;
bool LastErrorRelatesToUserCode = false;
bool LastErrorPassesLineFilter = false;
bool LastErrorWasIgnored = false;
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
index cbf21a0e2ae34..254ce7fc60fc9 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -170,6 +170,8 @@ template <> struct MappingTraits<ClangTidyOptions> {
IO.mapOptional("ImplementationFileExtensions",
Options.ImplementationFileExtensions);
IO.mapOptional("HeaderFilterRegex", Options.HeaderFilterRegex);
+ IO.mapOptional("ExcludeHeaderFilterRegex",
+ Options.ExcludeHeaderFilterRegex);
IO.mapOptional("FormatStyle", Options.FormatStyle);
IO.mapOptional("User", Options.User);
IO.mapOptional("CheckOptions", Options.CheckOptions);
@@ -192,6 +194,7 @@ ClangTidyOptions ClangTidyOptions::getDefaults() {
Options.HeaderFileExtensions = {"", "h", "hh", "hpp", "hxx"};
Options.ImplementationFileExtensions = {"c", "cc", "cpp", "cxx"};
Options.HeaderFilterRegex = "";
+ Options.ExcludeHeaderFilterRegex = "";
Options.SystemHeaders = false;
Options.FormatStyle = "none";
Options.User = std::nullopt;
@@ -231,6 +234,7 @@ ClangTidyOptions &ClangTidyOptions::mergeWith(const ClangTidyOptions &Other,
overrideValue(ImplementationFileExtensions,
Other.ImplementationFileExtensions);
overrideValue(HeaderFilterRegex, Other.HeaderFilterRegex);
+ overrideValue(ExcludeHeaderFilterRegex, Other.ExcludeHeaderFilterRegex);
overrideValue(SystemHeaders, Other.SystemHeaders);
overrideValue(FormatStyle, Other.FormatStyle);
overrideValue(User, Other.User);
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.h b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
index e7636cb5d9b06..85d5a02ebbc1b 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
@@ -83,6 +83,10 @@ struct ClangTidyOptions {
/// main files will always be displayed.
std::optional<std::string> HeaderFilterRegex;
+ /// \brief Exclude warnings from headers matching this filter, even if they
+ /// match \c HeaderFilterRegex.
+ std::optional<std::string> ExcludeHeaderFilterRegex;
+
/// Output warnings from system headers matching \c HeaderFilterRegex.
std::optional<bool> SystemHeaders;
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index f82f4417141d3..88c69a9991d26 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -132,6 +132,19 @@ option in .clang-tidy file, if any.
cl::init(""),
cl::cat(ClangTidyCategory));
+static cl::opt<std::string> ExcludeHeaderFilter("exclude-header-filter",
+ cl::desc(R"(
+Regular expression matching the names of the
+headers to exclude diagnostics from. Diagnostics
+from the main file of each translation unit are
+always displayed.
+Can be used together with -line-filter.
+This option overrides the 'ExcludeHeaderFilterRegex'
+option in .clang-tidy file, if any.
+)"),
+ cl::init(""),
+ cl::cat(ClangTidyCategory));
+
static cl::opt<bool> SystemHeaders("system-headers", desc(R"(
Display the errors from system headers.
This option overrides the 'SystemHeaders' option
@@ -353,6 +366,7 @@ static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider(
DefaultOptions.Checks = DefaultChecks;
DefaultOptions.WarningsAsErrors = "";
DefaultOptions.HeaderFilterRegex = HeaderFilter;
+ DefaultOptions.ExcludeHeaderFilterRegex = ExcludeHeaderFilter;
DefaultOptions.SystemHeaders = SystemHeaders;
DefaultOptions.FormatStyle = FormatStyle;
DefaultOptions.User = llvm::sys::Process::GetEnv("USER");
@@ -367,6 +381,8 @@ static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider(
OverrideOptions.WarningsAsErrors = WarningsAsErrors;
if (HeaderFilter.getNumOccurrences() > 0)
OverrideOptions.HeaderFilterRegex = HeaderFilter;
+ if (ExcludeHeaderFilter.getNumOccurrences() > 0)
+ OverrideOptions.ExcludeHeaderFilterRegex = ExcludeHeaderFilter;
if (SystemHeaders.getNumOccurrences() > 0)
OverrideOptions.SystemHeaders = SystemHeaders;
if (FormatStyle.getNumOccurrences() > 0)
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp
index a7498723de2bf..448ef9ddf166c 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp
@@ -11,6 +11,7 @@
// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers -quiet %s -- -I %S/Inputs/file-filter/system/.. -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK4-QUIET %s
// RUN: clang-tidy -checks='-*,cppcoreguidelines-pro-type-cstyle-cast' -header-filter='.*' -system-headers %s -- -I %S/Inputs/file-filter/system/.. -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK5 %s
// RUN: clang-tidy -checks='-*,cppcoreguidelines-pro-type-cstyle-cast' -header-filter='.*' %s -- -I %S/Inputs/file-filter/system/.. -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK5-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -exclude-header-filter='header1\.h' %s -- -I %S/Inputs/file-filter/ -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK6 %s
#include "header1.h"
// CHECK-NOT: warning:
@@ -21,6 +22,7 @@
// CHECK3-QUIET-NOT: warning:
// CHECK4: header1.h:1:12: warning: single-argument constructors
// CHECK4-QUIET: header1.h:1:12: warning: single-argument constructors
+// CHECK6-NOT: warning:
#include "header2.h"
// CHECK-NOT: warning:
@@ -31,6 +33,7 @@
// CHECK3-QUIET: header2.h:1:12: warning: single-argument constructors
// CHECK4: header2.h:1:12: warning: single-argument constructors
// CHECK4-QUIET: header2.h:1:12: warning: single-argument constructors
+// CHECK6: header2.h:1:12: warning: single-argument constructors
#include <system-header.h>
// CHECK-NOT: warning:
@@ -41,6 +44,7 @@
// CHECK3-QUIET-NOT: warning:
// CHECK4: system-header.h:1:12: warning: single-argument constructors
// CHECK4-QUIET: system-header.h:1:12: warning: single-argument constructors
+// CHECK6-NOT: warning:
class A { A(int); };
// CHECK: :[[@LINE-1]]:11: warning: single-argument constructors
@@ -51,6 +55,7 @@ class A { A(int); };
// CHECK3-QUIET: :[[@LINE-6]]:11: warning: single-argument constructors
// CHECK4: :[[@LINE-7]]:11: warning: single-argument constructors
// CHECK4-QUIET: :[[@LINE-8]]:11: warning: single-argument constructors
+// CHECK6: :[[@LINE-9]]:11: warning: single-argument constructors
// CHECK-NOT: warning:
// CHECK-QUIET-NOT: warning:
@@ -73,6 +78,8 @@ class A { A(int); };
// CHECK4-NOT: Suppressed {{.*}} warnings
// CHECK4-NOT: Use -header-filter=.* {{.*}}
// CHECK4-QUIET-NOT: Suppressed
+// CHECK6: Suppressed 2 warnings (2 in non-user code)
+// CHECK6: Use -header-filter=.* {{.*}}
int x = 123;
auto x_ptr = TO_FLOAT_PTR(&x);
|
|
Also python wrappers may require updating to pass those settings to clang-tidy |
This PR addresses clang-tidy's lack of ability to ignore some set of headers when analyzing a file (most commonly third party code that cannot be modified).
The underlying regex implementation for
As I mentioned in the description, one option to allow exclusion of headers would be migrating to |
justincady
force-pushed
the
exclude-header-filter
branch
from
a5610e8 to
e65bd48
Compare
This is a renewed attempt to land @toddlipcon's D34654. The comments on that patch indicate a broad desire for some ability to ignore headers. After considering various options, including migrating to std::regex, I believe this is the best path forward. It's intuitive to have separate regexes for including headers versus excluding them, and this approach has the added benefit of being completely opt-in. No existing configs will break, regardless of existing HeaderFilterRegex values. This functionality is useful for improving performance when analyzing a targeted subset of code, as well as in cases where some collection of headers cannot be modified (third party source, for example).
justincady
force-pushed
the
exclude-header-filter
branch
from
b93421a to
58d3d52
Compare
|
@justincady Could you please provide us with an example usage of |
|
@codectile In that example all headers will be analyzed using the enabled checks except |
Update the documentation to direct new users to the Github instead of the discontinued Phabricator archive. Also details more ways and information regarding clang-query usage. Partially resolves/disclaims #106656 and #106663 as per discussion in https://discourse.llvm.org/t/inconsistency-between-hasdescendant-in-clang-query-and-clang-libtooling-matchers/80799/. Also updates the out-of-tree guide. For context, I recently went through the Contributing guide while writing #102299, and many of these updates were from my experience trying to follow the guide. e.g. I was trying to link the shared library of an out-of-tree check as SHARED in CMake and encountered duplicate symbols like _ZTIN5clang4tidy14ClangTidyCheckE. It wasn't until I saw 84f137a that I found out I had to use MODULE. I also encountered the clang-query difference which was a surprise as the documentation said the two matchers were "virtually identical". Also, the -header-filter thing tripped me out until I found #25590 and #91400. Usually, when people say restrict and filter, they mean filter out (since -header-filter instead includes/filters in said headers).
|
I found a problem. In my project, I used #include "qapplication.h"
#include "qpushbutton.h"
#include "qstring.h"
#include "fmt/format.h"
class MainWindow: public QWidget {
public:
MainWindow(QWidget *parent= nullptr)
: QWidget(parent)
{
auto testMsg= fmt::format("This is {1} test: {0},{1}\n", "Hello", "Qt5");
pButton_ = new QPushButton(QString::fromStdString(testMsg), this);
resize(1000, 800);
}
protected:
void resizeEvent(QResizeEvent *event) override
{
int buttonWidth = width() / 4;
int buttonHeight= height() / 4;
pButton_->setGeometry((width() - buttonWidth) / 2,
(height() - buttonHeight) / 2,
buttonWidth,
buttonHeight);
QWidget::resizeEvent(event);
}
private:
QPushButton *pButton_ { nullptr };
};
int main(int argc, char *argv[])
{
QApplication app(argc, argv);
MainWindow mainWindow;
mainWindow.show();
return app.exec();
}This is my [{
"directory": "D:/CXXProject/build",
"command": "D:\\Tools\\MSYS2\\mingw64\\bin\\c++.exe -DFMT_SHARED -DQT_CORE_LIB -DQT_GUI_LIB -DQT_WIDGETS_LIB -isystem D:/Tools/Vcpkg/installed/x64-mingw-dynamic/include/qt5 -isystem D:/Tools/Vcpkg/installed/x64-mingw-dynamic/include/qt5/QtCore -isystem D:/Tools/Vcpkg/installed/x64-mingw-dynamic/tools/qt5/mkspecs/win32-g++ -isystem D:/Tools/Vcpkg/installed/x64-mingw-dynamic/include/qt5/QtWidgets -isystem D:/Tools/Vcpkg/installed/x64-mingw-dynamic/include/qt5/QtGui -isystem D:/Tools/Vcpkg/installed/x64-mingw-dynamic/include -g -fdiagnostics-color=always -o tests\\Framework\\CMakeFiles\\Framework.dir\\Qt5Test.cc.obj -c D:\\CXXProject\\tests\\Framework\\Qt5Test.cc",
"file": "D:\\CXXProject\\tests\\Framework\\Qt5Test.cc",
"output": "tests\\Framework\\CMakeFiles\\Framework.dir\\Qt5Test.cc.obj"
}]Use the command (clang-tidy-19) : Here is the output: D:\CXXProject\tests\Framework\Qt5Test.cc:17:26: warning: no header providing "QWidget" is directly included [misc-include-cleaner]
16 |
17 | class MainWindow: public QWidget {
| ^
D:\CXXProject\tests\Framework\Qt5Test.cc:19:5: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
19 | MainWindow(QWidget *parent= nullptr)
| ^
| explicit
D:\CXXProject\tests\Framework\Qt5Test.cc:19:25: warning: invalid case style for pointer parameter 'parent' [readability-identifier-naming]
19 | MainWindow(QWidget *parent= nullptr)
| ^~~~~~
| pArent
20 | : QWidget(parent)
| ~~~~~~
| pArent
D:\CXXProject\tests\Framework\Qt5Test.cc:24:16: warning: 1000 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
24 | resize(1000, 800);
| ^
D:\CXXProject\tests\Framework\Qt5Test.cc:24:22: warning: 800 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
24 | resize(1000, 800);
| ^
D:\CXXProject\tests\Framework\Qt5Test.cc:28:22: warning: no header providing "QResizeEvent" is directly included [misc-include-cleaner]
28 | void resizeEvent(QResizeEvent *event) override
| ^
D:\CXXProject\tests\Framework\Qt5Test.cc:28:36: warning: invalid case style for pointer parameter 'event' [readability-identifier-naming]
28 | void resizeEvent(QResizeEvent *event) override
| ^~~~~
| pEvent
29 | {
30 | int buttonWidth = width() / 4;
31 | int buttonHeight= height() / 4;
32 |
33 | pButton_->setGeometry((width() - buttonWidth) / 2,
34 | (height() - buttonHeight) / 2,
35 | buttonWidth,
36 | buttonHeight);
37 |
38 | QWidget::resizeEvent(event);
| ~~~~~
| pEvent
D:\CXXProject\tests\Framework\Qt5Test.cc:42:18: warning: invalid case style for private member 'pButton_' [readability-identifier-naming]
33 | pButton_->setGeometry((width() - buttonWidth) / 2,
| ~~~~~~~~
| PButton_
34 | (height() - buttonHeight) / 2,
35 | buttonWidth,
36 | buttonHeight);
37 |
38 | QWidget::resizeEvent(event);
39 | }
40 |
41 | private:
42 | QPushButton *pButton_ { nullptr };
| ^~~~~~~~
| PButton_
D:\CXXProject\tests\Framework\Qt5Test.cc:52:12: warning: static member accessed through instance [readability-static-accessed-through-instance]
52 | return app.exec();
| ^~~~
| QApplication::
Suppressed 3386 warnings (3386 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
Found compiler error(s).But actually clang-tidy will still perform unnecessary analysis on qt header files, but will not output the results. I would like to ask if this function should directly skip the analysis of excluded header files? |
|
This feature works as intended, it is only for filtering out diagnosed issues, not for skipping the analysis in those files. See #52959 for the feature you are looking for. TLDR: clang-tidy currently does not skip the analysis in (system) header files, |
I have read the issues in detail and it doesn't look like it has been resolved? But just want to filter out the content not interested in, even if you don't use |
Sadly no, this does not exist yet and the linked issue is for tracking that feature. The option implemented in this PR allows to filter out from the generated diagnostics those, that have been generated and their file name matched the regex, but not to ignore the code in those headers all together. This means that we still analyze that code and diagnose it, we just don't show the diagnostic for it. Most likely, the reason you are not seeing a difference in the number of suppressed diagnostics, is that the QT headers are already marked as system includes by your build system, which are automatically excluded from analysis unless the (In case this does not answer your question or the issue I linked is not what you're after, please file an issue so that it will be more searchable for others and so that the conversation gets scoped to that issue and not in a PR conversation, which will likely not be searched for by others.) |
This is a renewed attempt to land @toddlipcon's D34654. The comments on
that patch indicate a broad desire for some ability to ignore headers.
After considering various options, including migrating to std::regex, I
believe this is the best path forward. It's intuitive to have separate
regexes for including headers versus excluding them, and this approach
has the added benefit of being completely opt-in. No existing configs
will break, regardless of existing HeaderFilterRegex values.
This functionality is useful for improving performance when analyzing a
targeted subset of code, as well as in cases where some collection of
headers cannot be modified (third party source, for example).