Fixed #679: Better support for coroutines with return_void.

andreasfertig · Jan 10, 2025 · 3c1b690 · 3c1b690
1 parent cc5b06c
commit 3c1b690
Show file tree

Hide file tree

Showing 28 changed files with 233 additions and 43 deletions.
diff --git a/CoroutinesCodeGenerator.cpp b/CoroutinesCodeGenerator.cpp
@@ -112,7 +112,6 @@ class CoroutineASTTransformer : public StmtVisitor<CoroutineASTTransformer>
     CoroutineASTData&                     mASTData;
     Stmt*                                 mStaged{};
     bool                                  mSkip{};
-    bool                                  mFinalSuspend{};
     size_t&                               mSuspendsCount;
     llvm::DenseMap<VarDecl*, MemberExpr*> mVarNamePrefix{};
 
@@ -320,9 +319,7 @@ class CoroutineASTTransformer : public StmtVisitor<CoroutineASTTransformer>
 
     void VisitCoawaitExpr(CoawaitExpr* stmt)
     {
-        if(not mFinalSuspend) {
-            ++mSuspendsCount;
-        }
+        ++mSuspendsCount;
 
         if(const bool returnsVoid{stmt->getResumeExpr()->getType()->isVoidType()}; returnsVoid) {
             Visit(stmt->getOperand());
@@ -391,13 +388,9 @@ class CoroutineASTTransformer : public StmtVisitor<CoroutineASTTransformer>
         Visit(stmt->getReturnValueInit());
         Visit(stmt->getExceptionHandler());
         Visit(stmt->getReturnStmtOnAllocFailure());
-
+        Visit(stmt->getFallthroughHandler());
         Visit(stmt->getInitSuspendStmt());
-
-        // final suspend point doesn't need a label
-        mFinalSuspend = true;
         Visit(stmt->getFinalSuspendStmt());
-        mFinalSuspend = false;
     }
 
     void VisitStmt(Stmt* stmt)
@@ -756,7 +749,10 @@ void CoroutinesCodeGenerator::InsertArg(const CoroutineBodyStmt* stmt)
         funcBodyStmts.Add(c);
     }
 
-    // InsertArg(stmt->getFallthroughHandler());
+    if(const auto* coReturnVoid = dyn_cast_or_null<CoreturnStmt>(stmt->getFallthroughHandler())) {
+        coReturnVoid->dump();
+        funcBodyStmts.Add(coReturnVoid);
+    }
 
     auto* gotoFinalSuspend = Goto(FINAL_SUSPEND_NAME);
     funcBodyStmts.Add(gotoFinalSuspend);
@@ -939,16 +935,14 @@ void CoroutinesCodeGenerator::InsertArg(const CoroutineSuspendExpr* stmt)
     Expr*          initializeInitialAwaitResume = nullptr;
 
     auto addInitialAwaitSuspendCalled = [&] {
-        if(eState::FinalSuspend != mState) {
-            bodyStmts.Add(bop);
-
-            if(eState::InitialSuspend == mState) {
-                mState = eState::Body;
-                // https://timsong-cpp.github.io/cppwp/n4861/dcl.fct.def.coroutine#5.3
-                initializeInitialAwaitResume =
-                    Assign(mASTData.mFrameAccessDeclRef, mASTData.mInitialAwaitResumeCalledField, Bool(true));
-                bodyStmts.Add(initializeInitialAwaitResume);
-            }
+        bodyStmts.Add(bop);
+
+        if(eState::InitialSuspend == mState) {
+            mState = eState::Body;
+            // https://timsong-cpp.github.io/cppwp/n4861/dcl.fct.def.coroutine#5.3
+            initializeInitialAwaitResume =
+                Assign(mASTData.mFrameAccessDeclRef, mASTData.mInitialAwaitResumeCalledField, Bool(true));
+            bodyStmts.Add(initializeInitialAwaitResume);
         }
     };
 
@@ -974,16 +968,16 @@ void CoroutinesCodeGenerator::InsertArg(const CoroutineSuspendExpr* stmt)
         mOutputFormatHelper.AppendNewLine();
     }
 
+    auto* suspendLabel = Label(BuildResumeLabelName(mSuspendsCount));
+    InsertArg(suspendLabel);
+
     if(eState::FinalSuspend == mState) {
         auto* memExpr     = AccessMember(mASTData.mFrameAccessDeclRef, mASTData.mDestroyFnField, true);
         auto* callCoroFSM = Call(memExpr, {mASTData.mFrameAccessDeclRef});
         InsertArg(callCoroFSM);
         return;
     }
 
