[PATCH] D145639: [Coroutines] Fix premature conversion of return object

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D145639#4198794 <https://reviews.llvm.org/D145639#4198794>, @ChuanqiXu wrote:

> Reverting is common in Clang/LLVM. But let's wait for the response from @bruno temporarily. I think it makes sense to revert this one if @bruno don't response tomorrow. And we can commit this one and D145641 <https://reviews.llvm.org/D145641> quickly the next time. Recommit is common too.

@bgraur, FYI, hope waiting for a day works for this case.

In D145639#4198815 <https://reviews.llvm.org/D145639#4198815>, @ChuanqiXu wrote:

>> The RVO seems to be mandated by the standard.
>
> Yeah, I thought so... But WG21 decided to not make it in a recent meeting... So we can only make it as a non-mandated compiler optimization.

That's unexpected. Are there any wording change proposals published?
Our understanding comes from the reading of dcl.fct.def.coroutine/7 <https://timsong-cpp.github.io/cppwp/n4868/dcl.fct.def.coroutine#7>

> The expression promise.get_return_object() is used to initialize the glvalue result or prvalue result object of a call to a coroutine.

And we **think** RVO is mandated when initializing the **result objects** mentioned in general.
Also, from https://github.com/llvm/llvm-project/issues/56532:

> Q: if the initialization does occur later, by what mechanism the prvalue result of get_return_object is forwarded to that initialization.
> A: (see below)
> Case 1/2. Same type of get_return_object and coroutine return type.
> **Constructed in the return slot.**
> Case 3. Different types:
> (1) Constructed temporary object prior to initial suspend initialized with a call to get_return_object()
> (2) when coroutine needs to to return to the caller and needs to construct return value for the coroutine it is initialized with expiring value of the temporary obtained in step 1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145639