[clang] 4f4340e - [NFC] [Coroutines] Refactor implementation in checkFinalSuspendNoThrow

Chuanqi Xu via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 13 23:37:20 PST 2022


Author: Chuanqi Xu
Date: 2022-01-14T15:37:01+08:00
New Revision: 4f4340ee2af36909db77aeedb1d22c2ba52d2dfa

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

LOG: [NFC] [Coroutines] Refactor implementation in checkFinalSuspendNoThrow

Now when we are checking if the expression `co_await
promise.final_suspend()` is not throw, we would check unconditionally
for its child expressions recursively. It takes unnecessary time. And
the compiler would complains if the implementation in final_suspend()
may throw even if the higher level function signature marked noexcept
already.

This fixes bug48453 too.

Added: 
    

Modified: 
    clang/lib/Sema/SemaCoroutine.cpp
    clang/test/SemaCXX/coroutine-final-suspend-noexcept-exp-namespace.cpp
    clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index decfb6a5d67d6..e7e60b7e7daf6 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -663,32 +663,32 @@ static void checkNoThrow(Sema &S, const Stmt *E,
       ThrowingDecls.insert(D);
     }
   };
-  auto SC = E->getStmtClass();
-  if (SC == Expr::CXXConstructExprClass) {
-    auto const *Ctor = cast<CXXConstructExpr>(E)->getConstructor();
+
+  if (auto *CE = dyn_cast<CXXConstructExpr>(E)) {
+    CXXConstructorDecl *Ctor = CE->getConstructor();
     checkDeclNoexcept(Ctor);
     // Check the corresponding destructor of the constructor.
-    checkDeclNoexcept(Ctor->getParent()->getDestructor(), true);
-  } else if (SC == Expr::CallExprClass || SC == Expr::CXXMemberCallExprClass ||
-             SC == Expr::CXXOperatorCallExprClass) {
-    if (!cast<CallExpr>(E)->isTypeDependent()) {
-      checkDeclNoexcept(cast<CallExpr>(E)->getCalleeDecl());
-      auto ReturnType = cast<CallExpr>(E)->getCallReturnType(S.getASTContext());
-      // Check the destructor of the call return type, if any.
-      if (ReturnType.isDestructedType() ==
-          QualType::DestructionKind::DK_cxx_destructor) {
-        const auto *T =
-            cast<RecordType>(ReturnType.getCanonicalType().getTypePtr());
-        checkDeclNoexcept(
-            dyn_cast<CXXRecordDecl>(T->getDecl())->getDestructor(), true);
-      }
+    checkDeclNoexcept(Ctor->getParent()->getDestructor(), /*IsDtor=*/true);
+  } else if (auto *CE = dyn_cast<CallExpr>(E)) {
+    if (CE->isTypeDependent())
+      return;
+
+    checkDeclNoexcept(CE->getCalleeDecl());
+    QualType ReturnType = CE->getCallReturnType(S.getASTContext());
+    // Check the destructor of the call return type, if any.
+    if (ReturnType.isDestructedType() ==
+        QualType::DestructionKind::DK_cxx_destructor) {
+      const auto *T =
+          cast<RecordType>(ReturnType.getCanonicalType().getTypePtr());
+      checkDeclNoexcept(dyn_cast<CXXRecordDecl>(T->getDecl())->getDestructor(),
+                        /*IsDtor=*/true);
+    }
+  } else
+    for (const auto *Child : E->children()) {
+      if (!Child)
+        continue;
+      checkNoThrow(S, Child, ThrowingDecls);
     }
-  }
-  for (const auto *Child : E->children()) {
-    if (!Child)
-      continue;
-    checkNoThrow(S, Child, ThrowingDecls);
-  }
 }
 
 bool Sema::checkFinalSuspendNoThrow(const Stmt *FinalSuspend) {

diff  --git a/clang/test/SemaCXX/coroutine-final-suspend-noexcept-exp-namespace.cpp b/clang/test/SemaCXX/coroutine-final-suspend-noexcept-exp-namespace.cpp
index 131bae0d294fe..b501d537b7a8d 100644
--- a/clang/test/SemaCXX/coroutine-final-suspend-noexcept-exp-namespace.cpp
+++ b/clang/test/SemaCXX/coroutine-final-suspend-noexcept-exp-namespace.cpp
@@ -11,32 +11,26 @@ struct coroutine_traits { using promise_type = typename Ret::promise_type; };
 
 template <class Promise = void>
 struct coroutine_handle {
-  static coroutine_handle from_address(void *); // expected-note 2 {{must be declared with 'noexcept'}}
+  static coroutine_handle from_address(void *);
+  void *address() const noexcept;
 };
 template <>
 struct coroutine_handle<void> {
   template <class PromiseType>
-  coroutine_handle(coroutine_handle<PromiseType>); // expected-note 2 {{must be declared with 'noexcept'}}
+  coroutine_handle(coroutine_handle<PromiseType>);
+  void *address() const noexcept;
 };
 
-struct suspend_never {
-  bool await_ready() { return true; }       // expected-note 2 {{must be declared with 'noexcept'}}
+struct suspend_always {
+  bool await_ready() { return false; }      // expected-note 2 {{must be declared with 'noexcept'}}
   void await_suspend(coroutine_handle<>) {} // expected-note 2 {{must be declared with 'noexcept'}}
   void await_resume() {}                    // expected-note 2 {{must be declared with 'noexcept'}}
-  ~suspend_never() noexcept(false);         // expected-note 2 {{must be declared with 'noexcept'}}
-};
-
-struct suspend_always {
-  bool await_ready() { return false; }
-  void await_suspend(coroutine_handle<>) {}
-  void await_resume() {}
-  suspend_never operator co_await(); // expected-note 2 {{must be declared with 'noexcept'}}
-  ~suspend_always() noexcept(false); // expected-note 2 {{must be declared with 'noexcept'}}
+  ~suspend_always() noexcept(false);        // expected-note 2 {{must be declared with 'noexcept'}}
 };
 } // namespace experimental
 } // namespace std
 