-    auto* suspendLabel = Label(BuildResumeLabelName(mSuspendsCount));
-    InsertArg(suspendLabel);
-
     const auto* resumeExpr = stmt->getResumeExpr();
 
     if(not resumeExpr->getType()->isVoidType()) {

diff --git a/tests/EduCoroutineAllocFailureTest.expect b/tests/EduCoroutineAllocFailureTest.expect
@@ -204,6 +204,7 @@ void __fun_intResume(__fun_intFrame * __f)
     switch(__f->__suspend_index) {
       case 0: break;
       case 1: goto __resume_fun_int_1;
+      case 2: goto __resume_fun_int_2;
     }
 
     /* co_await EduCoroutineAllocFailureTest.cpp:40 */
@@ -234,9 +235,11 @@ void __fun_intResume(__fun_intFrame * __f)
   __f->__suspend_40_14_1 = __f->__promise.final_suspend();
   if(!__f->__suspend_40_14_1.await_ready()) {
     __f->__suspend_40_14_1.await_suspend(std::coroutine_handle<generator<int>::promise_type>::from_address(static_cast<void *>(__f)).operator std::coroutine_handle<void>());
+    __f->__suspend_index = 2;
     return;
   } 
 
+  __resume_fun_int_2:
   __f->destroy_fn(__f);
 }
 

diff --git a/tests/EduCoroutineBinaryExprTest.expect b/tests/EduCoroutineBinaryExprTest.expect
@@ -140,6 +140,7 @@ void __seqResume(__seqFrame * __f)
       case 3: goto __resume_seq_3;
       case 4: goto __resume_seq_4;
       case 5: goto __resume_seq_5;
+      case 6: goto __resume_seq_6;
     }
 
     /* co_await EduCoroutineBinaryExprTest.cpp:35 */
@@ -216,9 +217,11 @@ void __seqResume(__seqFrame * __f)
   __f->__suspend_35_11_1 = __f->__promise.final_suspend();
   if(!__f->__suspend_35_11_1.await_ready()) {
     __f->__suspend_35_11_1.await_suspend(std::coroutine_handle<generator::promise_type>::from_address(static_cast<void *>(__f)).operator std::coroutine_handle<void>());
+    __f->__suspend_index = 6;
     return;
   } 
 
+  __resume_seq_6:
   __f->destroy_fn(__f);
 }
 

diff --git a/tests/EduCoroutineCaptureConstTest.cerr b/tests/EduCoroutineCaptureConstTest.cerr
@@ -4,8 +4,8 @@
 .tmp.cpp:82:15: note: non-static data member 'start' declared const here
    82 |   const int & start;
       |   ~~~~~~~~~~~~^~~~~
-.tmp.cpp:147:12: error: no viable overloaded '='
-  147 |     __f->s = {0, '\0'};
+.tmp.cpp:148:12: error: no viable overloaded '='
+  148 |     __f->s = {0, '\0'};
       |     ~~~~~~ ^ ~~~~~~~~~
 .tmp.cpp:84:10: note: candidate function (the implicit copy assignment operator) not viable: 'this' argument has type 'const S', but method is not marked const
    84 |   struct S

diff --git a/tests/EduCoroutineCaptureConstTest.expect b/tests/EduCoroutineCaptureConstTest.expect
@@ -131,6 +131,7 @@ void __seqResume(__seqFrame * __f)
     switch(__f->__suspend_index) {
       case 0: break;
       case 1: goto __resume_seq_1;
+      case 2: goto __resume_seq_2;
     }
 
     /* co_await EduCoroutineCaptureConstTest.cpp:35 */
@@ -162,9 +163,11 @@ void __seqResume(__seqFrame * __f)
   __f->__suspend_35_11_1 = __f->__promise.final_suspend();
   if(!__f->__suspend_35_11_1.await_ready()) {
     __f->__suspend_35_11_1.await_suspend(std::coroutine_handle<generator::promise_type>::from_address(static_cast<void *>(__f)).operator std::coroutine_handle<void>());
+    __f->__suspend_index = 2;
     return;
   } 
 
+  __resume_seq_2:
   __f->destroy_fn(__f);
 }
 

