[clang-tidy] Fix bugprone-exception-escape not diagnosing throws in argument lists
#165955
+70
−12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
localspook
requested review from
EugeneZelenko,
HerrCai0907,
carlosgalvezp and
vbvictor
|
@llvm/pr-subscribers-clang-tools-extra Author: Victor Chernyakin (localspook) ChangesFixed #165766. See that issue for a description of the problem. Full diff: https://github.com/llvm/llvm-project/pull/165955.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
index fd4320eb8144b..db55d9f50e1bb 100644
--- a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
+++ b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
@@ -563,16 +563,6 @@ ExceptionAnalyzer::throwsException(const Stmt *St,
}
}
Results.merge(Uncaught);
- } else if (const auto *Call = dyn_cast<CallExpr>(St)) {
- if (const FunctionDecl *Func = Call->getDirectCallee()) {
- ExceptionInfo Excs =
- throwsException(Func, Caught, CallStack, Call->getBeginLoc());
- Results.merge(Excs);
- }
- } else if (const auto *Construct = dyn_cast<CXXConstructExpr>(St)) {
- ExceptionInfo Excs = throwsException(Construct->getConstructor(), Caught,
- CallStack, Construct->getBeginLoc());
- Results.merge(Excs);
} else if (const auto *DefaultInit = dyn_cast<CXXDefaultInitExpr>(St)) {
ExceptionInfo Excs =
throwsException(DefaultInit->getExpr(), Caught, CallStack);
@@ -605,6 +595,18 @@ ExceptionAnalyzer::throwsException(const Stmt *St,
ExceptionInfo Excs = throwsException(Child, Caught, CallStack);
Results.merge(Excs);
}
+
+ if (const auto *Call = dyn_cast<CallExpr>(St)) {
+ if (const FunctionDecl *Func = Call->getDirectCallee()) {
+ ExceptionInfo Excs =
+ throwsException(Func, Caught, CallStack, Call->getBeginLoc());
+ Results.merge(Excs);
+ }
+ } else if (const auto *Construct = dyn_cast<CXXConstructExpr>(St)) {
+ ExceptionInfo Excs = throwsException(Construct->getConstructor(), Caught,
+ CallStack, Construct->getBeginLoc());
+ Results.merge(Excs);
+ }
}
return Results;
}
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 33cc401bcb78f..76961e1efd945 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -260,7 +260,9 @@ Changes in existing checks
- Improved :doc:`bugprone-exception-escape
<clang-tidy/checks/bugprone/exception-escape>` check's handling of lambdas:
exceptions from captures are now diagnosed, exceptions in the bodies of
- lambdas that aren't actually invoked are not.
+ lambdas that aren't actually invoked are not. Additionally, fixed an issue
+ where the check wouldn't diagnose throws in arguments to functions or
+ constructors.
- Improved :doc:`bugprone-infinite-loop
<clang-tidy/checks/bugprone/infinite-loop>` check by adding detection for
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
index a52bbe2246d1e..b7af7d81a3a7b 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
@@ -948,7 +948,7 @@ const auto throw_in_noexcept_lambda = [] () noexcept { throw 42; };
// CHECK-MESSAGES: :[[@LINE-1]]:39: warning: an exception may be thrown in function 'operator()' which should not throw exceptions
// CHECK-MESSAGES: :[[@LINE-2]]:56: note: frame #0: unhandled exception of type 'int' may be thrown in function 'operator()' here
-void thrower() {
+int thrower() {
throw 42;
}
@@ -956,3 +956,40 @@ const auto indirect_throw_in_noexcept_lambda = [] () noexcept { thrower(); };
// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: an exception may be thrown in function 'operator()' which should not throw exceptions
// CHECK-MESSAGES: :[[@LINE-5]]:3: note: frame #0: unhandled exception of type 'int' may be thrown in function 'thrower' here
// CHECK-MESSAGES: :[[@LINE-3]]:65: note: frame #1: function 'operator()' calls function 'thrower' here
+
+int f(int);
+void throw_in_function_arg() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_in_function_arg' which should not throw exceptions
+ f(false ? 0 : throw 1);
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:17: note: frame #0: unhandled exception of type 'int' may be thrown in function 'throw_in_function_arg' here
+
+void indirect_throw_in_function_arg() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'indirect_throw_in_function_arg' which should not throw exceptions
+ f(thrower());
+}
+// CHECK-MESSAGES: :[[@LINE-19]]:3: note: frame #0: unhandled exception of type 'int' may be thrown in function 'thrower' here
+// CHECK-MESSAGES: :[[@LINE-3]]:5: note: frame #1: function 'indirect_throw_in_function_arg' calls function 'thrower' here
+
+struct S {
+ S(int) noexcept {}
+};
+
+void throw_in_constructor_arg() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_in_constructor_arg' which should not throw exceptions
+ S s(false ? 0 : throw 1);
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:19: note: frame #0: unhandled exception of type 'int' may be thrown in function 'throw_in_constructor_arg' here
+
+void indirect_throw_in_constructor_arg() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'indirect_throw_in_constructor_arg' which should not throw exceptions
+ S s = thrower();
+}
+// CHECK-MESSAGES: :[[@LINE-36]]:3: note: frame #0: unhandled exception of type 'int' may be thrown in function 'thrower' here
+// CHECK-MESSAGES: :[[@LINE-3]]:9: note: frame #1: function 'indirect_throw_in_constructor_arg' calls function 'thrower' here
+
+void weird_throw_in_call_subexpression() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'weird_throw_in_call_subexpression' which should not throw exceptions
+ (false ? []{} : throw 1)();
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:19: note: frame #0: unhandled exception of type 'int' may be thrown in function 'weird_throw_in_call_subexpression' here
|
|
@llvm/pr-subscribers-clang-tidy Author: Victor Chernyakin (localspook) ChangesFixed #165766. See that issue for a description of the problem. Full diff: https://github.com/llvm/llvm-project/pull/165955.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
index fd4320eb8144b..db55d9f50e1bb 100644
--- a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
+++ b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
@@ -563,16 +563,6 @@ ExceptionAnalyzer::throwsException(const Stmt *St,
}
}
Results.merge(Uncaught);
- } else if (const auto *Call = dyn_cast<CallExpr>(St)) {
- if (const FunctionDecl *Func = Call->getDirectCallee()) {
- ExceptionInfo Excs =
- throwsException(Func, Caught, CallStack, Call->getBeginLoc());
- Results.merge(Excs);
- }
- } else if (const auto *Construct = dyn_cast<CXXConstructExpr>(St)) {
- ExceptionInfo Excs = throwsException(Construct->getConstructor(), Caught,
- CallStack, Construct->getBeginLoc());
- Results.merge(Excs);
} else if (const auto *DefaultInit = dyn_cast<CXXDefaultInitExpr>(St)) {
ExceptionInfo Excs =
throwsException(DefaultInit->getExpr(), Caught, CallStack);
@@ -605,6 +595,18 @@ ExceptionAnalyzer::throwsException(const Stmt *St,
ExceptionInfo Excs = throwsException(Child, Caught, CallStack);
Results.merge(Excs);
}
+
+ if (const auto *Call = dyn_cast<CallExpr>(St)) {
+ if (const FunctionDecl *Func = Call->getDirectCallee()) {
+ ExceptionInfo Excs =
+ throwsException(Func, Caught, CallStack, Call->getBeginLoc());
+ Results.merge(Excs);
+ }
+ } else if (const auto *Construct = dyn_cast<CXXConstructExpr>(St)) {
+ ExceptionInfo Excs = throwsException(Construct->getConstructor(), Caught,
+ CallStack, Construct->getBeginLoc());
+ Results.merge(Excs);
+ }
}
return Results;
}
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 33cc401bcb78f..76961e1efd945 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -260,7 +260,9 @@ Changes in existing checks
- Improved :doc:`bugprone-exception-escape
<clang-tidy/checks/bugprone/exception-escape>` check's handling of lambdas:
exceptions from captures are now diagnosed, exceptions in the bodies of
- lambdas that aren't actually invoked are not.
+ lambdas that aren't actually invoked are not. Additionally, fixed an issue
+ where the check wouldn't diagnose throws in arguments to functions or
+ constructors.
- Improved :doc:`bugprone-infinite-loop
<clang-tidy/checks/bugprone/infinite-loop>` check by adding detection for
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
index a52bbe2246d1e..b7af7d81a3a7b 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
@@ -948,7 +948,7 @@ const auto throw_in_noexcept_lambda = [] () noexcept { throw 42; };
// CHECK-MESSAGES: :[[@LINE-1]]:39: warning: an exception may be thrown in function 'operator()' which should not throw exceptions
// CHECK-MESSAGES: :[[@LINE-2]]:56: note: frame #0: unhandled exception of type 'int' may be thrown in function 'operator()' here
-void thrower() {
+int thrower() {
throw 42;
}
@@ -956,3 +956,40 @@ const auto indirect_throw_in_noexcept_lambda = [] () noexcept { thrower(); };
// CHECK-MESSAGES: :[[@LINE-1]]:48: warning: an exception may be thrown in function 'operator()' which should not throw exceptions
// CHECK-MESSAGES: :[[@LINE-5]]:3: note: frame #0: unhandled exception of type 'int' may be thrown in function 'thrower' here
// CHECK-MESSAGES: :[[@LINE-3]]:65: note: frame #1: function 'operator()' calls function 'thrower' here
+
+int f(int);
+void throw_in_function_arg() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_in_function_arg' which should not throw exceptions
+ f(false ? 0 : throw 1);
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:17: note: frame #0: unhandled exception of type 'int' may be thrown in function 'throw_in_function_arg' here
+
+void indirect_throw_in_function_arg() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'indirect_throw_in_function_arg' which should not throw exceptions
+ f(thrower());
+}
+// CHECK-MESSAGES: :[[@LINE-19]]:3: note: frame #0: unhandled exception of type 'int' may be thrown in function 'thrower' here
+// CHECK-MESSAGES: :[[@LINE-3]]:5: note: frame #1: function 'indirect_throw_in_function_arg' calls function 'thrower' here
+
+struct S {
+ S(int) noexcept {}
+};
+
+void throw_in_constructor_arg() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_in_constructor_arg' which should not throw exceptions
+ S s(false ? 0 : throw 1);
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:19: note: frame #0: unhandled exception of type 'int' may be thrown in function 'throw_in_constructor_arg' here
+
+void indirect_throw_in_constructor_arg() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'indirect_throw_in_constructor_arg' which should not throw exceptions
+ S s = thrower();
+}
+// CHECK-MESSAGES: :[[@LINE-36]]:3: note: frame #0: unhandled exception of type 'int' may be thrown in function 'thrower' here
+// CHECK-MESSAGES: :[[@LINE-3]]:9: note: frame #1: function 'indirect_throw_in_constructor_arg' calls function 'thrower' here
+
+void weird_throw_in_call_subexpression() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'weird_throw_in_call_subexpression' which should not throw exceptions
+ (false ? []{} : throw 1)();
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:19: note: frame #0: unhandled exception of type 'int' may be thrown in function 'weird_throw_in_call_subexpression' here
|
vbvictor
reviewed
Nov 3, 2025
clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
Show resolved
Hide resolved
vbvictor
reviewed
Nov 4, 2025
Comment on lines
+602
to
+608
| if (const auto *Call = dyn_cast<CallExpr>(St)) { | ||
| if (const FunctionDecl *Func = Call->getDirectCallee()) { | ||
| ExceptionInfo Excs = | ||
| throwsException(Func, Caught, CallStack, Call->getBeginLoc()); | ||
| Results.merge(Excs); | ||
| } | ||
| } else if (const auto *Construct = dyn_cast<CXXConstructExpr>(St)) { |
There was a problem hiding this comment.
Thinking out loud, I think it will be more readable if instead of placing dyn_cast<CallExpr> in "main" else branch, we would extract "check for children" part into a separate function and use it like this:
} else if (const auto *Call = dyn_cast<CallExpr>(St)) {
Results.merge(CheckChildren(St)); // New code!
if (const FunctionDecl *Func = Call->getDirectCallee()) {
ExceptionInfo Excs =
throwsException(Func, Caught, CallStack, Call->getBeginLoc());
Results.merge(Excs);
}
} else if (const auto *Construct = dyn_cast<CXXConstructExpr>(St)) {
Results.merge(CheckChildren(St)); // New code!
ExceptionInfo Excs = throwsException(Construct->getConstructor(), Caught,
CallStack, Construct->getBeginLoc());
Results.merge(Excs);
}I'd rather see this more straightforward solution instead of twisting order of branches, which seem to me more bugprone and harder to read, WDYT?
There was a problem hiding this comment.
Ideally, I'd really like to see this big function separated in N smaller with each function corresponding to each type of node we process.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #165766. See that issue for a description of the problem.