mizvekov 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:
> > 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.
================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:854
if (Ex && !Ex->isTypeDependent()) {
+ NRVOResult NRVORes = IsThrownVarInScope ? getNRVOResult(Ex) : NRVOResult();
+
----------------
mizvekov wrote:
> mizvekov wrote:
> > aaronpuchert wrote:
> > > Any reason why you've moved that away from the comment that wants to
> > > explain it?
> > Yes, on C++2b this might modify Ex, and I moved it so this would happen
> > before we do the check with the type of the expression just below here.
> But yeah I forgot to move the comment, good catch.
Actually, had my wires crossed there with the P2266 DR, I only need to move
this line there.
================
Comment at: clang/lib/Sema/SemaStmt.cpp:3157-3159
+ // it would deduce to a reference.
+ if (AT->isDecltypeAuto() && Res.isParenthesized)
+ return Res = NRVOResult(), void();
----------------
aaronpuchert wrote:
> Can't we just do this when we know what it deduces to? It sounds weird to
> handle dependent contexts here because we shouldn't attempt any return value
> initialization then.
I think there are problems regarding the limitations of the current NRVO
algorithm, but I had not had much time to study it or think of improvements, so
I follow the current approach of trying to use as much available information as
possible in order to eliminate candidates early.
With that said, this is not well covered by the test suite and I could probably
get away with a lot less without breaking any existing tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99696/new/
https://reviews.llvm.org/D99696
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
