mizvekov added inline comments.
================
Comment at: clang/lib/Sema/SemaStmt.cpp:3066
- if (isCopyElisionCandidate(ReturnType, VD, CESK))
- return VD;
- return nullptr;
+static void downgradeNRVOResult(Sema::NRVOResult &Res, bool CanMove) {
+ Res.S = std::min(Res.S, CanMove ? Sema::NRVOResult::MoveEligible
----------------
Quuxplusone wrote:
> mizvekov wrote:
> > rsmith wrote:
> > > I find this name a little unclear (what do we mean by "downgrade"?). Can
> > > we call this something a bit more specific, such as `disallowCopyElision`
> > > or `disallowNRVO`?
> > How about `demoteNRVOStatus`?
> I'm with Richard here: if you mean "disallow copy elision," say so!
> IMO this should be member functions of `NamedReturnInfo`, and it should just
> be
>
> void disallowCopyElision() {
> S &= ~CopyElidable; // obviously this "should" work;
> } // ensure that your enum values actually make it work
>
> void disallowImplicitMove() {
> S = None;
> }
>
> The way you use it below is equivalent to e.g.
>
> if (VD->isExceptionVariable()) {
> Info.disallowCopyElision();
> if (!hasCXX20)
> Info.disallowImplicitMove();
> }
>
> which I think is a lot easier to puzzle out what's going on and how it
> corresponds to the standardese.
@Quuxplusone But I did not mean "disallow copy elision", the function will also
disallow implicit move based on a parameter, so would be a bit misleading.
That solution you proposed though is more repetitive and prone to error: it
does not enforce the invariant that you should always disallow copy elision
when disallowing implicit move.
Even if you amend that to having one manipulator function which enforces the
invariant, you still have two bools totaling four states, where only three
states make sense.
I would consider a better name though, not necessarily happy with this one
either.
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
