[clang] [llvm] [Clang][CodeGen][Coroutines] Make coroutine startup exception-safe (CWG2935) (PR #177628)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 23 10:05:20 PST 2026
github-actions[bot] wrote:
<!--LLVM CODE FORMAT COMMENT: {clang-format}-->
:warning: C/C++ code formatter, clang-format found issues in your code. :warning:
<details>
<summary>
You can test this locally with the following command:
</summary>
``````````bash
git-clang-format --diff origin/main HEAD --extensions cpp -- coroutine-cwg2935.cpp clang/lib/CodeGen/CGCoroutine.cpp clang/test/CodeGenCoroutines/coro-suspend-cleanups.cpp --diff_from_common_commit
``````````
:warning:
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing `origin/main` to the base branch/commit you want to compare against.
:warning:
</details>
<details>
<summary>
View the diff from clang-format here.
</summary>
``````````diff
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index cced2ec27..df59bb2cf 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -782,13 +782,11 @@ struct GetReturnObjectManager {
// Create an active flag (initialize to true) for conditional
// cleanup. We are not necessarily in a conditional branch here so
// use a simple temp alloca instead of createCleanupActiveFlag().
- auto ActiveFlag = CGF.CreateTempAlloca(Builder.getInt1Ty(),
- CharUnits::One(),
- "direct.gro.active");
+ auto ActiveFlag = CGF.CreateTempAlloca(
+ Builder.getInt1Ty(), CharUnits::One(), "direct.gro.active");
Builder.CreateStore(Builder.getTrue(), ActiveFlag);
- CGF.pushDestroyAndDeferDeactivation(
- DtorKind, CGF.ReturnValue,
- S.getReturnValue()->getType());
+ CGF.pushDestroyAndDeferDeactivation(DtorKind, CGF.ReturnValue,
+ S.getReturnValue()->getType());
CGF.initFullExprCleanupWithFlag(ActiveFlag);
DirectGroActiveFlag = ActiveFlag;
}
diff --git a/coroutine-cwg2935.cpp b/coroutine-cwg2935.cpp
index e0c3b8eb6..38a7a560c 100644
--- a/coroutine-cwg2935.cpp
+++ b/coroutine-cwg2935.cpp
@@ -1,387 +1,341 @@
-#include <coroutine>
#include <array>
#include <cassert>
+#include <coroutine>
#include <cstdlib>
-enum check_points
-{
- para_copy_ctor,
- para_dtor,
- promise_ctor,
- promise_dtor,
- get_return_obj,
- task_ctor,
- task_dtor,
- init_suspend,
- init_a_ready,
- init_a_suspend,
- init_a_resume,
- awaiter_ctor,
- awaiter_dtor,
- return_v,
- unhandled,
- fin_suspend
+enum check_points {
+ para_copy_ctor,
+ para_dtor,
+ promise_ctor,
+ promise_dtor,
+ get_return_obj,
+ task_ctor,
+ task_dtor,
+ init_suspend,
+ init_a_ready,
+ init_a_suspend,
+ init_a_resume,
+ awaiter_ctor,
+ awaiter_dtor,
+ return_v,
+ unhandled,
+ fin_suspend
};
using per_test_counts_type = std::array<int, fin_suspend + 1>;
per_test_counts_type per_test_counts{};
-void record(check_points cp)
-{
- // Each checkpoint's executions must be precisely recorded to prevent double free
- ++per_test_counts[cp];
+void record(check_points cp) {
+ // Each checkpoint's executions must be precisely recorded to prevent double
+ // free
+ ++per_test_counts[cp];
}
-void clear()
-{
- per_test_counts = per_test_counts_type{};
-}
+void clear() { per_test_counts = per_test_counts_type{}; }
std::array<bool, fin_suspend + 1> checked_cond{};
-// Each test will throw an exception at a designated location. After the coroutine is
-// invoked, the execution status of all checkpoints will be checked, and then switch
-// to the next test. Before throwing an exception, record the execution status first.
-void throw_c(check_points cp)
-{
- record(cp);
- // Once that point has been tested, it will not be tested again.
- if (checked_cond[cp] == false)
- {
- checked_cond[cp] = true;
- throw 0;
- }
+// Each test will throw an exception at a designated location. After the
+// coroutine is invoked, the execution status of all checkpoints will be
+// checked, and then switch to the next test. Before throwing an exception,
+// record the execution status first.
+void throw_c(check_points cp) {
+ record(cp);
+ // Once that point has been tested, it will not be tested again.
+ if (checked_cond[cp] == false) {
+ checked_cond[cp] = true;
+ throw 0;
+ }
}
std::size_t allocate_count = 0;
-void* operator new(std::size_t size)
-{
- ++allocate_count;
- // When the coroutine is invoked, memory allocation is the first operation performed
- if (void* ptr = std::malloc(size)) {
- return ptr;
- }
- std::abort();
+void *operator new(std::size_t size) {
+ ++allocate_count;
+ // When the coroutine is invoked, memory allocation is the first operation
+ // performed
+ if (void *ptr = std::malloc(size)) {
+ return ptr;
+ }
+ std::abort();
}
-void operator delete(void* ptr) noexcept
-{
- // Deallocation is performed last
- --allocate_count;
- std::free(ptr);
+void operator delete(void *ptr) noexcept {
+ // Deallocation is performed last
+ --allocate_count;
+ std::free(ptr);
}
-struct task
-{
- task() {
- throw_c(task_ctor);
- }
- ~task() {
- record(task_dtor);
- }
- // In this test, the task should be constructed directly, without copy elision
- task(task const&) = delete;
- struct promise_type
- {
- promise_type()
- {
- throw_c(promise_ctor);
- }
- ~promise_type()
- {
- record(promise_dtor);
- }
- promise_type(const promise_type&) = delete;
- task get_return_object()
- {
- throw_c(get_return_obj);
- return {};
- }
- auto initial_suspend()
- {
- throw_c(init_suspend);
- struct initial_awaiter {
- initial_awaiter() {
- throw_c(awaiter_ctor);
- }
- ~initial_awaiter() {
- record(awaiter_dtor);
- }
- initial_awaiter(const initial_awaiter&) = delete;
- bool await_ready()
- {
- throw_c(init_a_ready);
- return false;
- }
- bool await_suspend(std::coroutine_handle<void>)
- {
- throw_c(init_a_suspend);
- return false;
- }
- void await_resume()
- {
- // From this point onward, all exceptions are handled by `unhandled_exception`
- // Since the defect of exceptions escaping from `unhandled_exception` remains
- // unresolved (CWG2934), this test only covers the coroutine startup phase.
- // Once CWG2934 is resolved, further tests can be added based on this one.
- record(init_a_resume);
- }
- };
+struct task {
+ task() { throw_c(task_ctor); }
+ ~task() { record(task_dtor); }
+ // In this test, the task should be constructed directly, without copy elision
+ task(task const &) = delete;
+ struct promise_type {
+ promise_type() { throw_c(promise_ctor); }
+ ~promise_type() { record(promise_dtor); }
+ promise_type(const promise_type &) = delete;
+ task get_return_object() {
+ throw_c(get_return_obj);
+ return {};
+ }
+ auto initial_suspend() {
+ throw_c(init_suspend);
+ struct initial_awaiter {
+ initial_awaiter() { throw_c(awaiter_ctor); }
+ ~initial_awaiter() { record(awaiter_dtor); }
+ initial_awaiter(const initial_awaiter &) = delete;
+ bool await_ready() {
+ throw_c(init_a_ready);
+ return false;
+ }
+ bool await_suspend(std::coroutine_handle<void>) {
+ throw_c(init_a_suspend);
+ return false;
+ }
+ void await_resume() {
+ // From this point onward, all exceptions are handled by
+ // `unhandled_exception` Since the defect of exceptions escaping from
+ // `unhandled_exception` remains unresolved (CWG2934), this test only
+ // covers the coroutine startup phase. Once CWG2934 is resolved,
+ // further tests can be added based on this one.
+ record(init_a_resume);
+ }
+ };
- return initial_awaiter{};
- }
- void return_void()
- {
- record(return_v);
- }
- void unhandled_exception()
- {
- record(unhandled);
- }
- // Note that no exceptions may leak after final_suspend is called, otherwise
- // the behavior is undefined
- std::suspend_never final_suspend() noexcept
- {
- record(fin_suspend);
- return {};
- }
- };
+ return initial_awaiter{};
+ }
+ void return_void() { record(return_v); }
+ void unhandled_exception() { record(unhandled); }
+ // Note that no exceptions may leak after final_suspend is called, otherwise
+ // the behavior is undefined
+ std::suspend_never final_suspend() noexcept {
+ record(fin_suspend);
+ return {};
+ }
+ };
};
-struct copy_observer
-{
+struct copy_observer {
private:
- copy_observer() = default;
+ copy_observer() = default;
+
public:
- copy_observer(copy_observer const&)
- {
- throw_c(para_copy_ctor);
- }
- ~copy_observer()
- {
- record(para_dtor);
- }
+ copy_observer(copy_observer const &) { throw_c(para_copy_ctor); }
+ ~copy_observer() { record(para_dtor); }
- static copy_observer get() {
- return {};
- }
+ static copy_observer get() { return {}; }
};
-//
+//
const auto global_observer = copy_observer::get();
-task coro(copy_observer)
-{
- co_return;
-}
+task coro(copy_observer) { co_return; }
-void catch_coro() try
-{
- coro(global_observer);
-}
-catch (...)
-{
+void catch_coro() try { coro(global_observer); } catch (...) {
}
-// Currently, the conditions at the eight potential exception-throwing points need
-// to be checked. More checkpoints can be added after CWG2934 is resolved.
-int main()
-{
- per_test_counts_type e{};
- allocate_count = 0;
- catch_coro();
- e = {
- 1, // para_copy_ctor
- 0, // para_dtor
- 0, // promise_ctor
- 0, // promise_dtor
- 0, // get_return_obj
- 0, // task_ctor
- 0, // task_dtor
- 0, // init_suspend
- 0, // init_a_ready
- 0, // init_a_suspend
- 0, // init_a_resume
- 0, // awaiter_ctor
- 0, // awaiter_dtor
- 0, // return_v,
- 0, // unhandled,
- 0, // fin_suspend
- };
- assert(e == per_test_counts);
- clear();
- catch_coro();
- e = {
- 2, // para_copy_ctor
- 2, // para_dtor
- 1, // promise_ctor
- 0, // promise_dtor
- 0, // get_return_obj
- 0, // task_ctor
- 0, // task_dtor
- 0, // init_suspend
- 0, // init_a_ready
- 0, // init_a_suspend
- 0, // init_a_resume
- 0, // awaiter_ctor
- 0, // awaiter_dtor
- 0, // return_v,
- 0, // unhandled,
- 0, // fin_suspend
- };
- assert(e == per_test_counts);
- clear();
- catch_coro();
- e = {
- 2, // para_copy_ctor
- 2, // para_dtor
- 1, // promise_ctor
- 1, // promise_dtor
- 1, // get_return_obj
- 0, // task_ctor
- 0, // task_dtor
- 0, // init_suspend
- 0, // init_a_ready
- 0, // init_a_suspend
- 0, // init_a_resume
- 0, // awaiter_ctor
- 0, // awaiter_dtor
- 0, // return_v,
- 0, // unhandled,
- 0, // fin_suspend
- };
- assert(e == per_test_counts);
- clear();
- catch_coro();
- e = {
- 2, // para_copy_ctor
- 2, // para_dtor
- 1, // promise_ctor
- 1, // promise_dtor
- 1, // get_return_obj
- 1, // task_ctor
- 0, // task_dtor
- 0, // init_suspend
- 0, // init_a_ready
- 0, // init_a_suspend
- 0, // init_a_resume
- 0, // awaiter_ctor
- 0, // awaiter_dtor
- 0, // return_v,
- 0, // unhandled,
- 0, // fin_suspend
- };
- assert(e == per_test_counts);
- clear();
- catch_coro();
- e = {
- 2, // para_copy_ctor
- 2, // para_dtor
- 1, // promise_ctor
- 1, // promise_dtor
- 1, // get_return_obj
- 1, // task_ctor
- 1, // task_dtor
- 1, // init_suspend
- 0, // init_a_ready
- 0, // init_a_suspend
- 0, // init_a_resume
- 0, // awaiter_ctor
- 0, // awaiter_dtor
- 0, // return_v,
- 0, // unhandled,
- 0, // fin_suspend
- };
- assert(e == per_test_counts); // Clang currently fails starting from this line. If the code you modified causes tests above this line to fail, it indicates that you have broken the correct code and should start over from scratch.
- clear();
- catch_coro();
- e = {
- 2, // para_copy_ctor
- 2, // para_dtor
- 1, // promise_ctor
- 1, // promise_dtor
- 1, // get_return_obj
- 1, // task_ctor
- 1, // task_dtor
- 1, // init_suspend
- 0, // init_a_ready
- 0, // init_a_suspend
- 0, // init_a_resume
- 1, // awaiter_ctor
- 0, // awaiter_dtor
- 0, // return_v,
- 0, // unhandled,
- 0, // fin_suspend
- };
- assert(e == per_test_counts);
- clear();
- catch_coro();
- e = {
- 2, // para_copy_ctor
- 2, // para_dtor
- 1, // promise_ctor
- 1, // promise_dtor
- 1, // get_return_obj
- 1, // task_ctor
- 1, // task_dtor
- 1, // init_suspend
- 1, // init_a_ready
- 0, // init_a_suspend
- 0, // init_a_resume
- 1, // awaiter_ctor
- 1, // awaiter_dtor
- 0, // return_v,
- 0, // unhandled,
- 0, // fin_suspend
- };
- assert(e == per_test_counts);
- clear();
- catch_coro();
- e = {
- 2, // para_copy_ctor
- 2, // para_dtor
- 1, // promise_ctor
- 1, // promise_dtor
- 1, // get_return_obj
- 1, // task_ctor
- 1, // task_dtor
- 1, // init_suspend
- 1, // init_a_ready
- 1, // init_a_suspend
- 0, // init_a_resume
- 1, // awaiter_ctor
- 1, // awaiter_dtor
- 0, // return_v,
- 0, // unhandled,
- 0, // fin_suspend
- };
- assert(e == per_test_counts);
- clear();
- // Test for execution without exceptions
- {
- coro(global_observer);
- }
- e = {
- 2, // para_copy_ctor
- 2, // para_dtor
- 1, // promise_ctor
- 1, // promise_dtor
- 1, // get_return_obj
- 1, // task_ctor
- 1, // task_dtor
- 1, // init_suspend
- 1, // init_a_ready
- 1, // init_a_suspend
- 1, // init_a_resume
- 1, // awaiter_ctor
- 1, // awaiter_dtor
- 1, // return_v,
- 0, // unhandled,
- 1, // fin_suspend
- };
- assert(e == per_test_counts);
- clear();
- assert(allocate_count == 0);
+// Currently, the conditions at the eight potential exception-throwing points
+// need to be checked. More checkpoints can be added after CWG2934 is resolved.
+int main() {
+ per_test_counts_type e{};
+ allocate_count = 0;
+ catch_coro();
+ e = {
+ 1, // para_copy_ctor
+ 0, // para_dtor
+ 0, // promise_ctor
+ 0, // promise_dtor
+ 0, // get_return_obj
+ 0, // task_ctor
+ 0, // task_dtor
+ 0, // init_suspend
+ 0, // init_a_ready
+ 0, // init_a_suspend
+ 0, // init_a_resume
+ 0, // awaiter_ctor
+ 0, // awaiter_dtor
+ 0, // return_v,
+ 0, // unhandled,
+ 0, // fin_suspend
+ };
+ assert(e == per_test_counts);
+ clear();
+ catch_coro();
+ e = {
+ 2, // para_copy_ctor
+ 2, // para_dtor
+ 1, // promise_ctor
+ 0, // promise_dtor
+ 0, // get_return_obj
+ 0, // task_ctor
+ 0, // task_dtor
+ 0, // init_suspend
+ 0, // init_a_ready
+ 0, // init_a_suspend
+ 0, // init_a_resume
+ 0, // awaiter_ctor
+ 0, // awaiter_dtor
+ 0, // return_v,
+ 0, // unhandled,
+ 0, // fin_suspend
+ };
+ assert(e == per_test_counts);
+ clear();
+ catch_coro();
+ e = {
+ 2, // para_copy_ctor
+ 2, // para_dtor
+ 1, // promise_ctor
+ 1, // promise_dtor
+ 1, // get_return_obj
+ 0, // task_ctor
+ 0, // task_dtor
+ 0, // init_suspend
+ 0, // init_a_ready
+ 0, // init_a_suspend
+ 0, // init_a_resume
+ 0, // awaiter_ctor
+ 0, // awaiter_dtor
+ 0, // return_v,
+ 0, // unhandled,
+ 0, // fin_suspend
+ };
+ assert(e == per_test_counts);
+ clear();
+ catch_coro();
+ e = {
+ 2, // para_copy_ctor
+ 2, // para_dtor
+ 1, // promise_ctor
+ 1, // promise_dtor
+ 1, // get_return_obj
+ 1, // task_ctor
+ 0, // task_dtor
+ 0, // init_suspend
+ 0, // init_a_ready
+ 0, // init_a_suspend
+ 0, // init_a_resume
+ 0, // awaiter_ctor
+ 0, // awaiter_dtor
+ 0, // return_v,
+ 0, // unhandled,
+ 0, // fin_suspend
+ };
+ assert(e == per_test_counts);
+ clear();
+ catch_coro();
+ e = {
+ 2, // para_copy_ctor
+ 2, // para_dtor
+ 1, // promise_ctor
+ 1, // promise_dtor
+ 1, // get_return_obj
+ 1, // task_ctor
+ 1, // task_dtor
+ 1, // init_suspend
+ 0, // init_a_ready
+ 0, // init_a_suspend
+ 0, // init_a_resume
+ 0, // awaiter_ctor
+ 0, // awaiter_dtor
+ 0, // return_v,
+ 0, // unhandled,
+ 0, // fin_suspend
+ };
+ assert(e ==
+ per_test_counts); // Clang currently fails starting from this line. If
+ // the code you modified causes tests above this line
+ // to fail, it indicates that you have broken the
+ // correct code and should start over from scratch.
+ clear();
+ catch_coro();
+ e = {
+ 2, // para_copy_ctor
+ 2, // para_dtor
+ 1, // promise_ctor
+ 1, // promise_dtor
+ 1, // get_return_obj
+ 1, // task_ctor
+ 1, // task_dtor
+ 1, // init_suspend
+ 0, // init_a_ready
+ 0, // init_a_suspend
+ 0, // init_a_resume
+ 1, // awaiter_ctor
+ 0, // awaiter_dtor
+ 0, // return_v,
+ 0, // unhandled,
+ 0, // fin_suspend
+ };
+ assert(e == per_test_counts);
+ clear();
+ catch_coro();
+ e = {
+ 2, // para_copy_ctor
+ 2, // para_dtor
+ 1, // promise_ctor
+ 1, // promise_dtor
+ 1, // get_return_obj
+ 1, // task_ctor
+ 1, // task_dtor
+ 1, // init_suspend
+ 1, // init_a_ready
+ 0, // init_a_suspend
+ 0, // init_a_resume
+ 1, // awaiter_ctor
+ 1, // awaiter_dtor
+ 0, // return_v,
+ 0, // unhandled,
+ 0, // fin_suspend
+ };
+ assert(e == per_test_counts);
+ clear();
+ catch_coro();
+ e = {
+ 2, // para_copy_ctor
+ 2, // para_dtor
+ 1, // promise_ctor
+ 1, // promise_dtor
+ 1, // get_return_obj
+ 1, // task_ctor
+ 1, // task_dtor
+ 1, // init_suspend
+ 1, // init_a_ready
+ 1, // init_a_suspend
+ 0, // init_a_resume
+ 1, // awaiter_ctor
+ 1, // awaiter_dtor
+ 0, // return_v,
+ 0, // unhandled,
+ 0, // fin_suspend
+ };
+ assert(e == per_test_counts);
+ clear();
+ // Test for execution without exceptions
+ {
+ coro(global_observer);
+ }
+ e = {
+ 2, // para_copy_ctor
+ 2, // para_dtor
+ 1, // promise_ctor
+ 1, // promise_dtor
+ 1, // get_return_obj
+ 1, // task_ctor
+ 1, // task_dtor
+ 1, // init_suspend
+ 1, // init_a_ready
+ 1, // init_a_suspend
+ 1, // init_a_resume
+ 1, // awaiter_ctor
+ 1, // awaiter_dtor
+ 1, // return_v,
+ 0, // unhandled,
+ 1, // fin_suspend
+ };
+ assert(e == per_test_counts);
+ clear();
+ assert(allocate_count == 0);
}
\ No newline at end of file
``````````
</details>
https://github.com/llvm/llvm-project/pull/177628
More information about the cfe-commits
mailing list