Quuxplusone added inline comments.
================ Comment at: clang/lib/Sema/SemaStmt.cpp:3084-3085 + bool ForceCXX20) { + bool hasCXX11 = getLangOpts().CPlusPlus11 || ForceCXX20, + hasCXX20 = getLangOpts().CPlusPlus20 || ForceCXX20; + NamedReturnInfo Info{VD, NamedReturnInfo::MoveEligibleAndCopyElidable}; ---------------- Nit: please declare variables one-per-line. Change that `,` to a `;` and write `bool` again on line 3085 instead of four space characters. ================ 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 ---------------- 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. 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