diff --git a/tests/EduCoroutineCaptureThisTest.expect b/tests/EduCoroutineCaptureThisTest.expect
@@ -119,6 +119,7 @@ inline my_resumable coro(int x)
       switch(__f->__suspend_index) {
         case 0: break;
         case 1: goto __resume_coro_1;
+        case 2: goto __resume_coro_2;
       }
 
       /* co_await EduCoroutineCaptureThisTest.cpp:31 */
@@ -150,9 +151,11 @@ inline my_resumable coro(int x)
     __f->__suspend_31_16_1 = __f->__promise.final_suspend();
     if(!__f->__suspend_31_16_1.await_ready()) {
       __f->__suspend_31_16_1.await_suspend(std::coroutine_handle<my_resumable::promise_type>::from_address(static_cast<void *>(__f)).operator std::coroutine_handle<void>());
+      __f->__suspend_index = 2;
       return;
     } 
 
+    __resume_coro_2:
     __f->destroy_fn(__f);
   }
 

diff --git a/tests/EduCoroutineCoAwaitOperatorTest.cerr b/tests/EduCoroutineCoAwaitOperatorTest.cerr
@@ -1,7 +1,7 @@
 .tmp.cpp:167:3: error: unknown type name 'awaiter'
   167 |   awaiter __suspend_54_5;
       |   ^
-.tmp.cpp:249:10: warning: expression result unused [-Wunused-value]
-  249 |     __f->__suspend_56_14_res;
+.tmp.cpp:250:10: warning: expression result unused [-Wunused-value]
+  250 |     __f->__suspend_56_14_res;
       |     ~~~  ^~~~~~~~~~~~~~~~~~~
 1 warning and 1 error generated.
diff --git a/tests/EduCoroutineCoAwaitOperatorTest.expect b/tests/EduCoroutineCoAwaitOperatorTest.expect
@@ -209,6 +209,7 @@ void __gResume(__gFrame * __f)
       case 1: goto __resume_g_1;
       case 2: goto __resume_g_2;
       case 3: goto __resume_g_3;
+      case 4: goto __resume_g_4;
     }
 
     /* co_await EduCoroutineCoAwaitOperatorTest.cpp:51 */
@@ -247,6 +248,8 @@ void __gResume(__gFrame * __f)
     __resume_g_3:
     __f->__suspend_56_14_res = __f->__suspend_56_14.await_resume();
     __f->__suspend_56_14_res;
