[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.


================
Comment at: clang/lib/Sema/SemaCoroutine.cpp:1585-1586
   InitializedEntity Entity = InitializedEntity::InitializeVariable(GroDecl);
-  ExprResult Res = S.PerformMoveOrCopyInitialization(Entity, nullptr, GroType,
-                                                     this->ReturnValue);
+  ExprResult Res =
+      S.PerformCopyInitialization(Entity, SourceLocation(), ReturnValue);
   if (Res.isInvalid())
----------------
mizvekov wrote:
> mizvekov wrote:
> > mizvekov wrote:
> > > aaronpuchert wrote:
> > > > Perhaps this should just be direct initialization? Can't really find anything in the standard though.
> > > So I just checked this again. Regarding our conversation on IRC, what I said initially was correct: `ReturnValue` is always a member function expression, built by `makeReturnObject` -> `buildPromiseCall` -> `buildMemberCall`. So implicit move would never trigger here, and as far as I see there is no reason for this code to have used PerformMoveOrCopyInitialization in the first place.
> > Err I meant: member function *call* expression
> Also, I don't think we could force direct initialization here, if the object returned by Gro is volatile for example.
> With that said, copy elision from the sema action on the return statement of get_return_object should take care of removing this copy here, I think, I don't see any reason it would not work here just as well as for expressions coming from the parser.
In `InitializationKind` there is a distinction between `IK_Copy` (basically `T x = init;`) and `IK_Direct` (basically `T x(init);`). I don't know how this would be related to `volatile`, but it is related to `explicit`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696