[clang] 4a294b5 - [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (#73160)

[via cfe-commits]

Author: Yuxuan Chen
Date: 2023-11-28T19:04:29-08:00
New Revision: 4a294b5806417aa88c91aa05735b2d557ea5dfe5

URL: https://github.com/llvm/llvm-project/commit/4a294b5806417aa88c91aa05735b2d557ea5dfe5
DIFF: https://github.com/llvm/llvm-project/commit/4a294b5806417aa88c91aa05735b2d557ea5dfe5.diff

LOG: [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<CXXMemberCallExpr>` and as far as I understand it should be a
valid assumption. I can change to that if upstream prefers.

Added: 
    

Modified: 
    clang/lib/CodeGen/CGCoroutine.cpp
    clang/test/CodeGenCoroutines/coro-init-await-nontrivial-return.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index aaf122c0f83bc47..888d30bfb3e1d6a 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<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:
@@ -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 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 {{.*}}
+}