GorNishanov added a comment. Thank you for doing this. It looks very elegant, but, it is a little bit wrong. It creates two different initial_suspend objects. Run this example:
https://wandbox.org/permlink/Q1Zd2NUlolmw9YmX You will observe that in trunk we are getting the output: 0x216808c: constructed(initial) 0x216808c: await_ready 0x216808c: await_suspend 0x216808c: await_resume 0x216808c: destroyed 0x21680b8: constructed(final) 0x21680b8: await_ready 0x21680b8: await_suspend consumed 10 values with sum 45 0x21680b8: destroyed promise destroyed With this change, the output becomes: 0x1965c4c: constructed(initial) 0x1965c4c: await_ready 0x1965c4c: await_suspend 0x1965c4c: destroyed 0x1965c60: constructed(initial) 0x1965c60: await_resume 0x1965c60: destroyed 0x1965c80: constructed(final) 0x1965c80: await_ready 0x1965c80: await_suspend consumed 10 values with sum 45 0x1965c80: destroyed 0x1965c60: destroyed promise destroyed I suggest, first modify the unit test to check that we do not create two initial_suspend objects. Then to fix it :-) ================ Comment at: lib/CodeGen/CGCoroutine.cpp:595 + auto InitSuspend = S.getInitSuspendStmt(); CurCoro.Data->CurrentAwaitKind = AwaitKind::Init; ---------------- auto *InitialSuspend = ... See: https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable ================ Comment at: lib/CodeGen/CGCoroutine.cpp:605 auto Loc = S.getLocStart(); + auto AwaitExpr = + cast<CoawaitExpr>(cast<ExprWithCleanups>(InitSuspend)->getSubExpr()); ---------------- auto *AwaitExpr = ================ Comment at: lib/CodeGen/CGCoroutine.cpp:608 + + SmallVector<Stmt *, 2> Stmts; + Stmts.push_back(AwaitExpr->getResumeExpr()); ---------------- Consider: ``` std::array<Stmt *,2> Stmts = {AwaitExpr->getResumeExpr(), S.getBody()}; ``` instead Repository: rC Clang https://reviews.llvm.org/D45860 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
