[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

In D82029#2109167 , @lxfind wrote:

> Test failures are being fixed in https://reviews.llvm.org/D82338/new/


Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82029



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

[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

[Xun Li via Phabricator via cfe-commits]

lxfind added a comment.

Test failures are being fixed in https://reviews.llvm.org/D82338/new/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82029



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

[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

Hi!

I see a bunch of failures when I run libcxx testcases with this patch. Should 
there be "noexcept" on a number of more places?

Failed Tests (8):

  libc++ :: 
libcxx/experimental/language.support/support.coroutines/dialect_support.pass.cpp
  libc++ :: 
std/experimental/language.support/support.coroutines/coroutine.handle/coroutine.handle.prom/promise.pass.cpp
  libc++ :: 
std/experimental/language.support/support.coroutines/end.to.end/await_result.pass.cpp
  libc++ :: 
std/experimental/language.support/support.coroutines/end.to.end/bool_await_suspend.pass.cpp
  libc++ :: 
std/experimental/language.support/support.coroutines/end.to.end/expected.pass.cpp
  libc++ :: 
std/experimental/language.support/support.coroutines/end.to.end/fullexpr-dtor.pass.cpp
  libc++ :: 
std/experimental/language.support/support.coroutines/end.to.end/generator.pass.cpp
  libc++ :: 
std/experimental/language.support/support.coroutines/end.to.end/go.pass.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82029



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

[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

[Xun Li via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG516803dc8685: [Coroutines] Ensure co_await 
promise.final_suspend() does not throw (authored by lxfind).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82029

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/test/AST/Inputs/std-coroutine.h
  clang/test/AST/coroutine-source-location-crash.cpp
  clang/test/Analysis/more-dtors-cfg-output.cpp
  clang/test/CodeGenCXX/ubsan-coroutines.cpp
  clang/test/CodeGenCoroutines/Inputs/coroutine.h
  clang/test/CodeGenCoroutines/coro-alloc.cpp
  clang/test/CodeGenCoroutines/coro-always-inline.cpp
  clang/test/CodeGenCoroutines/coro-await-domination.cpp
  clang/test/CodeGenCoroutines/coro-await-resume-eh.cpp
  clang/test/CodeGenCoroutines/coro-await.cpp
  clang/test/CodeGenCoroutines/coro-dest-slot.cpp
  clang/test/CodeGenCoroutines/coro-gro-nrvo.cpp
  clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp
  clang/test/CodeGenCoroutines/coro-params.cpp
  clang/test/CodeGenCoroutines/coro-promise-dtor.cpp
  clang/test/CodeGenCoroutines/coro-ret-void.cpp
  clang/test/CodeGenCoroutines/coro-return-voidtype-initlist.cpp
  clang/test/CodeGenCoroutines/coro-return.cpp
  clang/test/CodeGenCoroutines/coro-unhandled-exception.cpp
  clang/test/Index/coroutines.cpp
  clang/test/SemaCXX/Inputs/std-coroutine.h
  clang/test/SemaCXX/co_await-range-for.cpp
  clang/test/SemaCXX/coreturn-eh.cpp
  clang/test/SemaCXX/coreturn.cpp
  clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp
  clang/test/SemaCXX/coroutine-rvo.cpp
  clang/test/SemaCXX/coroutine-unhandled_exception-warning.cpp
  clang/test/SemaCXX/coroutine-uninitialized-warning-crash.cpp
  clang/test/SemaCXX/coroutines.cpp

Index: clang/test/SemaCXX/coroutines.cpp
===
--- clang/test/SemaCXX/coroutines.cpp
+++ clang/test/SemaCXX/coroutines.cpp
@@ -52,21 +52,24 @@
 };
 
 struct awaitable {
-  bool await_ready();
-  template  void await_suspend(F);
-  void await_resume();
+  bool await_ready() noexcept;
+  template 
+  void await_suspend(F) noexcept;
+  void await_resume() noexcept;
 } a;
 
 struct suspend_always {
-  bool await_ready() { return false; }
-  template  void await_suspend(F);
-  void await_resume() {}
+  bool await_ready() noexcept { return false; }
+  template 
+  void await_suspend(F) noexcept;
+  void await_resume() noexcept {}
 };
 
 struct suspend_never {
-  bool await_ready() { return true; }
-  template  void await_suspend(F);
-  void await_resume() {}
+  bool await_ready() noexcept { return true; }
+  template 
+  void await_suspend(F) noexcept;
+  void await_resume() noexcept {}
 };
 
 struct auto_await_suspend {
@@ -127,7 +130,7 @@
 struct promise {
   void get_return_object();
   suspend_always initial_suspend();
-  suspend_always final_suspend();
+  suspend_always final_suspend() noexcept;
   awaitable yield_value(int); // expected-note 2{{candidate}}
   awaitable yield_value(yielded_thing); // expected-note 2{{candidate}}
   not_awaitable yield_value(void()); // expected-note 2{{candidate}}
@@ -138,7 +141,7 @@
 struct promise_void {
   void get_return_object();
   suspend_always initial_suspend();
-  suspend_always final_suspend();
+  suspend_always final_suspend() noexcept;
   void return_void();
   void unhandled_exception();
 };
@@ -152,13 +155,13 @@
 namespace experimental {
 template 
 struct coroutine_handle {
-  static coroutine_handle from_address(void *);
+  static coroutine_handle from_address(void *) noexcept;
 };
 template <>
 struct coroutine_handle {
   template 
-  coroutine_handle(coroutine_handle);
-  static coroutine_handle from_address(void *);
+  coroutine_handle(coroutine_handle) noexcept;
+  static coroutine_handle from_address(void *) noexcept;
 };
 }} // namespace std::experimental
 
@@ -402,7 +405,7 @@
 
 namespace adl_ns {
 struct coawait_arg_type {};
-awaitable operator co_await(coawait_arg_type);
+awaitable operator co_await(coawait_arg_type) noexcept;
 }
 
 namespace dependent_operator_co_await_lookup {
@@ -434,7 +437,7 @@
 typedef transform_awaitable await_arg;
 coro get_return_object();
 transformed initial_suspend();
-::adl_ns::coawait_arg_type final_suspend();
+::adl_ns::coawait_arg_type final_suspend() noexcept;
 transformed await_transform(transform_awaitable);
 void unhandled_exception();
 void return_void();
@@ -444,7 +447,7 @@
 typedef AwaitArg await_arg;
 coro get_return_object();
 awaitable initial_suspend();
-awaitable final_suspend();
+awaitable final_suspend() noexcept;
 void unhandled_exception();
 void return_void();
   };
@@ -529,7 +532,7 @@
 void return_value(int());
 
 suspend_never initial_suspend();
-suspend_never final_suspend();

[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

[Brian Gesiak via Phabricator via cfe-commits]

modocache accepted this revision.
modocache added a comment.
This revision is now accepted and ready to land.

Sweet! Thanks for the reviews/responses, LGTM :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82029



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

[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

[Xun Li via Phabricator via cfe-commits]

lxfind added a comment.

All tests are passing now. Thanks for reviewing!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82029



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

[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

[JunMa via Phabricator via cfe-commits]

junparser added a comment.

In D82029#2100675 , @modocache wrote:

> Excellent, thank you! The test failures on the diff appear to be legitimate, 
> they reproduce for me when I apply this patch to my local checkout and run 
> `ninja check-clang`. Could you take a look?
>
> This LGTM otherwise! Also adding @junparser in case they have any thoughts, 
> if not I'll accept after the test failures are addressed, Thanks!


@modocache LGTM, @lxfind Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82029



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

[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

[Xun Li via Phabricator via cfe-commits]

lxfind updated this revision to Diff 272109.
lxfind added a comment.

Addressed comments: Updated error message, and sorted notes. The tests are kept 
unchanged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82029

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/test/AST/Inputs/std-coroutine.h
  clang/test/AST/coroutine-source-location-crash.cpp
  clang/test/Analysis/more-dtors-cfg-output.cpp
  clang/test/CodeGenCXX/ubsan-coroutines.cpp
  clang/test/CodeGenCoroutines/Inputs/coroutine.h
  clang/test/CodeGenCoroutines/coro-alloc.cpp
  clang/test/CodeGenCoroutines/coro-always-inline.cpp
  clang/test/CodeGenCoroutines/coro-await-domination.cpp
  clang/test/CodeGenCoroutines/coro-await-resume-eh.cpp
  clang/test/CodeGenCoroutines/coro-await.cpp
  clang/test/CodeGenCoroutines/coro-dest-slot.cpp
  clang/test/CodeGenCoroutines/coro-gro-nrvo.cpp
  clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp
  clang/test/CodeGenCoroutines/coro-params.cpp
  clang/test/CodeGenCoroutines/coro-promise-dtor.cpp
  clang/test/CodeGenCoroutines/coro-ret-void.cpp
  clang/test/CodeGenCoroutines/coro-return-voidtype-initlist.cpp
  clang/test/CodeGenCoroutines/coro-return.cpp
  clang/test/CodeGenCoroutines/coro-unhandled-exception.cpp
  clang/test/Index/coroutines.cpp
  clang/test/SemaCXX/Inputs/std-coroutine.h
  clang/test/SemaCXX/co_await-range-for.cpp
  clang/test/SemaCXX/coreturn-eh.cpp
  clang/test/SemaCXX/coreturn.cpp
  clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp
  clang/test/SemaCXX/coroutine-rvo.cpp
  clang/test/SemaCXX/coroutine-unhandled_exception-warning.cpp
  clang/test/SemaCXX/coroutine-uninitialized-warning-crash.cpp
  clang/test/SemaCXX/coroutines.cpp

Index: clang/test/SemaCXX/coroutines.cpp
===
--- clang/test/SemaCXX/coroutines.cpp
+++ clang/test/SemaCXX/coroutines.cpp
@@ -52,21 +52,24 @@
 };
 
 struct awaitable {
-  bool await_ready();
-  template  void await_suspend(F);
-  void await_resume();
+  bool await_ready() noexcept;
+  template 
+  void await_suspend(F) noexcept;
+  void await_resume() noexcept;
 } a;
 
 struct suspend_always {
-  bool await_ready() { return false; }
-  template  void await_suspend(F);
-  void await_resume() {}
+  bool await_ready() noexcept { return false; }
+  template 
+  void await_suspend(F) noexcept;
+  void await_resume() noexcept {}
 };
 
 struct suspend_never {
-  bool await_ready() { return true; }
-  template  void await_suspend(F);
-  void await_resume() {}
+  bool await_ready() noexcept { return true; }
+  template 
+  void await_suspend(F) noexcept;
+  void await_resume() noexcept {}
 };
 
 struct auto_await_suspend {
@@ -127,7 +130,7 @@
 struct promise {
   void get_return_object();
   suspend_always initial_suspend();
-  suspend_always final_suspend();
+  suspend_always final_suspend() noexcept;
   awaitable yield_value(int); // expected-note 2{{candidate}}
   awaitable yield_value(yielded_thing); // expected-note 2{{candidate}}
   not_awaitable yield_value(void()); // expected-note 2{{candidate}}
@@ -138,7 +141,7 @@
 struct promise_void {
   void get_return_object();
   suspend_always initial_suspend();
-  suspend_always final_suspend();
+  suspend_always final_suspend() noexcept;
   void return_void();
   void unhandled_exception();
 };
@@ -152,13 +155,13 @@
 namespace experimental {
 template 
 struct coroutine_handle {
-  static coroutine_handle from_address(void *);
+  static coroutine_handle from_address(void *) noexcept;
 };
 template <>
 struct coroutine_handle {
   template 
-  coroutine_handle(coroutine_handle);
-  static coroutine_handle from_address(void *);
+  coroutine_handle(coroutine_handle) noexcept;
+  static coroutine_handle from_address(void *) noexcept;
 };
 }} // namespace std::experimental
 
@@ -402,7 +405,7 @@
 
 namespace adl_ns {
 struct coawait_arg_type {};
-awaitable operator co_await(coawait_arg_type);
+awaitable operator co_await(coawait_arg_type) noexcept;
 }
 
 namespace dependent_operator_co_await_lookup {
@@ -434,7 +437,7 @@
 typedef transform_awaitable await_arg;
 coro get_return_object();
 transformed initial_suspend();
-::adl_ns::coawait_arg_type final_suspend();
+::adl_ns::coawait_arg_type final_suspend() noexcept;
 transformed await_transform(transform_awaitable);
 void unhandled_exception();
 void return_void();
@@ -444,7 +447,7 @@
 typedef AwaitArg await_arg;
 coro get_return_object();
 awaitable initial_suspend();
-awaitable final_suspend();
+awaitable final_suspend() noexcept;
 void unhandled_exception();
 void return_void();
   };
@@ -529,7 +532,7 @@
 void return_value(int());
 
 suspend_never initial_suspend();
-suspend_never final_suspend();
+suspend_never final_suspend() 

[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

[Xun Li via Phabricator via cfe-commits]

lxfind marked an inline comment as done.
lxfind added inline comments.



Comment at: clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp:14
+struct coroutine_handle {
+  static coroutine_handle from_address(void *); // expected-note {{must be 
declared with 'noexcept'}}
+};

lewissbaker wrote:
> I'm not sure that we should be _requiring_ the compiler to emit an error for 
> this line.
> 
> The language specification does not require implementations to declare the 
> from_address() method as noexcept, even though Clang now requires standard 
> library implementations to declare this method as noexcept - this is an 
> additional implementation requirement that Clang is placing on standard 
> library implementations for them to be compatible with Clang's coroutines 
> implementation.
> 
> I guess this is probably ok to raise as an error, though, as most users will 
> just be using the compiler-provided implementation and both libc++/libstdc++ 
> are (currently) compatible.
`from_address` is in fact called as part of `co_await 
__promise.final_suspend()`.
Specifically, when this `co_await` is translated into a call to 
`finalSuspendObjAwaiter.await_suspend(handle)`, the handle parameter is 
obtained by calling `from_address`.
Since the specification requires that `co_await __promise.final_suspend()` 
should not be potentially throwing, it must requires `from_address` to be 
declared as `noexcept`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82029



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

[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

[Lewis Baker via Phabricator via cfe-commits]

lewissbaker added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10521
+def err_coroutine_promise_final_suspend_requires_nothrow : Error<
+  "all code generated by co_await __promise.final_suspend() must not throw"
+>;

I'd perhaps word this to be more direct in what the compiler is requiring.
It's possible to have a function that does not throw that is not declared 
noexcept, but what the compiler requires here is that the expression is 
noexcept - or in standardese "not potentially throwing".

So maybe something like:
> the expression 'co_await __promise.final_suspend()' is required to be 
> non-throwing




Comment at: clang/lib/Sema/SemaCoroutine.cpp:662-664
+  for (const auto *D : ThrowingDecls) {
+S.Diag(D->getEndLoc(), diag::note_coroutine_function_declare_noexcept);
+  }

Should we be sorting the ThrowingDecls by something other than the pointer so 
that the error output has a deterministic order here?



Comment at: clang/lib/Sema/SemaExceptionSpec.cpp:1049
 
-  FT = S.ResolveExceptionSpec(Loc.isInvalid() ? E->getBeginLoc() : Loc, FT);
+  if (Loc.isValid() || (Loc.isInvalid() && E)) {
+FT = S.ResolveExceptionSpec(Loc.isInvalid() ? E->getBeginLoc() : Loc, FT);

The function documentation says at least one of E and Loc must be true.
Should this be an assertion, then, rather than a conditional check?



Comment at: clang/test/AST/Inputs/std-coroutine.h:13
 struct coroutine_handle {
-  static coroutine_handle from_address(void *);
+  static coroutine_handle from_address(void *) noexcept;
 };

Note that the actual `std::coroutine_handle::from_address()` method is not 
specified to have a noexcept declaration, even though it is defined as such in 
both libstdc++/libc++ implementation.

See http://eel.is/c++draft/coroutine.handle#export.import-itemdecl:2

Note, however, that the specification doesn't define a co_await expression in 
terms of coroutine_handle::from_address() or coroutine_handle::from_promise(), 
but is rather just defined in terms of some handle 'h' that refers to the 
current coroutine.

See http://eel.is/c++draft/expr.await#3.5

So while technically we shouldn't be requiring the from_address() method to be 
noexcept, I think that it's probably ok since the compiler, at least in theory, 
has control over how it constructs a handle and can choose to call the 
from_address() method assuming that it is defined noexcept, even though it is 
not required to be.




Comment at: clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp:14
+struct coroutine_handle {
+  static coroutine_handle from_address(void *); // expected-note {{must be 
declared with 'noexcept'}}
+};

I'm not sure that we should be _requiring_ the compiler to emit an error for 
this line.

The language specification does not require implementations to declare the 
from_address() method as noexcept, even though Clang now requires standard 
library implementations to declare this method as noexcept - this is an 
additional implementation requirement that Clang is placing on standard library 
implementations for them to be compatible with Clang's coroutines 
implementation.

I guess this is probably ok to raise as an error, though, as most users will 
just be using the compiler-provided implementation and both libc++/libstdc++ 
are (currently) compatible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82029



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

[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

[Xun Li via Phabricator via cfe-commits]

lxfind updated this revision to Diff 271801.
lxfind added a comment.
Herald added a subscriber: arphaman.

Address feedback and update failed tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82029

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/test/AST/Inputs/std-coroutine.h
  clang/test/AST/coroutine-source-location-crash.cpp
  clang/test/Analysis/more-dtors-cfg-output.cpp
  clang/test/CodeGenCXX/ubsan-coroutines.cpp
  clang/test/CodeGenCoroutines/Inputs/coroutine.h
  clang/test/CodeGenCoroutines/coro-alloc.cpp
  clang/test/CodeGenCoroutines/coro-always-inline.cpp
  clang/test/CodeGenCoroutines/coro-await-domination.cpp
  clang/test/CodeGenCoroutines/coro-await-resume-eh.cpp
  clang/test/CodeGenCoroutines/coro-await.cpp
  clang/test/CodeGenCoroutines/coro-dest-slot.cpp
  clang/test/CodeGenCoroutines/coro-gro-nrvo.cpp
  clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp
  clang/test/CodeGenCoroutines/coro-params.cpp
  clang/test/CodeGenCoroutines/coro-promise-dtor.cpp
  clang/test/CodeGenCoroutines/coro-ret-void.cpp
  clang/test/CodeGenCoroutines/coro-return-voidtype-initlist.cpp
  clang/test/CodeGenCoroutines/coro-return.cpp
  clang/test/CodeGenCoroutines/coro-unhandled-exception.cpp
  clang/test/Index/coroutines.cpp
  clang/test/SemaCXX/Inputs/std-coroutine.h
  clang/test/SemaCXX/co_await-range-for.cpp
  clang/test/SemaCXX/coreturn-eh.cpp
  clang/test/SemaCXX/coreturn.cpp
  clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp
  clang/test/SemaCXX/coroutine-rvo.cpp
  clang/test/SemaCXX/coroutine-unhandled_exception-warning.cpp
  clang/test/SemaCXX/coroutine-uninitialized-warning-crash.cpp
  clang/test/SemaCXX/coroutines.cpp

Index: clang/test/SemaCXX/coroutines.cpp
===
--- clang/test/SemaCXX/coroutines.cpp
+++ clang/test/SemaCXX/coroutines.cpp
@@ -52,21 +52,24 @@
 };
 
 struct awaitable {
-  bool await_ready();
-  template  void await_suspend(F);
-  void await_resume();
+  bool await_ready() noexcept;
+  template 
+  void await_suspend(F) noexcept;
+  void await_resume() noexcept;
 } a;
 
 struct suspend_always {
-  bool await_ready() { return false; }
-  template  void await_suspend(F);
-  void await_resume() {}
+  bool await_ready() noexcept { return false; }
+  template 
+  void await_suspend(F) noexcept;
+  void await_resume() noexcept {}
 };
 
 struct suspend_never {
-  bool await_ready() { return true; }
-  template  void await_suspend(F);
-  void await_resume() {}
+  bool await_ready() noexcept { return true; }
+  template 
+  void await_suspend(F) noexcept;
+  void await_resume() noexcept {}
 };
 
 struct auto_await_suspend {
@@ -127,7 +130,7 @@
 struct promise {
   void get_return_object();
   suspend_always initial_suspend();
-  suspend_always final_suspend();
+  suspend_always final_suspend() noexcept;
   awaitable yield_value(int); // expected-note 2{{candidate}}
   awaitable yield_value(yielded_thing); // expected-note 2{{candidate}}
   not_awaitable yield_value(void()); // expected-note 2{{candidate}}
@@ -138,7 +141,7 @@
 struct promise_void {
   void get_return_object();
   suspend_always initial_suspend();
-  suspend_always final_suspend();
+  suspend_always final_suspend() noexcept;
   void return_void();
   void unhandled_exception();
 };
@@ -152,13 +155,13 @@
 namespace experimental {
 template 
 struct coroutine_handle {
-  static coroutine_handle from_address(void *);
+  static coroutine_handle from_address(void *) noexcept;
 };
 template <>
 struct coroutine_handle {
   template 
-  coroutine_handle(coroutine_handle);
-  static coroutine_handle from_address(void *);
+  coroutine_handle(coroutine_handle) noexcept;
+  static coroutine_handle from_address(void *) noexcept;
 };
 }} // namespace std::experimental
 
@@ -402,7 +405,7 @@
 
 namespace adl_ns {
 struct coawait_arg_type {};
-awaitable operator co_await(coawait_arg_type);
+awaitable operator co_await(coawait_arg_type) noexcept;
 }
 
 namespace dependent_operator_co_await_lookup {
@@ -434,7 +437,7 @@
 typedef transform_awaitable await_arg;
 coro get_return_object();
 transformed initial_suspend();
-::adl_ns::coawait_arg_type final_suspend();
+::adl_ns::coawait_arg_type final_suspend() noexcept;
 transformed await_transform(transform_awaitable);
 void unhandled_exception();
 void return_void();
@@ -444,7 +447,7 @@
 typedef AwaitArg await_arg;
 coro get_return_object();
 awaitable initial_suspend();
-awaitable final_suspend();
+awaitable final_suspend() noexcept;
 void unhandled_exception();
 void return_void();
   };
@@ -529,7 +532,7 @@
 void return_value(int());
 
 suspend_never initial_suspend();
-suspend_never final_suspend();
+suspend_never final_suspend() noexcept;

[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

[Brian Gesiak via Phabricator via cfe-commits]

modocache added a reviewer: junparser.
modocache added a subscriber: junparser.
modocache added a comment.

Excellent, thank you! The test failures on the diff appear to be legitimate, 
they reproduce for me when I apply this patch to my local checkout and run 
`ninja check-clang`. Could you take a look?

This LGTM otherwise! Also adding @junparser in case they have any thoughts, if 
not I'll accept after the test failures are addressed, Thanks!




Comment at: clang/lib/Sema/SemaExceptionSpec.cpp:1051
+FT = S.ResolveExceptionSpec(Loc.isInvalid() ? E->getBeginLoc() : Loc, FT);
+  }
   if (!FT)

Small nit-pick: Omit the `{` and `}` here, as that's the convention in the 
surrounding lines of this file and in the LLVM project in general.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82029



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

[PATCH] D82029: [Coroutines] Ensure co_await promise.final_suspend() does not throw

[Xun Li via Phabricator via cfe-commits]

lxfind created this revision.
lxfind added reviewers: lewissbaker, modocache.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch addresses https://bugs.llvm.org/show_bug.cgi?id=46256
The spec of coroutine requires that the expression co_­await 
promise.final_­suspend() shall not be potentially-throwing.
To check this, we recursively look at every call (including Call, MemberCall, 
OperatorCall and Constructor) in all code
generated by the final suspend, and ensure that the callees are declared with 
noexcept. We also look at any returned data
type that requires explicit destruction, and check their destructors for 
noexcept.

This patch does not check declarations with dependent types yet, which will be 
done in future patches.

Updated all tests to add noexcept to the required functions, and added a 
dedicated test for this patch.

This patch might start to cause existing codebase fail to compile because most 
people may not have been strict in tagging
all the related functions noexcept.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82029

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/test/AST/Inputs/std-coroutine.h
  clang/test/AST/coroutine-source-location-crash.cpp
  clang/test/CodeGenCXX/ubsan-coroutines.cpp
  clang/test/CodeGenCoroutines/Inputs/coroutine.h
  clang/test/CodeGenCoroutines/coro-alloc.cpp
  clang/test/CodeGenCoroutines/coro-always-inline.cpp
  clang/test/CodeGenCoroutines/coro-await-domination.cpp
  clang/test/CodeGenCoroutines/coro-await-resume-eh.cpp
  clang/test/CodeGenCoroutines/coro-await.cpp
  clang/test/CodeGenCoroutines/coro-dest-slot.cpp
  clang/test/CodeGenCoroutines/coro-gro-nrvo.cpp
  clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp
  clang/test/CodeGenCoroutines/coro-params.cpp
  clang/test/CodeGenCoroutines/coro-promise-dtor.cpp
  clang/test/CodeGenCoroutines/coro-ret-void.cpp
  clang/test/CodeGenCoroutines/coro-return-voidtype-initlist.cpp
  clang/test/CodeGenCoroutines/coro-return.cpp
  clang/test/CodeGenCoroutines/coro-unhandled-exception.cpp
  clang/test/SemaCXX/Inputs/std-coroutine.h
  clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp
  clang/test/SemaCXX/coroutine-rvo.cpp
  clang/test/SemaCXX/coroutine-unhandled_exception-warning.cpp
  clang/test/SemaCXX/coroutine-uninitialized-warning-crash.cpp
  clang/test/SemaCXX/coroutines.cpp

Index: clang/test/SemaCXX/coroutines.cpp
===
--- clang/test/SemaCXX/coroutines.cpp
+++ clang/test/SemaCXX/coroutines.cpp
@@ -52,21 +52,24 @@
 };
 
 struct awaitable {
-  bool await_ready();
-  template  void await_suspend(F);
-  void await_resume();
+  bool await_ready() noexcept;
+  template 
+  void await_suspend(F) noexcept;
+  void await_resume() noexcept;
 } a;
 
 struct suspend_always {
-  bool await_ready() { return false; }
-  template  void await_suspend(F);
-  void await_resume() {}
+  bool await_ready() noexcept { return false; }
+  template 
+  void await_suspend(F) noexcept;
+  void await_resume() noexcept {}
 };
 
 struct suspend_never {
-  bool await_ready() { return true; }
-  template  void await_suspend(F);
-  void await_resume() {}
+  bool await_ready() noexcept { return true; }
+  template 
+  void await_suspend(F) noexcept;
+  void await_resume() noexcept {}
 };
 
 struct auto_await_suspend {
@@ -127,7 +130,7 @@
 struct promise {
   void get_return_object();
   suspend_always initial_suspend();
-  suspend_always final_suspend();
+  suspend_always final_suspend() noexcept;
   awaitable yield_value(int); // expected-note 2{{candidate}}
   awaitable yield_value(yielded_thing); // expected-note 2{{candidate}}
   not_awaitable yield_value(void()); // expected-note 2{{candidate}}
@@ -138,7 +141,7 @@
 struct promise_void {
   void get_return_object();
   suspend_always initial_suspend();
-  suspend_always final_suspend();
+  suspend_always final_suspend() noexcept;
   void return_void();
   void unhandled_exception();
 };
@@ -152,13 +155,13 @@
 namespace experimental {
 template 
 struct coroutine_handle {
-  static coroutine_handle from_address(void *);
+  static coroutine_handle from_address(void *) noexcept;
 };
 template <>
 struct coroutine_handle {
   template 
-  coroutine_handle(coroutine_handle);
-  static coroutine_handle from_address(void *);
+  coroutine_handle(coroutine_handle) noexcept;
+  static coroutine_handle from_address(void *) noexcept;
 };
 }} // namespace std::experimental
 
@@ -402,7 +405,7 @@
 
 namespace adl_ns {
 struct coawait_arg_type {};
-awaitable operator co_await(coawait_arg_type);
+awaitable operator co_await(coawait_arg_type) noexcept;
 }
 
 namespace dependent_operator_co_await_lookup {
@@ -434,7 +437,7 @@
 typedef transform_awaitable await_arg;
 coro get_return_object();
 transformed initial_suspend();
-