[PATCH] D82332: [Coroutines] Handle dependent promise types for final_suspend non-throw check

2020-06-25 Thread Xun Li via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc25acec84594: [Coroutines] Handle dependent promise types 
for final_suspend non-throw check (authored by lxfind).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82332/new/

https://reviews.llvm.org/D82332

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp

Index: clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp
===
--- clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp
+++ clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp
@@ -11,27 +11,27 @@
 
 template 
 struct coroutine_handle {
-  static coroutine_handle from_address(void *); // expected-note {{must be declared with 'noexcept'}}
+  static coroutine_handle from_address(void *); // expected-note 2 {{must be declared with 'noexcept'}}
 };
 template <>
 struct coroutine_handle {
   template 
-  coroutine_handle(coroutine_handle); // expected-note {{must be declared with 'noexcept'}}
+  coroutine_handle(coroutine_handle); // expected-note 2 {{must be declared with 'noexcept'}}
 };
 
 struct suspend_never {
-  bool await_ready() { return true; }   // expected-note {{must be declared with 'noexcept'}}
-  void await_suspend(coroutine_handle<>) {} // expected-note {{must be declared with 'noexcept'}}
-  void await_resume() {}// expected-note {{must be declared with 'noexcept'}}
-  ~suspend_never() noexcept(false); // expected-note {{must be declared with 'noexcept'}}
+  bool await_ready() { return true; }   // 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 {{must be declared with 'noexcept'}}
-  ~suspend_always() noexcept(false); // expected-note {{must be declared with 'noexcept'}}
+  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'}}
 };
 
 } // namespace experimental
@@ -50,7 +50,7 @@
   struct promise_type {
 coro_t get_return_object();
 suspend_never initial_suspend();
-suspend_always final_suspend(); // expected-note {{must be declared with 'noexcept'}}
+suspend_always final_suspend(); // expected-note 2 {{must be declared with 'noexcept'}}
 void return_void();
 static void unhandled_exception();
   };
@@ -60,3 +60,13 @@
   A a{};
   co_await a;
 }
+
+template 
+coro_t f_dep(T n) { // expected-error {{the expression 'co_await __promise.final_suspend()' is required to be non-throwing}}
+  A a{};
+  co_await a;
+}
+
+void foo() {
+  f_dep(5); // expected-note {{in instantiation of function template specialization 'f_dep' requested here}}
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -7630,7 +7630,8 @@
 return StmtError();
   StmtResult FinalSuspend =
   getDerived().TransformStmt(S->getFinalSuspendStmt());
-  if (FinalSuspend.isInvalid())
+  if (FinalSuspend.isInvalid() ||
+  !SemaRef.checkFinalSuspendNoThrow(FinalSuspend.get()))
 return StmtError();
   ScopeInfo->setCoroutineSuspends(InitSuspend.get(), FinalSuspend.get());
   assert(isa(InitSuspend.get()) && isa(FinalSuspend.get()));
Index: clang/lib/Sema/SemaCoroutine.cpp
===
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -642,7 +642,6 @@
   } else if (SC == Expr::CallExprClass || SC == Expr::CXXMemberCallExprClass ||
  SC == Expr::CXXOperatorCallExprClass) {
 if (!cast(E)->isTypeDependent()) {
-  // FIXME: Handle dependent types.
   checkDeclNoexcept(cast(E)->getCalleeDecl());
   auto ReturnType = cast(E)->getCallReturnType(S.getASTContext());
   // Check the destructor of the call return type, if any.
@@ -662,22 +661,20 @@
   }
 }
 
