[PATCH] D145641: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain conditions

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno marked 3 inline comments as done.
bruno added inline comments.


================
Comment at: clang/lib/CodeGen/CGCoroutine.cpp:483-499
+    // The call to get_­return_­object is sequenced before the call to
+    // initial_­suspend and is invoked at most once, but there are caveats
+    // regarding on whether the prvalue result object may be initialized
+    // directly/eager or delayed, depending on the types involved.
+    //
+    // The general cases:
+    // 1. Same type of get_return_object and coroutine return type (direct
----------------
ChuanqiXu wrote:
> I'll feel better if we can attach the conclusion and links from the wg21.
I can add some of that but unfortunately we don't have the working yet, it still has a CWG round to go: https://github.com/cplusplus/papers/issues/1414

The best explanation for the problem is under Gor's writeup: https://github.com/GorNishanov/await/blob/master/2023_Issaquah/cwg2563-response.md, I instead added a comment based on it since we cannot guarantee this link will be there forever.


================
Comment at: clang/lib/CodeGen/CGCoroutine.cpp:500-508
+    DirectEmit = [&]() {
+      auto *RVI = S.getReturnValueInit();
+      if (!RVI || CGF.FnRetTy->isVoidType())
+        return true;
+      auto GroType = RVI->getType();
+      if (GroType->isVoidType())
+        return true;
----------------
ChuanqiXu wrote:
> What's the case about returning void and the return_object type is different from the returning type?
In that case we do `DirectEmit` since there's no reason to delay initialization, am I missing something?


================
Comment at: clang/test/SemaCXX/coroutine-no-move-ctor.cpp:24-26
 invoker f() {
   co_return;
 }
----------------
ChuanqiXu wrote:
> How about adding a failing case that the types are not matched?
I'm having trouble crafting one. I extracted similar examples from other testcases and even though I can confirm they correctly apply delayed initialization, they don't require copies (usually directly call conversion operators or similar) and we don't trigger the reverse usecase, do you have one testcase in mind I could use?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145641