-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[clang-tidy] Emit warnings from user headers by default #164165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: Baranov Victor (vbvictor) ChangesCloses #158132. Full diff: https://github.com/llvm/llvm-project/pull/164165.diff 15 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
index 21455db7c7e7b..c4b47a440e44b 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -247,7 +247,7 @@ ClangTidyOptions ClangTidyOptions::getDefaults() {
Options.WarningsAsErrors = "";
Options.HeaderFileExtensions = {"", "h", "hh", "hpp", "hxx"};
Options.ImplementationFileExtensions = {"c", "cc", "cpp", "cxx"};
- Options.HeaderFilterRegex = "";
+ Options.HeaderFilterRegex = ".*";
Options.ExcludeHeaderFilterRegex = "";
Options.SystemHeaders = false;
Options.FormatStyle = "none";
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index 64157f530b8c0..5d1e6b24cf38d 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -93,7 +93,7 @@ Configuration files:
WarningsAsErrors: ''
HeaderFileExtensions: ['', 'h','hh','hpp','hxx']
ImplementationFileExtensions: ['c','cc','cpp','cxx']
- HeaderFilterRegex: ''
+ HeaderFilterRegex: '.*'
FormatStyle: none
InheritParentConfig: true
User: user
@@ -133,13 +133,15 @@ file, if any.
static cl::opt<std::string> HeaderFilter("header-filter", desc(R"(
Regular expression matching the names of the
headers to output diagnostics from. Diagnostics
-from the main file of each translation unit are
-always displayed.
+from the main file and all non-system headers
+of each translation unit are always displayed.
+Set this option to an empty string to disable
+diagnostics from non-system headers.
Can be used together with -line-filter.
This option overrides the 'HeaderFilterRegex'
option in .clang-tidy file, if any.
)"),
- cl::init(""),
+ cl::init(".*"),
cl::cat(ClangTidyCategory));
static cl::opt<std::string> ExcludeHeaderFilter("exclude-header-filter",
@@ -379,9 +381,9 @@ static void printStats(const ClangTidyStats &Stats) {
<< " with check filters";
llvm::errs() << ").\n";
if (Stats.ErrorsIgnoredNonUserCode)
- llvm::errs() << "Use -header-filter=.* to display errors from all "
- "non-system headers. Use -system-headers to display "
- "errors from system headers as well.\n";
+ llvm::errs() << "Use -header-filter=.* or leave it as default to display "
+ "errors from all non-system headers. Use -system-headers "
+ "to display errors from system headers as well.\n";
}
}
diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
index f495f449b5b30..8050813de3445 100755
--- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -470,8 +470,8 @@ async def main() -> None:
default=None,
help="Regular expression matching the names of the "
"headers to output diagnostics from. Diagnostics from "
- "the main file of each translation unit are always "
- "displayed.",
+ "the main file and all non-system headers of each "
+ "translation unit are always displayed.",
)
parser.add_argument(
"-source-filter",
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a94dd9737468c..be1fba1c294ed 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -70,6 +70,11 @@ Potentially Breaking Changes
:doc:`bugprone-signed-char-misuse
<clang-tidy/checks/bugprone/signed-char-misuse>`
+- :program:`clang-tidy` now displays warnings from all non-system headers by
+ default. Previously, users had to explicitly opt-in to header warnings using
+ `-header-filter='.*'`. To disable warnings from non-system, set `-header-filter`
+ to an empty string.
+
Improvements to clangd
----------------------
@@ -132,6 +137,11 @@ Improvements to clang-tidy
when run over C files. If ``-std`` is not specified, it defaults to
``c99-or-later``.
+- :program:`clang-tidy` now displays warnings from all non-system headers by
+ default. Previously, users had to explicitly opt-in to header warnings using
+ `-header-filter='.*'`. To disable warnings from non-system, set `-header-filter`
+ to an empty string.
+
- :program:`clang-tidy` no longer attemps to analyze code from system headers
by default, greatly improving performance. This behavior is disabled if the
`SystemHeaders` option is enabled.
diff --git a/clang-tools-extra/docs/clang-tidy/index.rst b/clang-tools-extra/docs/clang-tidy/index.rst
index bd2c40e948f34..260d35392b18e 100644
--- a/clang-tools-extra/docs/clang-tidy/index.rst
+++ b/clang-tools-extra/docs/clang-tidy/index.rst
@@ -216,8 +216,10 @@ An overview of all the command-line options:
.clang-tidy file, if any.
--header-filter=<string> - Regular expression matching the names of the
headers to output diagnostics from. Diagnostics
- from the main file of each translation unit are
- always displayed.
+ from the main file and all non-system headers
+ of each translation unit are always displayed.
+ Set this option to an empty string to disable
+ diagnostics from non-system headers.
Can be used together with -line-filter.
This option overrides the 'HeaderFilterRegex'
option in .clang-tidy file, if any.
@@ -338,7 +340,7 @@ An overview of all the command-line options:
WarningsAsErrors: ''
HeaderFileExtensions: ['', 'h','hh','hpp','hxx']
ImplementationFileExtensions: ['c','cc','cpp','cxx']
- HeaderFilterRegex: ''
+ HeaderFilterRegex: '.*'
FormatStyle: none
InheritParentConfig: true
User: user
diff --git a/clang-tools-extra/test/clang-tidy/checkers/abseil/no-internal-dependencies.cpp b/clang-tools-extra/test/clang-tidy/checkers/abseil/no-internal-dependencies.cpp
index 2949d7fdd0274..f6eb7c5e25949 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/abseil/no-internal-dependencies.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/abseil/no-internal-dependencies.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s abseil-no-internal-dependencies %t, -- -- -I %S/Inputs
+// RUN: %check_clang_tidy %s abseil-no-internal-dependencies %t, -- -header-filter='' -- -I %S/Inputs
// RUN: clang-tidy -checks='-*, abseil-no-internal-dependencies' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
#include "absl/strings/internal-file.h"
diff --git a/clang-tools-extra/test/clang-tidy/checkers/abseil/no-namespace.cpp b/clang-tools-extra/test/clang-tidy/checkers/abseil/no-namespace.cpp
index 78821c373f5c4..c8a5752ed86a6 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/abseil/no-namespace.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/abseil/no-namespace.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s abseil-no-namespace %t -- -- -I %S/Inputs
+// RUN: %check_clang_tidy %s abseil-no-namespace %t -- -header-filter='' -- -I %S/Inputs
// RUN: clang-tidy -checks='-*, abseil-no-namespace' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
/// Warning will not be triggered on internal Abseil code that is included.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp
index 0f36efe656bf9..b17e8903c41c2 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/reserved-identifier.cpp
@@ -1,8 +1,9 @@
-// RUN: %check_clang_tidy %s bugprone-reserved-identifier %t -- -- \
+// RUN: %check_clang_tidy %s bugprone-reserved-identifier %t -- \
+// RUN: -header-filter='' -- \
// RUN: -I%S/Inputs/reserved-identifier \
// RUN: -isystem %S/Inputs/reserved-identifier/system
-// no warnings expected without -header-filter=
+// no warnings expected with -header-filter=''
#include "user-header.h"
#include <system-header.h>
diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/upgrade-googletest-case.cpp b/clang-tools-extra/test/clang-tidy/checkers/google/upgrade-googletest-case.cpp
index edb11b9863532..5b30541a96a42 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/google/upgrade-googletest-case.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/google/upgrade-googletest-case.cpp
@@ -1,5 +1,5 @@
-// RUN: %check_clang_tidy %s google-upgrade-googletest-case %t -- -- -I%S/Inputs
-// RUN: %check_clang_tidy -check-suffix=NOSUITE %s google-upgrade-googletest-case %t -- -- -DNOSUITE -I%S/Inputs/gtest/nosuite
+// RUN: %check_clang_tidy %s google-upgrade-googletest-case %t -- -- -isystem%S/Inputs
+// RUN: %check_clang_tidy -check-suffix=NOSUITE %s google-upgrade-googletest-case %t -- -- -DNOSUITE -isystem%S/Inputs/gtest/nosuite
#include "gtest/gtest.h"
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-auto-ptr.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-auto-ptr.cpp
index 2281c1acad94f..371f3ddf6d650 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-auto-ptr.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/replace-auto-ptr.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s modernize-replace-auto-ptr %t -- -- -I %S/Inputs/replace-auto-ptr
+// RUN: %check_clang_tidy %s modernize-replace-auto-ptr %t -- -- -isystem %S/Inputs/replace-auto-ptr
// CHECK-FIXES: #include <utility>
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
index 8288f39126a11..5b8eca2825645 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s modernize-use-using %t -- -- -fno-delayed-template-parsing -I %S/Inputs/use-using/
+// RUN: %check_clang_tidy %s modernize-use-using %t -- -- -fno-delayed-template-parsing -isystem %S/Inputs/use-using/
typedef int Type;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp
index 223f07724c5d0..c452f69fad07d 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s readability-duplicate-include %t -- -- -isystem %S/Inputs/duplicate-include/system -I %S/Inputs/duplicate-include
+// RUN: %check_clang_tidy %s readability-duplicate-include %t -- \
+// RUN: -header-filter='' \
+// RUN: -- -isystem %S/Inputs/duplicate-include/system -I %S/Inputs/duplicate-include
int a;
#include <string.h>
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp
index 86502759c2bcd..5b1f9cb3f3c9e 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp
@@ -82,7 +82,9 @@
// RUN: readability-identifier-naming.LocalPointerPrefix: 'l_', \
// RUN: readability-identifier-naming.LocalConstantPointerCase: CamelCase, \
// RUN: readability-identifier-naming.LocalConstantPointerPrefix: 'lc_', \
-// RUN: }}' -- -fno-delayed-template-parsing -Dbad_macro \
+// RUN: }}' \
+// RUN: -header-filter='' \
+// RUN: -- -fno-delayed-template-parsing -Dbad_macro \
// RUN: -I%S/Inputs/identifier-naming \
// RUN: -isystem %S/Inputs/identifier-naming/system
@@ -91,8 +93,7 @@
#include <system-header.h>
#include <coroutines.h>
#include "user-header.h"
-// NO warnings or fixes expected from declarations within header files without
-// the -header-filter= option
+// NO warnings or fixes expected from declarations with the -header-filter='' option
namespace FOO_NS {
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: invalid case style for namespace 'FOO_NS' [readability-identifier-naming]
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/default-header-filter.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/default-header-filter.cpp
new file mode 100644
index 0000000000000..a7f22e6cd8e7b
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/default-header-filter.cpp
@@ -0,0 +1,27 @@
+
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' --config='{}' %s -- -I %S/Inputs/file-filter -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK-DEFAULT %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' --config='{}' -header-filter='' %s -- -I %S/Inputs/file-filter -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK-EMPTY %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' --config='{}' -header-filter='.*' %s -- -I %S/Inputs/file-filter -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK-EXPLICIT %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' --config='{}' %s -- -I %S/Inputs/file-filter -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK-NO-SYSTEM %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' --config='{}' -system-headers %s -- -I %S/Inputs/file-filter -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK-WITH-SYSTEM %s
+
+#include "header1.h"
+// CHECK-DEFAULT: header1.h:1:12: warning: single-argument constructors must be marked explicit
+// CHECK-EMPTY-NOT: header1.h:1:12: warning:
+// CHECK-EXPLICIT: header1.h:1:12: warning: single-argument constructors must be marked explicit
+// CHECK-NO-SYSTEM: header1.h:1:12: warning: single-argument constructors must be marked explicit
+// CHECK-WITH-SYSTEM: header1.h:1:12: warning: single-argument constructors must be marked explicit
+
+#include <system-header.h>
+// CHECK-DEFAULT-NOT: system-header.h:1:12: warning:
+// CHECK-EMPTY-NOT: system-header.h:1:12: warning:
+// CHECK-EXPLICIT-NOT: system-header.h:1:12: warning:
+// CHECK-NO-SYSTEM-NOT: system-header.h:1:12: warning:
+// CHECK-WITH-SYSTEM: system-header.h:1:12: warning: single-argument constructors must be marked explicit
+
+class A { A(int); };
+// CHECK-DEFAULT: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+// CHECK-EMPTY: :[[@LINE-2]]:11: warning: single-argument constructors must be marked explicit
+// CHECK-EXPLICIT: :[[@LINE-3]]:11: warning: single-argument constructors must be marked explicit
+// CHECK-NO-SYSTEM: :[[@LINE-4]]:11: warning: single-argument constructors must be marked explicit
+// CHECK-WITH-SYSTEM: :[[@LINE-5]]:11: warning: single-argument constructors must be marked explicit
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 d9ec1049963b0..485e9fb1f0cb7 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp
@@ -66,7 +66,7 @@ class A { A(int); };
// CHECK4-NOT: warning:
// CHECK4-QUIET-NOT: warning:
-// CHECK: Use -header-filter=.* to display errors from all non-system headers.
+// CHECK: Use -header-filter=.* or leave it as default to display errors from all non-system headers.
// CHECK-QUIET-NOT: Suppressed
// CHECK2-QUIET-NOT: Suppressed
// CHECK3: Use -header-filter=.* {{.*}}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But please wait for other reviewers.
from the main file and all non-system headers | ||
of each translation unit are always displayed. | ||
Set this option to an empty string to disable | ||
diagnostics from non-system headers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wording is a bit confusing to me. Warnings from non-system headers are not always displayed, the point is that you can disable them with this option, right? Something like this seems more accurate to me:
from the main file and all non-system headers | |
of each translation unit are always displayed. | |
Set this option to an empty string to disable | |
diagnostics from non-system headers. | |
from the main file of each translation unit | |
are always displayed. Diagnostics from | |
non-system headers it includes are displayed | |
by default, but can be disabled with this option. |
(Also applies in other places where the wording is repeated)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would simply keep the original text as-is and document what the default value for this option is (if it's not already stated somewhere else).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Closes #158132.