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
