[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -129,14 +129,48 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
, 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;
ChuanqiXu9 wrote:
Please don't land it when you have different opinions.
It should be safe to return false directly if we know the function is nothrow.
Then it may be slightly more efficient.
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/yuxuanchen1997 closed https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -129,14 +129,48 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
, 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;
yuxuanchen1997 wrote:
The logic doesn't go this way.
If a function can throw, the task of this (conservative) analysis is to return
true and nothing else needs to be done. Otherwise, fall through and recursively
visit all children, which may include Stmts that throw.
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -129,14 +129,48 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
, 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;
ChuanqiXu9 wrote:
```suggestion
if (!FunctionCanThrow(Callee))
return false;
```
nit: This reads better. In case we know the called function is nothrow, we
don't need to check it further recursively especially await_resume doesn't have
arguments except `this`.
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/ChuanqiXu9 approved this pull request. LGTM with nit. https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
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
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
, 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 ) {
+ 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(E)) {
+auto *Temporary = BindTempExpr->getTemporary();
+const auto *DtorProto = Temporary->getDestructor()
+->getCanonicalDecl()
+->getType()
+->getAs();
+bool DtorCanThrow = !FunctionProtoNoexcept(DtorProto);
+if (DtorCanThrow) {
+ return true;
+}
+E = BindTempExpr->getSubExpr();
+ }
if (const auto *CE = dyn_cast(E))
if (const auto *Proto =
CE->getMethodDecl()->getType()->getAs())
- if (isNoexceptExceptionSpec(Proto->getExceptionSpecType()) &&
- Proto->canThrow() == CT_Cannot)
+ if (FunctionProtoNoexcept(Proto))
return false;
return true;
}
@@ -233,7 +253,7 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction
, CGCoroData
// 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;
@@ -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
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
yuxuanchen1997 wrote: @ChuanqiXu9, I updated the check like you suggested, and it should look much nicer now. The only caveat is that the `children()` call isn't going to cover the implicit destructor call in a `CXXBindTemporaryExpr`. That pattern matching case is here to stay and likely there are many other types implicit calls that make this analysis unsound (See https://eel.is/c++draft/except.spec#6). In reality none of them can reach here though. https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
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
Date: Tue, 21 Nov 2023 21:38:12 -0800
Subject: [PATCH 1/2] 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
, 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 ) {
+ 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(E)) {
+auto *Temporary = BindTempExpr->getTemporary();
+const auto *DtorProto = Temporary->getDestructor()
+->getCanonicalDecl()
+->getType()
+->getAs();
+bool DtorCanThrow = !FunctionProtoNoexcept(DtorProto);
+if (DtorCanThrow) {
+ return true;
+}
+E = BindTempExpr->getSubExpr();
+ }
if (const auto *CE = dyn_cast(E))
if (const auto *Proto =
CE->getMethodDecl()->getType()->getAs())
- if (isNoexceptExceptionSpec(Proto->getExceptionSpecType()) &&
- Proto->canThrow() == CT_Cannot)
+ if (FunctionProtoNoexcept(Proto))
return false;
return true;
}
@@ -233,7 +253,7 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction
, CGCoroData
// 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;
@@ -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
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -129,7 +130,14 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
+static bool ResumeExprCanThrow(const CoroutineSuspendExpr ) {
+ 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(E)) {
+E = BindTempExpr->getSubExpr();
+ }
yuxuanchen1997 wrote:
I guess you are correct that this is easy to miss some cases. In my case I
missed the fact that the destructor of the `BindTempExpr->getTemporary()` can
throw if marked `noexcept(false)`. I updated the PR to cover this case and
including corresponding tests.
Next up I will try to see how many cases I'll need to cover in a recursive
check on `children()` like the one in `Sema::checkFinalSuspendNoThrow`.
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -129,7 +130,14 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
+static bool ResumeExprCanThrow(const CoroutineSuspendExpr ) {
+ 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(E)) {
+E = BindTempExpr->getSubExpr();
+ }
ChuanqiXu9 wrote:
Maybe it is not a such big task as you imaged, you can take a look at
`Sema::checkFinalSuspendNoThrow` and we should be able to make a simpler change
than that.
The reason why I don't like the current pattern is that, every time I wrote:
```
if (auto *WantedExpressionType = isa(E))
E = WantedExpressionType->subExpr();
...
```
I found I always missed some situations.
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -129,7 +130,14 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
+static bool ResumeExprCanThrow(const CoroutineSuspendExpr ) {
+ 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(E)) {
+E = BindTempExpr->getSubExpr();
+ }
apolloww wrote:
I think we are only targeting the resume part of init_suspend here, so the
cases are limited. Can we change the function name to something more specific,
like `InitResumeExprCanThrow`?
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/apolloww deleted https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -129,7 +130,14 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
+static bool ResumeExprCanThrow(const CoroutineSuspendExpr ) {
+ 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(E)) {
+E = BindTempExpr->getSubExpr();
+ }
apolloww wrote:
I think we are only targeting the resume part of `init_suspend` here, so the
cases are limited. Can we change the function name to something more specific,
like `InitResumeExprCanThrow`?
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -38,9 +39,52 @@ 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 @_ZN14NontrivialTypeD1Ev(
+// CHECK-NEXT: call void
@_ZN9can_throw4Task23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN9can_throw14NontrivialTypeD1Ev(
// CHECK-NEXT: store i1 false, ptr {{.*}}
+}
+
+namespace no_throw {
+struct NontrivialType {
+ ~NontrivialType() {}
+};
+
+struct Task {
yuxuanchen1997 wrote:
I applied your suggested change. I think this name is still bikesheddable
though.
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/yuxuanchen1997 updated
https://github.com/llvm/llvm-project/pull/73160
>From 08e2293255a504043fe404cceaeb3ff1fc0dc344 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen
Date: Tue, 21 Nov 2023 21:38:12 -0800
Subject: [PATCH] add checks for nested noexcept in cxxtempexpr
---
clang/lib/CodeGen/CGCoroutine.cpp | 11 -
.../coro-init-await-nontrivial-return.cpp | 44 ++-
2 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp
b/clang/lib/CodeGen/CGCoroutine.cpp
index aaf122c0f83bc47..8aebc5563757cba 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -129,7 +129,14 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
+static bool ResumeExprCanThrow(const CoroutineSuspendExpr ) {
+ 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(E)) {
+E = BindTempExpr->getSubExpr();
+ }
if (const auto *CE = dyn_cast(E))
if (const auto *Proto =
CE->getMethodDecl()->getType()->getAs())
@@ -233,7 +240,7 @@ static LValueOrRValue emitSuspendExpression(CodeGenFunction
, CGCoroData
// 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..5d24841091f339c 100644
--- a/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
+++ b/clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
@@ -7,6 +7,7 @@ struct NontrivialType {
~NontrivialType() {}
};
+namespace can_throw {
struct Task {
struct promise_type;
using handle_type = std::coroutine_handle;
@@ -38,9 +39,48 @@ 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 {{.*}}
+}
+
+namespace no_throw {
+struct InitNoThrowTask {
+struct promise_type;
+using handle_type = std::coroutine_handle;
+
+struct initial_suspend_awaiter {
+bool await_ready() {
+return false;
+}
+
+void await_suspend(handle_type h) {}
+
+NontrivialType 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 {}; }
+InitNoThrowTask get_return_object() {
+return InitNoThrowTask{handle_type::from_promise(*this)};
+}
+};
+
+handle_type handler;
+};
+
+InitNoThrowTask coro_create() {
+co_return;
+}
+
+// CHECK-LABEL: define{{.*}} ptr @_ZN8no_throw11coro_createEv(
+// CHECK: init.ready:
+// CHECK-NEXT: call void
@_ZN8no_throw15InitNoThrowTask23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -129,7 +130,14 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
+static bool ResumeExprCanThrow(const CoroutineSuspendExpr ) {
+ 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(E)) {
+E = BindTempExpr->getSubExpr();
+ }
yuxuanchen1997 wrote:
I find this a little overkill for what we need. The old code here didn't intend
to handle more cases either because it just expected the AST to be in a
specific shape (`CXXMemberCallExpr`). This fix is merely just to amend its
functionality to work on a temporary expression as well should there be a
return value to be discarded. It should be clear intent here that we don't care
about anything else. (Ideally, I want an assertion here that it's either of the
two `Expr` types)
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -38,9 +39,52 @@ 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 @_ZN14NontrivialTypeD1Ev(
+// CHECK-NEXT: call void
@_ZN9can_throw4Task23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN9can_throw14NontrivialTypeD1Ev(
// CHECK-NEXT: store i1 false, ptr {{.*}}
+}
+
+namespace no_throw {
+struct NontrivialType {
+ ~NontrivialType() {}
+};
+
+struct Task {
ChuanqiXu9 wrote:
It looks a little bit confusing. Let's try to rename it to InitNoThrowTask.
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -129,7 +130,14 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
+static bool ResumeExprCanThrow(const CoroutineSuspendExpr ) {
+ 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(E)) {
+E = BindTempExpr->getSubExpr();
+ }
ChuanqiXu9 wrote:
Such pattern match doesn't smell good. How about looking into its children
recursively if we find `E` is not CXXMemberCallExpr?
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -12,9 +12,10 @@ #include "CGCleanup.h" #include "CodeGenFunction.h" -#include "llvm/ADT/ScopeExit.h" +#include "clang/AST/ExprCXX.h" #include "clang/AST/StmtCXX.h" #include "clang/AST/StmtVisitor.h" +#include "llvm/ADT/ScopeExit.h" ChuanqiXu9 wrote: Is this change necessary? https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/yuxuanchen1997 edited https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
