-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[clang-tidy] Improve bugprone-exception-escape
's handling of lambdas
#160592
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
Conversation
@llvm/pr-subscribers-clang-tidy Author: Victor Chernyakin (localspook) ChangesFixes #132605. Full diff: https://github.com/llvm/llvm-project/pull/160592.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 bdde7249d2796..fd4320eb8144b 100644
--- a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
+++ b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
@@ -595,6 +595,11 @@ ExceptionAnalyzer::throwsException(const Stmt *St,
Results.merge(DestructorExcs);
}
}
+ } else if (const auto *Lambda = dyn_cast<LambdaExpr>(St)) {
+ for (const Stmt *Init : Lambda->capture_inits()) {
+ ExceptionInfo Excs = throwsException(Init, Caught, CallStack);
+ Results.merge(Excs);
+ }
} else {
for (const Stmt *Child : St->children()) {
ExceptionInfo Excs = throwsException(Child, Caught, CallStack);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 7cdff86beeec6..63ea45cb39270 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -210,6 +210,11 @@ Changes in existing checks
correcting a spelling mistake on its option
``NamePrefixSuffixSilenceDissimilarityTreshold``.
+- 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.
+
- Improved :doc:`bugprone-infinite-loop
<clang-tidy/checks/bugprone/infinite-loop>` check by adding detection for
variables introduced by structured bindings.
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 b10bd1d482867..da200cd6de8df 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
@@ -894,3 +894,41 @@ void pointer_exception_can_not_escape_with_void_handler() noexcept {
} catch (void *) {
}
}
+
+void throw_in_uninvoked_lambda() noexcept {
+ [] { throw 42; };
+}
+
+struct copy_constructor_throws {
+ copy_constructor_throws(const copy_constructor_throws&) { throw 42; }
+};
+
+void throw_in_lambda_default_by_value_capture(const copy_constructor_throws& a) noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function 'throw_in_lambda_default_by_value_capture' which should not throw exceptions
+ [=] { a; };
+ // CHECK-MESSAGES: :[[@LINE-6]]:61: note: frame #0: unhandled exception of type 'int' may be thrown in function 'copy_constructor_throws' here
+ // CHECK-MESSAGES: :[[@LINE-2]]:4: note: frame #1: function 'throw_in_lambda_default_by_value_capture' calls function 'copy_constructor_throws' here
+}
+
+void throw_in_lambda_explicit_by_value_capture(const copy_constructor_throws& a) noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function 'throw_in_lambda_explicit_by_value_capture' which should not throw exceptions
+ [a] {};
+ // CHECK-MESSAGES: :[[@LINE-13]]:61: note: frame #0: unhandled exception of type 'int' may be thrown in function 'copy_constructor_throws' here
+ // CHECK-MESSAGES: :[[@LINE-2]]:4: note: frame #1: function 'throw_in_lambda_explicit_by_value_capture' calls function 'copy_constructor_throws' here
+}
+
+void no_throw_in_lambda_by_reference_capture(const copy_constructor_throws& a) noexcept {
+ [&] { a; };
+ [&a] {};
+}
+
+void throw_in_uninvoked_lambda() noexcept {
+ [] { throw 42; };
+}
+
+void throw_in_lambda() noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function 'throw_in_lambda' which should not throw exceptions
+ [] { throw 42; }();
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: note: frame #0: unhandled exception of type 'int' may be thrown in function 'operator()' here
+ // CHECK-MESSAGES: :[[@LINE-2]]:19: note: frame #1: function 'throw_in_lambda' calls function 'operator()' here
+}
|
@llvm/pr-subscribers-clang-tools-extra Author: Victor Chernyakin (localspook) ChangesFixes #132605. Full diff: https://github.com/llvm/llvm-project/pull/160592.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 bdde7249d2796..fd4320eb8144b 100644
--- a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
+++ b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
@@ -595,6 +595,11 @@ ExceptionAnalyzer::throwsException(const Stmt *St,
Results.merge(DestructorExcs);
}
}
+ } else if (const auto *Lambda = dyn_cast<LambdaExpr>(St)) {
+ for (const Stmt *Init : Lambda->capture_inits()) {
+ ExceptionInfo Excs = throwsException(Init, Caught, CallStack);
+ Results.merge(Excs);
+ }
} else {
for (const Stmt *Child : St->children()) {
ExceptionInfo Excs = throwsException(Child, Caught, CallStack);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 7cdff86beeec6..63ea45cb39270 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -210,6 +210,11 @@ Changes in existing checks
correcting a spelling mistake on its option
``NamePrefixSuffixSilenceDissimilarityTreshold``.
+- 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.
+
- Improved :doc:`bugprone-infinite-loop
<clang-tidy/checks/bugprone/infinite-loop>` check by adding detection for
variables introduced by structured bindings.
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 b10bd1d482867..da200cd6de8df 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
@@ -894,3 +894,41 @@ void pointer_exception_can_not_escape_with_void_handler() noexcept {
} catch (void *) {
}
}
+
+void throw_in_uninvoked_lambda() noexcept {
+ [] { throw 42; };
+}
+
+struct copy_constructor_throws {
+ copy_constructor_throws(const copy_constructor_throws&) { throw 42; }
+};
+
+void throw_in_lambda_default_by_value_capture(const copy_constructor_throws& a) noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function 'throw_in_lambda_default_by_value_capture' which should not throw exceptions
+ [=] { a; };
+ // CHECK-MESSAGES: :[[@LINE-6]]:61: note: frame #0: unhandled exception of type 'int' may be thrown in function 'copy_constructor_throws' here
+ // CHECK-MESSAGES: :[[@LINE-2]]:4: note: frame #1: function 'throw_in_lambda_default_by_value_capture' calls function 'copy_constructor_throws' here
+}
+
+void throw_in_lambda_explicit_by_value_capture(const copy_constructor_throws& a) noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function 'throw_in_lambda_explicit_by_value_capture' which should not throw exceptions
+ [a] {};
+ // CHECK-MESSAGES: :[[@LINE-13]]:61: note: frame #0: unhandled exception of type 'int' may be thrown in function 'copy_constructor_throws' here
+ // CHECK-MESSAGES: :[[@LINE-2]]:4: note: frame #1: function 'throw_in_lambda_explicit_by_value_capture' calls function 'copy_constructor_throws' here
+}
+
+void no_throw_in_lambda_by_reference_capture(const copy_constructor_throws& a) noexcept {
+ [&] { a; };
+ [&a] {};
+}
+
+void throw_in_uninvoked_lambda() noexcept {
+ [] { throw 42; };
+}
+
+void throw_in_lambda() noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function 'throw_in_lambda' which should not throw exceptions
+ [] { throw 42; }();
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: note: frame #0: unhandled exception of type 'int' may be thrown in function 'operator()' here
+ // CHECK-MESSAGES: :[[@LINE-2]]:19: note: frame #1: function 'throw_in_lambda' calls function 'operator()' here
+}
|
d9d55b5
to
b665234
Compare
b665234
to
e0ca1f5
Compare
I'd generally wish to add more edge cases for lambdas, just as with functions. |
Also, could you share how we traversed before the fix and now? (If you debugged it)
I thought we needed some modelling for calling the lambda in order to properly handle |
Before this change, the code was going into this branch: llvm-project/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp Lines 598 to 603 in 738e927
That's the same branch it goes into when it processes a CompoundStmt . In other words, it confused this:
void f() noexcept { [] { throw 42; }; } for this: void f() noexcept { { throw 42; } } My change only affects the lambda expression itself; the actual call to a lambda goes through the existing code paths. |
a6cb241
to
5cf9561
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.
could you add some test of
- nested lambda
- more complex case in lambda e.g. call a function may throw.
Added some more tests, tell me if I should add any others |
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
Fixes #132605.