-using namespace std;
+using namespace std::experimental;
 
 struct A {
   bool await_ready();
@@ -48,8 +42,8 @@ struct A {
 struct coro_t {
   struct promise_type {
     coro_t get_return_object();
-    std::experimental::suspend_never initial_suspend();
-    std::experimental::suspend_always final_suspend(); // expected-note 2 {{must be declared with 'noexcept'}}
+    suspend_always initial_suspend();
+    suspend_always final_suspend(); // expected-note 2 {{must be declared with 'noexcept'}}
     void return_void();
     static void unhandled_exception();
   };
@@ -69,3 +63,44 @@ coro_t f_dep(T n) { // expected-error {{the expression 'co_await __promise.final
 void foo() {
   f_dep<int>(5); // expected-note {{in instantiation of function template specialization 'f_dep<int>' requested here}}
 }
+
+struct PositiveFinalSuspend {
+  bool await_ready() noexcept;
+  coroutine_handle<> await_suspend(coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+struct correct_coro {
+  struct promise_type {
+    correct_coro get_return_object();
+    suspend_always initial_suspend();
+    PositiveFinalSuspend final_suspend() noexcept;
+    void return_void();
+    static void unhandled_exception();
+  };
+};
+
+correct_coro f2(int n) {
+  co_return;
+}
+
+struct NegativeFinalSuspend {
+  bool await_ready() noexcept;
+  coroutine_handle<> await_suspend(coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+  ~NegativeFinalSuspend() noexcept(false); // expected-note {{must be declared with 'noexcept'}}
+};
+
+struct incorrect_coro {
+  struct promise_type {
+    incorrect_coro get_return_object();
+    suspend_always initial_suspend();
+    NegativeFinalSuspend final_suspend() noexcept;
+    void return_void();
+    static void unhandled_exception();
+  };
+};
+
+incorrect_coro f3(int n) { // expected-error {{the expression 'co_await __promise.final_suspend()' is required to be non-throwing}}
+  co_return;
+}

diff  --git a/clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp b/clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp
index 8635e4156a419..35c00b84ea398 100644
--- a/clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp
+++ b/clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp
@@ -10,27 +10,21 @@ struct coroutine_traits { using promise_type = typename Ret::promise_type; };
 
 template <class Promise = void>
 struct coroutine_handle {
-  static coroutine_handle from_address(void *); // expected-note 2 {{must be declared with 'noexcept'}}
+  static coroutine_handle from_address(void *);
+  void *address() const noexcept;
 };
 template <>
 struct coroutine_handle<void> {
   template <class PromiseType>
-  coroutine_handle(coroutine_handle<PromiseType>); // expected-note 2 {{must be declared with 'noexcept'}}
+  coroutine_handle(coroutine_handle<PromiseType>);
+  void *address() const noexcept;
 };
 
-struct suspend_never {
-  bool await_ready() { return true; }       // expected-note 2 {{must be declared with 'noexcept'}}
+struct suspend_always {
+  bool await_ready() { return false; }      // expected-note 2 {{must be declared with 'noexcept'}}
   void await_suspend(coroutine_handle<>) {} // expected-note 2 {{must be declared with 'noexcept'}}
   void await_resume() {}                    // expected-note 2 {{must be declared with 'noexcept'}}
-  ~suspend_never() noexcept(false);         // expected-note 2 {{must be declared with 'noexcept'}}
-};
-
-struct suspend_always {
-  bool await_ready() { return false; }
-  void await_suspend(coroutine_handle<>) {}
-  void await_resume() {}
-  suspend_never operator co_await(); // expected-note 2 {{must be declared with 'noexcept'}}
-  ~suspend_always() noexcept(false); // expected-note 2 {{must be declared with 'noexcept'}}
+  ~suspend_always() noexcept(false);        // expected-note 2 {{must be declared with 'noexcept'}}
 };
 
 } // namespace std
@@ -47,7 +41,7 @@ struct A {
 struct coro_t {
   struct promise_type {
     coro_t get_return_object();
-    suspend_never initial_suspend();
+    suspend_always initial_suspend();
     suspend_always final_suspend(); // expected-note 2 {{must be declared with 'noexcept'}}
     void return_void();
     static void unhandled_exception();
@@ -68,3 +62,44 @@ coro_t f_dep(T n) { // expected-error {{the expression 'co_await __promise.final
 void foo() {
   f_dep<int>(5); // expected-note {{in instantiation of function template specialization 'f_dep<int>' requested here}}
 }
+
+struct PositiveFinalSuspend {
+  bool await_ready() noexcept;
+  coroutine_handle<> await_suspend(coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+struct correct_coro {
+  struct promise_type {
+    correct_coro get_return_object();
+    suspend_always initial_suspend();
+    PositiveFinalSuspend final_suspend() noexcept;
+    void return_void();
+    static void unhandled_exception();
+  };
+};
+
+correct_coro f2(int n) {
+  co_return;
+}
+
+struct NegativeFinalSuspend {
+  bool await_ready() noexcept;
+  coroutine_handle<> await_suspend(coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+  ~NegativeFinalSuspend() noexcept(false); // expected-note {{must be declared with 'noexcept'}}
+};
+
+struct incorrect_coro {
+  struct promise_type {
+    incorrect_coro get_return_object();
+    suspend_always initial_suspend();
+    NegativeFinalSuspend final_suspend() noexcept;
+    void return_void();
+    static void unhandled_exception();
+  };
+};
+
+incorrect_coro f3(int n) { // expected-error {{the expression 'co_await __promise.final_suspend()' is required to be non-throwing}}
+  co_return;
+}


        


More information about the cfe-commits mailing list