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())
----------------
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.
================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:854
if (Ex && !Ex->isTypeDependent()) {
+ NRVOResult NRVORes = IsThrownVarInScope ? getNRVOResult(Ex) : NRVOResult();
+
----------------
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.
================
Comment at: clang/lib/Sema/SemaStmt.cpp:3030-3032
+ Res.S = Res.S != Sema::NRVOResult::None && CanMove
+ ? Sema::NRVOResult::MoveEligible
+ : Sema::NRVOResult::None;
----------------
aaronpuchert wrote:
> The way you've designed the enumeration, couldn't you use `std::min` here?
Yes exactly!
The previous version of this patch I submitted used exactly that, but then for
some reason I thought people could find std::min a bit odd here, and so I
changed it.
But now I think I was right the first time around :)
================
Comment at: clang/lib/Sema/SemaStmt.cpp:3140
+/// \param ReturnType This is the return type of the function.
+void Sema::updNRVOResultWithRetType(NRVOResult &Res, QualType ReturnType) {
+ if (!Res.Candidate)
----------------
aaronpuchert wrote:
> `NRVOResult` seems to be small, why not make this a proper function and let
> it return the result?
It does use the result parameter as input and output. This function can only
"downgrade" a result it received previously.
I could make it receive a result and return the result, but then would probably
just use it like this:
```
NRVORes = updNRVOResultWithRetType(NRVORes, ...);
```
Do you think that looks clearer?
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
