modocache added inline comments.
================
Comment at: lib/Sema/SemaCoroutine.cpp:197-200
- if (S.isUnevaluatedContext()) {
- S.Diag(Loc, diag::err_coroutine_unevaluated_context) << Keyword;
- return false;
- }
----------------
lewissbaker wrote:
> Does removing this check now mean that we're not checking that `co_return`
> statements don't appear in unevaluated contexts?
>
> Or is that already handled elsewhere by the fact that `co_return` statements
> are not expressions and are therefore detected earlier as a grammar violation
> when parsing `sizeof()` expression?
That's right! If one were to attempt to use a `co_return` within a
subexpression of `sizeof`, they'd see `error: expected expression` before
they'd ever have seen this error message. So I believe there's no need to
perform this check for `co_return`, and I believe that's why the revisions to
the standard included in the Coroutines TS don't make any special mention of
disallowing `co_return` in those contexts.
================
Comment at: lib/Sema/SemaCoroutine.cpp:675
+ // Second emphasis of [expr.await]p2: must be outside of an exception
handler.
+ if (S.getCurScope()->getFlags() & Scope::CatchScope) {
+ S.Diag(Loc, diag::err_coroutine_within_handler) << Keyword;
----------------
lewissbaker wrote:
> modocache wrote:
> > EricWF wrote:
> > > We can still build a valid AST after encountering this error, no?
> > >
> > >
> > I believe so. Just to be clear: you'd like me to continue building the AST
> > even after emitting this error diagnostic? My understanding is that most of
> > this file bails soon after any error is encountered (correct me if that's
> > wrong). I'm happy to change that, but I wonder if it'd be better to do that
> > in a separate diff...?
> Do the scope flags reset when a lambda occurs within a catch-block?
>
> eg. The following should still be valid.
> ```
> try { ... }
> catch (...) {
> []() -> task { co_await foo(); }();
> }
> ```
I believe they're reset, the example you posted compiles fine with this patch.
I'll add a test case specifically for this and confirm exactly where the scope
flags are reset or ignored in the lambda case.
================
Comment at: test/SemaCXX/coroutines.cpp:299-311
// FIXME: The spec says this is ill-formed.
void operator=(CtorDtor&) {
co_yield 0; // expected-error {{'co_yield' cannot be used in a copy
assignment operator}}
}
void operator=(CtorDtor const &) {
co_yield 0; // expected-error {{'co_yield' cannot be used in a copy
assignment operator}}
}
----------------
lewissbaker wrote:
> Not related to this diff, but...
>
> I don't think that these should be ill-formed.
>
> According to N4775 there are only exclusions added for [class.ctor] and
> [class.dtor].
> I can't see anything in the spec that says that assignment special member
> functions cannot be coroutines.
That's a great point. Could you create a Bugzilla for this work? And maybe we
can get @GorNishanov's opinion?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59076/new/
https://reviews.llvm.org/D59076
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
