-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[clang-tidy] Remove 'clang-analyzer-*' checks from default checks. #157306
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-tidy @llvm/pr-subscribers-clang-tools-extra Author: Baranov Victor (vbvictor) ChangesCloses #146482. Full diff: https://github.com/llvm/llvm-project/pull/157306.diff 2 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index bef3b938b5afd..ed9b195f0dbde 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -102,8 +102,7 @@ Configuration files:
)");
const char DefaultChecks[] = // Enable these checks by default:
- "clang-diagnostic-*," // * compiler diagnostics
- "clang-analyzer-*"; // * Static Analyzer checks
+ "clang-diagnostic-*"; // * compiler diagnostics
static cl::opt<std::string> Checks("checks", desc(R"(
Comma-separated list of globs with optional '-'
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e1b6daf75457d..d97efe667f98e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -139,6 +139,8 @@ Improvements to clang-tidy
scripts by adding the `-hide-progress` option to suppress progress and
informational messages.
+- Removed `clang-analyzer-*` check from default checks in :program:`clang-tidy`.
+
New checks
^^^^^^^^^^
|
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, but I wonder if we need a 2-release cycle for this, after all people might silently lose an important part of clang-tidy coverage and the note in the release notes could be hard to spot in the middle of the other notes.
Perhaps having a section for "Breaking changes" or otherwise "Stuff you need to take action on" would help.
Would be good to hear what other reviewers think!
I support this, another solution could be bold text or preamble like "Important |
Filed #158434, once it is merged we could mark it as a breaking change. |
✅ With the latest revision this PR passed the C/C++ code formatter. |
54d4521
to
4ab5499
Compare
Now that #158434 is merged, should we move the change to the release notes to that section? |
f11b956
to
57c0b1e
Compare
Yes, I finally rebased on |
57c0b1e
to
534a01f
Compare
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
But do we want to mention the change twice in the change log? In another PR we are only mentioning the breaking change in the breaking changes section, not again in the improvement's section. |
I initially thought to duplicate entries just in case if someone would read only clang-tidy part. I think it's better to have in both |
I totally agree with the consistency. I personally think once is fine, but twice won't hurt. The other PR I mentioned would be #161064 |
Oh, must have missed that, we probably should add in tidy release notes for consistency |
Closes #146482.