-/// Check that the expression co_await promise.final_suspend() shall not be
-/// potentially-throwing.
-static bool checkNoThrow(Sema &S, const Stmt *FinalSuspend) {
+bool Sema::checkFinalSuspendNoThrow(const Stmt *FinalSuspend) {
   llvm::SmallPtrSet ThrowingDecls;
   // We first collect all declarations that should not throw but not declared
   // with noexcept. We then sort them 

[PATCH] D82332: [Coroutines] Handle dependent promise types for final_suspend non-throw check

2020-06-25 Thread Brian Gesiak via Phabricator via cfe-commits
modocache accepted this revision.
modocache added a comment.
This revision is now accepted and ready to land.

Thanks, this looks good to me!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82332/new/

https://reviews.llvm.org/D82332



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D82332: [Coroutines] Handle dependent promise types for final_suspend non-throw check

2020-06-23 Thread Xun Li via Phabricator via cfe-commits
lxfind created this revision.
Herald added subscribers: cfe-commits, modocache.
Herald added a project: clang.
lxfind updated this revision to Diff 272553.
lxfind added a comment.
lxfind added reviewers: Benabik, lewissbaker, junparser.
lxfind updated this revision to Diff 272786.
lxfind edited the summary of this revision.
lxfind requested review of this revision.

Simplify tests


lxfind added a comment.

Rebase


Check that the co_await promise.final_suspend() does not potentially throw 
again after we have resolved dependent types.
This takes care of the cases where promises types are templated.
Added test cases for this scenario and confirmed that the checks happen now.
Also run libcxx tests locally to make sure all tests pass.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82332

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp

Index: clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp
===
--- clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp
+++ clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp
@@ -11,27 +11,27 @@
 
 template 
 struct coroutine_handle {
-  static coroutine_handle from_address(void *); // expected-note {{must be declared with 'noexcept'}}
+  static coroutine_handle from_address(void *); // expected-note 2 {{must be declared with 'noexcept'}}
 };
 template <>
 struct coroutine_handle {
   template 
-  coroutine_handle(coroutine_handle); // expected-note {{must be declared with 'noexcept'}}
+  coroutine_handle(coroutine_handle); // expected-note 2 {{must be declared with 'noexcept'}}
 };
 
 struct suspend_never {
-  bool await_ready() { return true; }   // expected-note {{must be declared with 'noexcept'}}
-  void await_suspend(coroutine_handle<>) {} // expected-note {{must be declared with 'noexcept'}}
-  void await_resume() {}// expected-note {{must be declared with 'noexcept'}}
-  ~suspend_never() noexcept(false); // expected-note {{must be declared with 'noexcept'}}
+  bool await_ready() { return true; }   // 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 {{must be declared with 'noexcept'}}
-  ~suspend_always() noexcept(false); // expected-note {{must be declared with 'noexcept'}}
+  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'}}
 };
 
 } // namespace experimental
@@ -50,7 +50,7 @@
   struct promise_type {
 coro_t get_return_object();
 suspend_never initial_suspend();
-suspend_always final_suspend(); // expected-note {{must be declared with 'noexcept'}}
+suspend_always final_suspend(); // expected-note 2 {{must be declared with 'noexcept'}}
 void return_void();
 static void unhandled_exception();
   };
@@ -60,3 +60,13 @@
   A a{};
   co_await a;
 }
+
+template 
+coro_t f_dep(T n) { // expected-error {{the expression 'co_await __promise.final_suspend()' is required to be non-throwing}}
+  A a{};
+  co_await a;
+}
+
+void foo() {
+  f_dep(5); // expected-note {{in instantiation of function template specialization 'f_dep' requested here}}
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -7630,7 +7630,8 @@
 return StmtError();
   StmtResult FinalSuspend =
   getDerived().TransformStmt(S->getFinalSuspendStmt());
-  if (FinalSuspend.isInvalid())
+  if (FinalSuspend.isInvalid() ||
+  !SemaRef.checkFinalSuspendNoThrow(FinalSuspend.get()))
 return StmtError();
   ScopeInfo->setCoroutineSuspends(InitSuspend.get(), FinalSuspend.get());
   assert(isa(InitSuspend.get()) && isa(FinalSuspend.get()));
Index: clang/lib/Sema/SemaCoroutine.cpp
===
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -631,7 +631,6 @@
   } else if (SC == Expr::CallExprClass || SC == Expr::CXXMemberCallExprClass ||
  SC == Expr::CXXOperatorCallExprClass) {
 if (!cast(E)->isTypeDependent()) {
-  // FIXME: Handle dependent types.
   checkDeclNoexcept(cast(E)->getCalleeDecl());
   auto ReturnType = cast(E)->getCallReturnType(S.getASTContext());
   // Ch