+    /* co_return EduCoroutineCoAwaitOperatorTest.cpp:51 */
+    __f->__promise.return_void();
     goto __final_suspend;
   } catch(...) {
     if(!__f->__initial_await_suspend_called) {
@@ -262,9 +265,11 @@ void __gResume(__gFrame * __f)
   __f->__suspend_51_16_1 = __f->__promise.final_suspend();
   if(!__f->__suspend_51_16_1.await_ready()) {
     __f->__suspend_51_16_1.await_suspend(std::coroutine_handle<my_future<int>::promise_type>::from_address(static_cast<void *>(__f)).operator std::coroutine_handle<void>());
+    __f->__suspend_index = 4;
     return;
   } 
 
+  __resume_g_4:
   __f->destroy_fn(__f);
 }
 

diff --git a/tests/EduCoroutineCoreturnWithCoawaitTest.cerr b/tests/EduCoroutineCoreturnWithCoawaitTest.cerr
@@ -1,41 +1,41 @@
-.tmp.cpp:274:26: error: object of type 'generator' cannot be assigned because its copy assignment operator is implicitly deleted
-  274 |     __f->__suspend_56_24 = simpleReturn(__f->v);
+.tmp.cpp:278:26: error: object of type 'generator' cannot be assigned because its copy assignment operator is implicitly deleted
+  278 |     __f->__suspend_56_24 = simpleReturn(__f->v);
       |                          ^
 .tmp.cpp:69:10: note: copy assignment operator is implicitly deleted because 'generator' has a user-declared move constructor
    69 |   inline generator(generator && rhs)
       |          ^
-.tmp.cpp:285:26: error: object of type 'generator' cannot be assigned because its copy assignment operator is implicitly deleted
-  285 |     __f->__suspend_56_51 = simpleReturn(__f->v + 1);
+.tmp.cpp:289:26: error: object of type 'generator' cannot be assigned because its copy assignment operator is implicitly deleted
+  289 |     __f->__suspend_56_51 = simpleReturn(__f->v + 1);
       |                          ^
 .tmp.cpp:69:10: note: copy assignment operator is implicitly deleted because 'generator' has a user-declared move constructor
    69 |   inline generator(generator && rhs)
       |          ^
-.tmp.cpp:407:26: error: object of type 'generator' cannot be assigned because its copy assignment operator is implicitly deleted
-  407 |     __f->__suspend_60_24 = simpleReturn(__f->v);
+.tmp.cpp:414:26: error: object of type 'generator' cannot be assigned because its copy assignment operator is implicitly deleted
+  414 |     __f->__suspend_60_24 = simpleReturn(__f->v);
       |                          ^
 .tmp.cpp:69:10: note: copy assignment operator is implicitly deleted because 'generator' has a user-declared move constructor
    69 |   inline generator(generator && rhs)
       |          ^
-.tmp.cpp:418:26: error: object of type 'generator' cannot be assigned because its copy assignment operator is implicitly deleted
-  418 |     __f->__suspend_60_51 = simpleReturn(__f->v + 1);
+.tmp.cpp:425:26: error: object of type 'generator' cannot be assigned because its copy assignment operator is implicitly deleted
+  425 |     __f->__suspend_60_51 = simpleReturn(__f->v + 1);
       |                          ^
 .tmp.cpp:69:10: note: copy assignment operator is implicitly deleted because 'generator' has a user-declared move constructor
    69 |   inline generator(generator && rhs)
       |          ^
-.tmp.cpp:429:26: error: object of type 'generator' cannot be assigned because its copy assignment operator is implicitly deleted
-  429 |     __f->__suspend_60_80 = simpleReturn(__f->v + 2);
+.tmp.cpp:436:26: error: object of type 'generator' cannot be assigned because its copy assignment operator is implicitly deleted
+  436 |     __f->__suspend_60_80 = simpleReturn(__f->v + 2);
       |                          ^
 .tmp.cpp:69:10: note: copy assignment operator is implicitly deleted because 'generator' has a user-declared move constructor
    69 |   inline generator(generator && rhs)
       |          ^
-.tmp.cpp:548:26: error: object of type 'generator' cannot be assigned because its copy assignment operator is implicitly deleted
-  548 |     __f->__suspend_67_24 = simpleReturn(__f->v);
+.tmp.cpp:558:26: error: object of type 'generator' cannot be assigned because its copy assignment operator is implicitly deleted
+  558 |     __f->__suspend_67_24 = simpleReturn(__f->v);
       |                          ^
 .tmp.cpp:69:10: note: copy assignment operator is implicitly deleted because 'generator' has a user-declared move constructor
    69 |   inline generator(generator && rhs)
       |          ^
-.tmp.cpp:559:26: error: object of type 'generator' cannot be assigned because its copy assignment operator is implicitly deleted
-  559 |     __f->__suspend_67_51 = simpleReturn(__f->v + 1);
+.tmp.cpp:569:26: error: object of type 'generator' cannot be assigned because its copy assignment operator is implicitly deleted
+  569 |     __f->__suspend_67_51 = simpleReturn(__f->v + 1);
       |                          ^
 .tmp.cpp:69:10: note: copy assignment operator is implicitly deleted because 'generator' has a user-declared move constructor
    69 |   inline generator(generator && rhs)

diff --git a/tests/EduCoroutineCoreturnWithCoawaitTest.expect b/tests/EduCoroutineCoreturnWithCoawaitTest.expect
@@ -148,6 +148,7 @@ void __simpleReturnResume(__simpleReturnFrame * __f)
     switch(__f->__suspend_index) {
       case 0: break;
       case 1: goto __resume_simpleReturn_1;
+      case 2: goto __resume_simpleReturn_2;
     }
 
     /* co_await EduCoroutineCoreturnWithCoawaitTest.cpp:50 */
@@ -178,9 +179,11 @@ void __simpleReturnResume(__simpleReturnFrame * __f)
   __f->__suspend_50_11_1 = __f->__promise.final_suspend();
   if(!__f->__suspend_50_11_1.await_ready()) {
     __f->__suspend_50_11_1.await_suspend(std::coroutine_handle<generator::promise_type>::from_address(static_cast<void *>(__f)).operator std::coroutine_handle<void>());
+    __f->__suspend_index = 2;
     return;
   } 
 
+  __resume_simpleReturn_2:
   __f->destroy_fn(__f);
 }
 
@@ -256,6 +259,7 @@ void __additionAwaitReturnResume(__additionAwaitReturnFrame * __f)
       case 1: goto __resume_additionAwaitReturn_1;
       case 2: goto __resume_additionAwaitReturn_2;
       case 3: goto __resume_additionAwaitReturn_3;
