https://github.com/yuxuanchen1997 updated https://github.com/llvm/llvm-project/pull/73160
>From dbd06bc001c0e00436fcacac231d9d80b443edf4 Mon Sep 17 00:00:00 2001 From: Yuxuan Chen <y...@meta.com> Date: Tue, 21 Nov 2023 21:38:12 -0800 Subject: [PATCH 1/3] add checks for nested noexcept in cxxtempexpr --- clang/lib/CodeGen/CGCoroutine.cpp | 28 ++++++-- .../coro-init-await-nontrivial-return.cpp | 66 ++++++++++++++++++- 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp index aaf122c0f83bc47..37ac5bca38c7049 100644 --- a/clang/lib/CodeGen/CGCoroutine.cpp +++ b/clang/lib/CodeGen/CGCoroutine.cpp @@ -129,12 +129,32 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData &Coro, AwaitKind Kind) { return Prefix; } -static bool memberCallExpressionCanThrow(const Expr *E) { +static bool FunctionProtoNoexcept(const FunctionProtoType *Proto) { + return isNoexceptExceptionSpec(Proto->getExceptionSpecType()) && + Proto->canThrow() == CT_Cannot; +} + +static bool ResumeExprCanThrow(const CoroutineSuspendExpr &S) { + const Expr *E = S.getResumeExpr(); + + // If the return type of await_resume is not void, get the CXXMemberCallExpr + // from its SubExpr. + if (const auto *BindTempExpr = dyn_cast<CXXBindTemporaryExpr>(E)) { + auto *Temporary = BindTempExpr->getTemporary(); + const auto *DtorProto = Temporary->getDestructor() + ->getCanonicalDecl() + ->getType() + ->getAs<FunctionProtoType>(); + bool DtorCanThrow = !FunctionProtoNoexcept(DtorProto); + if (DtorCanThrow) { + return true; + } + E = BindTempExpr->getSubExpr(); + } if (const auto *CE = dyn_cast<CXXMemberCallExpr>(E)) if (const auto *Proto = CE->getMethodDecl()->getType()->getAs<FunctionProtoType>()) - if (isNoexceptExceptionSpec(Proto->getExceptionSpecType()) && - Proto->canThrow() == CT_Cannot) + if (FunctionProtoNoexcept(Proto)) return false; return true; } @@ -233,7 +253,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())) { + ResumeExprCanThrow(S)) { 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 c4b8da327f5c140..052b4e235e739fe 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<promise_type>; @@ -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 <typename R> +struct NoexceptResumeTask { + struct promise_type; + using handle_type = std::coroutine_handle<promise_type>; + + 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<NontrivialType>; + +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<NontrivialTypeWithThrowingDtor>; + +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 {{.*}} +} >From eebbffba771f14c92c90cd023aaeed491f51819f Mon Sep 17 00:00:00 2001 From: Yuxuan Chen <y...@meta.com> Date: Tue, 28 Nov 2023 14:00:40 -0800 Subject: [PATCH 2/3] new way to (conservatively) check stmt no throw --- clang/lib/CodeGen/CGCoroutine.cpp | 63 +++++++++++++++++++------------ 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp index 37ac5bca38c7049..eb9dc3f357eea44 100644 --- a/clang/lib/CodeGen/CGCoroutine.cpp +++ b/clang/lib/CodeGen/CGCoroutine.cpp @@ -129,34 +129,49 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData &Coro, AwaitKind Kind) { return Prefix; } -static bool FunctionProtoNoexcept(const FunctionProtoType *Proto) { - return isNoexceptExceptionSpec(Proto->getExceptionSpecType()) && - Proto->canThrow() == CT_Cannot; +// 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<FunctionProtoType>(); + if (!Proto) { + // Function proto is not found, we conservatively assume throwing. + return true; + } + return !isNoexceptExceptionSpec(Proto->getExceptionSpecType()) || + Proto->canThrow() != CT_Cannot; } -static bool ResumeExprCanThrow(const CoroutineSuspendExpr &S) { - const Expr *E = S.getResumeExpr(); - - // If the return type of await_resume is not void, get the CXXMemberCallExpr - // from its SubExpr. - if (const auto *BindTempExpr = dyn_cast<CXXBindTemporaryExpr>(E)) { - auto *Temporary = BindTempExpr->getTemporary(); - const auto *DtorProto = Temporary->getDestructor() - ->getCanonicalDecl() - ->getType() - ->getAs<FunctionProtoType>(); - bool DtorCanThrow = !FunctionProtoNoexcept(DtorProto); - if (DtorCanThrow) { +static bool ResumeStmtCanThrow(const Stmt *S) { + if (const auto *CE = dyn_cast<CallExpr>(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<CXXBindTemporaryExpr>(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; } - E = BindTempExpr->getSubExpr(); } - if (const auto *CE = dyn_cast<CXXMemberCallExpr>(E)) - if (const auto *Proto = - CE->getMethodDecl()->getType()->getAs<FunctionProtoType>()) - if (FunctionProtoNoexcept(Proto)) - return false; - return true; + return false; } // Emit suspend expression which roughly looks like: @@ -253,7 +268,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 && - ResumeExprCanThrow(S)) { + ResumeStmtCanThrow(S.getResumeExpr())) { Coro.ResumeEHVar = CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh")); Builder.CreateFlagStore(true, Coro.ResumeEHVar); >From cee5adb2a97a009e8234f107a0c5365624cc31f1 Mon Sep 17 00:00:00 2001 From: Yuxuan Chen <y...@meta.com> Date: Tue, 28 Nov 2023 14:36:32 -0800 Subject: [PATCH 3/3] style changes --- clang/lib/CodeGen/CGCoroutine.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp index eb9dc3f357eea44..888d30bfb3e1d6a 100644 --- a/clang/lib/CodeGen/CGCoroutine.cpp +++ b/clang/lib/CodeGen/CGCoroutine.cpp @@ -166,11 +166,10 @@ static bool ResumeStmtCanThrow(const Stmt *S) { // Fall through to visit the children. } - for (const auto *child : S->children()) { - if (ResumeStmtCanThrow(child)) { + for (const auto *child : S->children()) + if (ResumeStmtCanThrow(child)) return true; - } - } + return false; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits