From 4a294b5806417aa88c91aa05735b2d557ea5dfe5 Mon Sep 17 00:00:00 2001 From: Yuxuan Chen Date: Tue, 28 Nov 2023 19:04:29 -0800 Subject: [PATCH] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (#73160) Previously we were not properly skipping the generation of the `try { }` block around the `init_suspend.await_resume()` if the `await_resume` is not returning void. The reason being that the resume expression was wrapped in a `CXXBindTemporaryExpr` and the first dyn_cast failed, silently ignoring the noexcept. This only mattered for `init_suspend` because it had its own try block. This patch changes to first extract the sub expression when we see a `CXXBindTemporaryExpr`. Then perform the same logic to check for `noexcept`. Another version of this patch also wanted to assert the second step by `cast` and as far as I understand it should be a valid assumption. I can change to that if upstream prefers. --- clang/lib/CodeGen/CGCoroutine.cpp | 52 ++++++++++++--- .../coro-init-await-nontrivial-return.cpp | 66 ++++++++++++++++++- 2 files changed, 107 insertions(+), 11 deletions(-) diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp index aaf122c0f83bc4..888d30bfb3e1d6 100644 --- a/clang/lib/CodeGen/CGCoroutine.cpp +++ b/clang/lib/CodeGen/CGCoroutine.cpp @@ -129,14 +129,48 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData &Coro, AwaitKind Kind) { return Prefix; } -static bool memberCallExpressionCanThrow(const Expr *E) { - if (const auto *CE = dyn_cast(E)) - if (const auto *Proto = - CE->getMethodDecl()->getType()->getAs()) - if (isNoexceptExceptionSpec(Proto->getExceptionSpecType()) && - Proto->canThrow() == CT_Cannot) - return false; - return true; +// Check if function can throw based on prototype noexcept, also works for +// destructors which are implicitly noexcept but can be marked noexcept(false). +static bool FunctionCanThrow(const FunctionDecl *D) { + const auto *Proto = D->getType()->getAs(); + if (!Proto) { + // Function proto is not found, we conservatively assume throwing. + return true; + } + return !isNoexceptExceptionSpec(Proto->getExceptionSpecType()) || + Proto->canThrow() != CT_Cannot; +} + +static bool ResumeStmtCanThrow(const Stmt *S) { + if (const auto *CE = dyn_cast(S)) { + const auto *Callee = CE->getDirectCallee(); + if (!Callee) + // We don't have direct callee. Conservatively assume throwing. + return true; + + if (FunctionCanThrow(Callee)) + return true; + + // Fall through to visit the children. + } + + if (const auto *TE = dyn_cast(S)) { + // Special handling of CXXBindTemporaryExpr here as calling of Dtor of the + // temporary is not part of `children()` as covered in the fall through. + // We need to mark entire statement as throwing if the destructor of the + // temporary throws. + const auto *Dtor = TE->getTemporary()->getDestructor(); + if (FunctionCanThrow(Dtor)) + return true; + + // Fall through to visit the children. + } + + for (const auto *child : S->children()) + if (ResumeStmtCanThrow(child)) + return true; + + return false; } // Emit suspend expression which roughly looks like: @@ -233,7 +267,7 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction &CGF, CGCoroData &Co // is marked as 'noexcept', we avoid generating this additional IR. CXXTryStmt *TryStmt = nullptr; if (Coro.ExceptionHandler && Kind == AwaitKind::Init && - memberCallExpressionCanThrow(S.getResumeExpr())) { + ResumeStmtCanThrow(S.getResumeExpr())) { Coro.ResumeEHVar = CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh")); Builder.CreateFlagStore(true, Coro.ResumeEHVar); diff --git a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp index c4b8da327f5c14..052b4e235e739f 100644 --- a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp +++ b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp @@ -7,6 +7,11 @@ struct NontrivialType { ~NontrivialType() {} }; +struct NontrivialTypeWithThrowingDtor { + ~NontrivialTypeWithThrowingDtor() noexcept(false) {} +}; + +namespace can_throw { struct Task { struct promise_type; using handle_type = std::coroutine_handle; @@ -38,9 +43,66 @@ Task coro_create() { co_return; } -// CHECK-LABEL: define{{.*}} ptr @_Z11coro_createv( +// CHECK-LABEL: define{{.*}} ptr @_ZN9can_throw11coro_createEv( // CHECK: init.ready: // CHECK-NEXT: store i1 true, ptr {{.*}} -// CHECK-NEXT: call void @_ZN4Task23initial_suspend_awaiter12await_resumeEv( +// CHECK-NEXT: call void @_ZN9can_throw4Task23initial_suspend_awaiter12await_resumeEv( // CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev( // CHECK-NEXT: store i1 false, ptr {{.*}} +} + +template +struct NoexceptResumeTask { + struct promise_type; + using handle_type = std::coroutine_handle; + + struct initial_suspend_awaiter { + bool await_ready() { + return false; + } + + void await_suspend(handle_type h) {} + + R await_resume() noexcept { return {}; } + }; + + struct promise_type { + void return_void() {} + void unhandled_exception() {} + initial_suspend_awaiter initial_suspend() { return {}; } + std::suspend_never final_suspend() noexcept { return {}; } + NoexceptResumeTask get_return_object() { + return NoexceptResumeTask{handle_type::from_promise(*this)}; + } + }; + + handle_type handler; +}; + +namespace no_throw { +using InitNoThrowTask = NoexceptResumeTask; + +InitNoThrowTask coro_create() { + co_return; +} + +// CHECK-LABEL: define{{.*}} ptr @_ZN8no_throw11coro_createEv( +// CHECK: init.ready: +// CHECK-NEXT: call void @_ZN18NoexceptResumeTaskI14NontrivialTypeE23initial_suspend_awaiter12await_resumeEv( +// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev( +} + +namespace throwing_dtor { +using InitTaskWithThrowingDtor = NoexceptResumeTask; + +InitTaskWithThrowingDtor coro_create() { + co_return; +} + +// CHECK-LABEL: define{{.*}} ptr @_ZN13throwing_dtor11coro_createEv( +// CHECK: init.ready: +// CHECK-NEXT: store i1 true, ptr {{.*}} +// CHECK-NEXT: call void @_ZN18NoexceptResumeTaskI30NontrivialTypeWithThrowingDtorE23initial_suspend_awaiter12await_resumeEv( +// CHECK-NEXT: call void @_ZN30NontrivialTypeWithThrowingDtorD1Ev( +// CHECK-NEXT: store i1 false, ptr {{.*}} +}