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

Yuxuan Chen via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 28 14:36:46 PST 2023


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 <ych at 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 <ych at 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 <ych at 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;
 }
 



More information about the cfe-commits mailing list