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

Brian Gesiak via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 9 11:54:58 PDT 2019


modocache added a subscriber: lewissbaker.
modocache added a comment.

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

It looks like it should work to me, but maybe @lewissbaker or @GorNishanov can answer definitively.



================
Comment at: cfe/trunk/lib/Sema/SemaCoroutine.cpp:846
+  if (E) {
+    auto NRVOCandidate = this->getCopyElisionCandidate(E->getType(), E, CES_AsIfByStdMove);
+    if (NRVOCandidate) {
----------------
aaronpuchert wrote:
> Why not `CES_Strict` like in `Sema::BuildReturnStmt`? With `CES_Strict` the test still works, and we can also return references.
So this fixes your test case? If so it sounds good to me. I'll make this change or you can feel free to if you get around to it first.


================
Comment at: cfe/trunk/lib/Sema/SemaCoroutine.cpp:849
+      InitializedEntity Entity =
+          InitializedEntity::InitializeResult(Loc, E->getType(), NRVOCandidate);
+      ExprResult MoveResult = this->PerformMoveOrCopyInitialization(
----------------
aaronpuchert wrote:
> 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?
Makes sense! I can send a patch to do this, or feel free to commit one yourself if you get to it first.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D51741





More information about the cfe-commits mailing list