wchilders marked an inline comment as done.
wchilders added inline comments.
================
Comment at: clang/lib/Sema/SemaInit.cpp:6466
if (Entity.allowsNRVO())
CurInit = S.BuildCXXConstructExpr(Loc, Step.Type,
Step.Function.FoundDecl,
----------------
rsmith wrote:
> It looks like the other callers to `BuildCXXConstructExpr` are also missing
> this handling. Can we put the call to `CheckForImmediateInvocation` in
> `BuildCXXConstructExpr` to handle all those cases at once, or do we need to
> defer it until after the other stuff below?
Those changes shouldn't have impact here.
I choose to apply this here as when looking at other uses of
`CheckForImmediateInvocation`, `MaybeBindToTemporary` was performed on the
expr, prior to the expr being passed to `CheckForImmediateInvocation`.
Upon changing this over, all tests (upstream and ours) still pass, so if you
don't see any issues, I'm fine with moving this into `BuildCXXConstructExpr`.
Additionally, upon closer inspection, there is low coupling between
`MaybeBindToTemporary` and `CXXConstructExpr`, where as there is significant
coupling for ObjC and `CallExpr`s. Unless something I've said here gives you
some doubts, I'm fine with moving this to `BuildCXXConstructExpr`, there's
nothing that immediately jumps out to me as a problem.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76396/new/
https://reviews.llvm.org/D76396
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
