rusyaev-roman 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;
----------------
ChuanqiXu wrote:
> 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.
I've already added a comment at the beginning of `updateNRVOCandidate` function
where this point is mentioned:
```
// ... Therefore, we need to clear return slots for other
// variables defined before the current return statement in the current
// scope and in outer scopes.
```
If it's not enough, please let me know.
> Have you tested any larger projects?
Yes, I've built the `clang` itself and `compiler-rt` project. Then I've checked
them to run 'check-all' (on built clang and compiler-rt). Everything works.
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
