[PATCH] D51741: [coro]Pass rvalue reference for named local variable to return_value

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.
Herald added a project: LLVM.

This change breaks the following code that worked before:

  task<MoveOnly&> f(MoveOnly &value) {
    co_return value;
  }

The error message is:

  clang/test/SemaCXX/coroutine-rvo.cpp:60:13: error: call to deleted constructor of 'MoveOnly'
    co_return value;
              ^~~~~
  clang/test/SemaCXX/coroutine-rvo.cpp:43:3: note: 'MoveOnly' has been explicitly marked deleted here
    MoveOnly(const MoveOnly&) = delete;
    ^

Is that maybe intentional, and is the code not intended to compile?



================
Comment at: cfe/trunk/lib/Sema/SemaCoroutine.cpp:846
+  if (E) {
+    auto NRVOCandidate = this->getCopyElisionCandidate(E->getType(), E, CES_AsIfByStdMove);
+    if (NRVOCandidate) {
----------------
Why not `CES_Strict` like in `Sema::BuildReturnStmt`? With `CES_Strict` the test still works, and we can also return references.


================
Comment at: cfe/trunk/lib/Sema/SemaCoroutine.cpp:849
+      InitializedEntity Entity =
+          InitializedEntity::InitializeResult(Loc, E->getType(), NRVOCandidate);
+      ExprResult MoveResult = this->PerformMoveOrCopyInitialization(
----------------
The last parameter has type `bool`, and because we're in `if (NRVOCandidate)`, that will always be true. Wouldn't it be more straightforward to just pass `true` into the function?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D51741