[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Lewis Baker via Phabricator via cfe-commits]

lewissbaker added inline comments.


================
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;
----------------
modocache wrote:
> modocache wrote:
> > 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.
> When the parser encounters a lambda, it takes the path `Parser::ParseLambdaExpression -> Parser::ParseLambdaExpressionAfterIntroducer -> Parser::ParseCompoundStatementBody`, which creates an inner scope for the body of the lambda. This inner scope does not have the `Scope::CatchScope` flag, so it doesn't result in the error.
> 
> Although, your question did make me realize that this compiles just fine, even with this patch:
> 
> ```
> void example() {
>   try {
>     throw;
>   } catch (...) {
>     try {
>       co_await a;
>     }
>   }
> }
> ```
> 
> But I believe this above case should also be treated as an error, right?
Yes, I believe that co_await within a try-block within a catch-block should not be allowed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59076