-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Fix false positive in bugprone-throw-keyword-missing #115302
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
Fix false positive in bugprone-throw-keyword-missing #115302
Conversation
@llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang-tools-extra Author: Carlos Galvez (carlosgalvezp) ChangesFixes #115055 Full diff: https://github.com/llvm/llvm-project/pull/115302.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
index 64c155c29cf8b9..17d2e75e4f666f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ThrowKeywordMissingCheck.cpp
@@ -15,9 +15,6 @@ using namespace clang::ast_matchers;
namespace clang::tidy::bugprone {
void ThrowKeywordMissingCheck::registerMatchers(MatchFinder *Finder) {
- auto CtorInitializerList =
- cxxConstructorDecl(hasAnyConstructorInitializer(anything()));
-
Finder->addMatcher(
cxxConstructExpr(
hasType(cxxRecordDecl(
@@ -27,7 +24,7 @@ void ThrowKeywordMissingCheck::registerMatchers(MatchFinder *Finder) {
stmt(anyOf(cxxThrowExpr(), callExpr(), returnStmt()))),
hasAncestor(decl(anyOf(varDecl(), fieldDecl()))),
hasAncestor(expr(cxxNewExpr(hasAnyPlacementArg(anything())))),
- allOf(hasAncestor(CtorInitializerList),
+ allOf(hasAncestor(cxxConstructorDecl()),
unless(hasAncestor(cxxCatchStmt()))))))
.bind("temporary-exception-not-thrown"),
this);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 51ba157ab05deb..e3294c12896357 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -177,6 +177,10 @@ Changes in existing checks
usages of ``sizeof()``, ``alignof()``, and ``offsetof()`` when adding or
subtracting from a pointer directly or when used to scale a numeric value.
+- Improved :doc:`bugprone-throw-keyword-missing
+ <clang-tidy/checks/bugprone/throw-keyword-missing>` by fixing a false positive
+ when using non-static member initializers and a constructor.
+
- Improved :doc:`bugprone-unchecked-optional-access
<clang-tidy/checks/bugprone/unchecked-optional-access>` to support
`bsl::optional` and `bdlb::NullableValue` from
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/throw-keyword-missing.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/throw-keyword-missing.cpp
index 49233c0deefdf0..aa4bccb109f3f3 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/throw-keyword-missing.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/throw-keyword-missing.cpp
@@ -139,6 +139,14 @@ CtorInitializerListTest::CtorInitializerListTest(float) try : exc(RegularExcepti
RegularException();
}
+// https://github.com/llvm/llvm-project/issues/115055
+class CtorInitializerListTest2 {
+ public:
+ CtorInitializerListTest2(){}
+ private:
+ RegularException exc{};
+};
+
RegularException funcReturningExceptionTest(int i) {
return RegularException();
}
|
Ok it seems the Windows build is broken in general, please disregard that! |
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
I filed #115540 because I noticed how the hasAncestor
matcher in the unless
part means that we are excluding quite some cases. (unrelated to this PRI)
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
clang-tools-extra/test/clang-tidy/checkers/bugprone/throw-keyword-missing.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/bugprone/throw-keyword-missing.cpp
Outdated
Show resolved
Hide resolved
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/52/builds/3602 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/24/builds/2580 Here is the relevant piece of the build log for the reference
|
Fixes llvm#115055 --------- Co-authored-by: Carlos Gálvez <[email protected]>
Fixes #115055