aaronpuchert added inline comments.
================
Comment at: clang/lib/Sema/SemaStmt.cpp:3030-3032
+ Res.S = Res.S != Sema::NRVOResult::None && CanMove
+ ? Sema::NRVOResult::MoveEligible
+ : Sema::NRVOResult::None;
----------------
mizvekov wrote:
> 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 :)
Perhaps you can just add a comment to the enumeration saying that the values
are totally ordered with regards to implication so that ⇒ is >=.
================
Comment at: clang/lib/Sema/SemaStmt.cpp:3058
-bool Sema::isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD,
- CopyElisionSemanticsKind CESK) {
- QualType VDType = VD->getType();
- // - in a return statement in a function with ...
- // ... a class return type ...
- if (!ReturnType.isNull() && !ReturnType->isDependentType()) {
- if (!ReturnType->isRecordType())
- return false;
- // ... the same cv-unqualified type as the function return type ...
- // When considering moving this expression out, allow dissimilar types.
- if (!(CESK & CES_AllowDifferentTypes) && !VDType->isDependentType() &&
- !Context.hasSameUnqualifiedType(ReturnType, VDType))
- return false;
+ // (other than a function ... parameter)
+ if (VD->getKind() == Decl::ParmVar) {
----------------
mizvekov wrote:
> aaronpuchert wrote:
> > These comments seem to be separated from their context now...
> This one comment here is talking about the line just below it.
> Any other examples where it seems separated?
I meant the context of the comment itself. Originally this was quoting a clause
```
// - in a return statement in a function [where] ...
// ... the expression is the name of a non-volatile automatic object ...
```
and then additional parts later. Now you're having that initial part in a
different function and it's not immediately clear what you're quoting here.
To be clear, this was about "(other than a function ... parameter)" below.
================
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)
----------------
mizvekov wrote:
> 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?
Yes, that would seem more natural to me. Technically a caller could decide to
not use the same variable in both places, for example it could pass a temporary
or pass the result on directly.
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
