Skip to content

Commit

Permalink
[Clang] CGCoroutines skip emitting try block for value returning `noe…
Browse files Browse the repository at this point in the history
…xcept` init `await_resume` calls (llvm#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<CXXMemberCallExpr>` and as far as I understand it should be a
valid assumption. I can change to that if upstream prefers.
  • Loading branch information
yuxuanchen1997 authored Nov 29, 2023
1 parent 5933589 commit 4a294b5
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 11 deletions.
52 changes: 43 additions & 9 deletions clang/lib/CodeGen/CGCoroutine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<CXXMemberCallExpr>(E))
if (const auto *Proto =
CE->getMethodDecl()->getType()->getAs<FunctionProtoType>())
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<FunctionProtoType>();
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<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;

return false;
}

// Emit suspend expression which roughly looks like:
Expand Down Expand Up @@ -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);
Expand Down
66 changes: 64 additions & 2 deletions clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down Expand Up @@ -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 {{.*}}
}

0 comments on commit 4a294b5

Please sign in to comment.