[PATCH] D115219: [C++20] [Coroutines] Mark coroutine done if unhandled_exception throws

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D115219#3178370 <https://reviews.llvm.org/D115219#3178370>, @ChuanqiXu wrote:

> In D115219#3177213 <https://reviews.llvm.org/D115219#3177213>, @rjmccall wrote:
>
>> Okay.  Well, I'm glad it works.  I guess I find it a little strange that `coro.end` doesn't already mark the coroutine done.  I guess the normal C++ lowering always generates a final suspension before reaching a non-unwind `coro.end`?
>
> Yes, there is always a final suspend before coro.end in normal path. So that the coroutine would be marked done all the time except `unhandled_exception` throws.

Ah, I see.  This is the one place where an exception can be thrown before we reach that point.

You should update the coroutines documentation to mention this effect of `llvm.coro.end(true)`.



================
Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:292
+  case coro::ABI::Switch: {
+    // In C++'s specificatin, the corotine should be marked as done
+    // if the coroutine leaves in unwinding path.
----------------



================
Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:293
+    // In C++'s specificatin, the corotine should be marked as done
+    // if the coroutine leaves in unwinding path.
+    //
----------------



================
Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:304
+        Shape.FrameTy->getTypeAtIndex(coro::Shape::SwitchFieldIndex::Resume)));
+    Builder.CreateStore(NullPtr, GepIndex);
     if (!InResume)
----------------
We need to do this store elsewhere, right, like during final suspends?  Can we make a common function for it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115219