+      case 4: goto __resume_additionAwaitReturn_4;
     }
 
     /* co_await EduCoroutineCoreturnWithCoawaitTest.cpp:55 */
@@ -308,9 +312,11 @@ void __additionAwaitReturnResume(__additionAwaitReturnFrame * __f)
   __f->__suspend_55_11_1 = __f->__promise.final_suspend();
   if(!__f->__suspend_55_11_1.await_ready()) {
     __f->__suspend_55_11_1.await_suspend(std::coroutine_handle<generator::promise_type>::from_address(static_cast<void *>(__f)).operator std::coroutine_handle<void>());
+    __f->__suspend_index = 4;
     return;
   } 
 
+  __resume_additionAwaitReturn_4:
   __f->destroy_fn(__f);
 }
 
@@ -389,6 +395,7 @@ void __additionAwaitReturn2Resume(__additionAwaitReturn2Frame * __f)
       case 2: goto __resume_additionAwaitReturn2_2;
       case 3: goto __resume_additionAwaitReturn2_3;
       case 4: goto __resume_additionAwaitReturn2_4;
+      case 5: goto __resume_additionAwaitReturn2_5;
     }
 
     /* co_await EduCoroutineCoreturnWithCoawaitTest.cpp:59 */
@@ -452,9 +459,11 @@ void __additionAwaitReturn2Resume(__additionAwaitReturn2Frame * __f)
   __f->__suspend_59_11_1 = __f->__promise.final_suspend();
   if(!__f->__suspend_59_11_1.await_ready()) {
     __f->__suspend_59_11_1.await_suspend(std::coroutine_handle<generator::promise_type>::from_address(static_cast<void *>(__f)).operator std::coroutine_handle<void>());
+    __f->__suspend_index = 5;
     return;
   } 
 
+  __resume_additionAwaitReturn2_5:
   __f->destroy_fn(__f);
 }
 
@@ -530,6 +539,7 @@ void __additionAwaitReturnWithIntResume(__additionAwaitReturnWithIntFrame * __f)
       case 1: goto __resume_additionAwaitReturnWithInt_1;
       case 2: goto __resume_additionAwaitReturnWithInt_2;
       case 3: goto __resume_additionAwaitReturnWithInt_3;
+      case 4: goto __resume_additionAwaitReturnWithInt_4;
     }
 
     /* co_await EduCoroutineCoreturnWithCoawaitTest.cpp:63 */
@@ -582,9 +592,11 @@ void __additionAwaitReturnWithIntResume(__additionAwaitReturnWithIntFrame * __f)
   __f->__suspend_63_11_1 = __f->__promise.final_suspend();
   if(!__f->__suspend_63_11_1.await_ready()) {
     __f->__suspend_63_11_1.await_suspend(std::coroutine_handle<generator::promise_type>::from_address(static_cast<void *>(__f)).operator std::coroutine_handle<void>());
+    __f->__suspend_index = 4;
     return;
   } 
 
+  __resume_additionAwaitReturnWithInt_4:
   __f->destroy_fn(__f);
 }
 

diff --git a/tests/EduCoroutineCustomYieldTypeTest.expect b/tests/EduCoroutineCustomYieldTypeTest.expect
@@ -208,6 +208,7 @@ void __seqResume(__seqFrame * __f)
       case 0: break;
       case 1: goto __resume_seq_1;
       case 2: goto __resume_seq_2;
+      case 3: goto __resume_seq_3;
     }
 
     /* co_await EduCoroutineCustomYieldTypeTest.cpp:86 */
@@ -237,6 +238,8 @@ void __seqResume(__seqFrame * __f)
       __f->__suspend_88_14.await_resume();
     }
 
+    /* co_return EduCoroutineCustomYieldTypeTest.cpp:86 */
+    __f->__promise.return_void();
     goto __final_suspend;
   } catch(...) {
     if(!__f->__initial_await_suspend_called) {
@@ -252,9 +255,11 @@ void __seqResume(__seqFrame * __f)
   __f->__suspend_86_11_1 = __f->__promise.final_suspend();
   if(!__f->__suspend_86_11_1.await_ready()) {
     __f->__suspend_86_11_1.await_suspend(std::coroutine_handle<generator::promise_type>::from_address(static_cast<void *>(__f)).operator std::coroutine_handle<void>());
+    __f->__suspend_index = 3;
     return;
   } 
 
+  __resume_seq_3:
   __f->destroy_fn(__f);
 }