ChuanqiXu added inline comments.
================
Comment at: clang/lib/Sema/Scope.cpp:152-154
+ // Consider the variable as NRVO candidate if the return slot is available
+ // for it in the current scope, or if it can be available in outer scopes.
+ NRVO = CanBePutInReturnSlot ? VD : nullptr;
----------------
rusyaev-roman wrote:
> ChuanqiXu wrote:
> > What if NRVO contains a value already? It is possible due to the value of
> > NRVO could be set by its children.
> Actually this is intention. If the parent has already NRVO candidate, then it
> should be invalidated (or not). Let's consider the following examples:
>
>
> ```
> X foo(bool b) {
> X x;
> X y;
> if (b)
> return x;
> else
> return y; // when we process this return statement, the parent has
> already NRVO and it will be invalidated (this is correct behavior)
> }
> ```
>
> ```
> X foo(bool b) {
> X x;
> if (b)
> return x;
>
> X y;
> // when we process this return statement, the parent has already NRVO and
> it WON't be invalidated
> // (this is correct behavior), because a return slot will be available
> for it
> return y;
> }
> ```
>
> ```
> X foo(bool b) {
> X x;
> if (b)
> return x;
>
> // when we process this return statement, the parent has already NRVO and
> it WON't be invalidated (this is correct behavior)
> return x;
> }
> ```
>
> ```
> X foo(bool b, X x) {
> X y;
>
> if (b)
> return x;
>
> // when we process this return statement, the parent contains nullptr
> (invalid candidate) and it will be invalidated (this is correct behavior)
> return y;
> }
> ```
>
> ```
> X foo(bool b, X x) {
> if (b)
> return x;
>
> X y;
> // when we process this return statement, the parent contains nullptr
> (invalid candidate) and it WON't be invalidated (this is correct behavior)
> return y;
> }
> ```
Oh, I see. Tricky. I don't find invalid cases now. But I recommend to comment
that the children would maintain the `ReturnSlots` of their parents. (This is
anti-intuition)
Have you tested any larger projects? Like libc++, libstdc++ or something like
folly. I feel we need to do such tests to avoid we get anything wrong.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119792/new/
https://reviews.llvm.org/D119792
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
