Quuxplusone added inline comments.
================
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:
> > 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.
> But for this function, I don't see how it could make sense to actually use it
> like that.
>
> You are either using it in a context where you don't have a ReturnType
> (throw, co_return), and you only care about the initial NRVOResult and don't
> call this function at all, or you are in a return statement, where you will
> call this function passing the initial NRVOResult, which will then have no
> further utility.
I haven't looked at the code, but FWIW, I'm always in favor of more
"function-style" functions. Which would you rather deal with — `void
square(int& x)` or `int square(int x)`? The same rationale applies uniformly
everywhere. Even if we are "consuming" the argument in this particular
instance, that might change after another few years of maintenance; and
regardless, it might be easier for the maintainer to understand
"function-style" code even if they aren't going to change it.
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
