Is it safe to modify RVO values within an RAII construct?

Question

Consider the following program:

#include <functional>
#include <iostream>

class RvoObj {
  public:
    RvoObj(int x) : x_{x} {}
    RvoObj(const RvoObj& obj) : x_{obj.x_} { std::cout << "copied\n"; }
    RvoObj(RvoObj&& obj) : x_{obj.x_} { std::cout << "moved\n"; }

    int x() const { return x_; }
    void set_x(int x) { x_ = x; }

  private:
    int x_;
};

class Finally {
  public:
    Finally(std::function<void()> f) : f_{f} {}
    ~Finally() { f_(); }

  private:
    std::function<void()> f_;
};

RvoObj BuildRvoObj() {
    RvoObj obj{3};
    Finally run{[&obj]() { obj.set_x(5); }};
    return obj;
}

int main() {
    auto obj = BuildRvoObj();
    std::cout << obj.x() << '\n';
    return 0;
}

Both clang and gcc (demo) output 5 without invoking the copy or move constructors.

Is this behavior well-defined and guaranteed by the C++17 standard?

"*now that copy-elision is required*" That's [not how it works](https://stackoverflow.com/a/38043447/734069). There is no guaranteed elision in your code. — Nicol Bolas, Jan 29 '20 at 21:40
What terminology should I use to describe the behavior observed in the demo? — Brian Rodriguez, Jan 29 '20 at 21:42

Nicol Bolas · Answer 1 · 2020-01-29T21:33:27.567

Copy elision only permits an implementation to remove the presence of the object being generated by a function. That is, it can remove the copy from obj to the return value object of foo and the destructor of obj. However, the implementation can't change anything else.

The copy to the return value would happen before destructors for local objects in the function are called. And the destructor of obj would happen after the destructor of run, because destructors for automatic variables are executed in reverse-order of their construction.

This means that it is safe for run to access obj in its destructor. Whether the object denoted by obj is destroyed after run completes or not does not change this fact.

However, there is one problem. See, return <variable_name>; for a local variable is required to invoke a move operation. In your case, moving from RvoObj is the same as copying from it. So for your specific code, it'll be fine.

But if RvoObj were, for example, unique_ptr<T>, you'd be in trouble. Why? Because the move operation to the return value happens before destructors for local variables are called. So in this case obj will be in the moved-from state, which for unique_ptr means that it's empty.

That's bad.

If the move is elided, then there's no problem. But since elision is not required, there is potentially a problem, since your code will behave differently based on whether elision happens or not. Which is implementation-defined.

So generally speaking, it's best not to have destructors rely on the existence of local variables that you're returning.

The above purely relates to your question about undefined behavior. It isn't UB to do something that changes behavior based on whether elision happens or not. The standard defines that one or the other will happen.

However, you cannot and should not rely upon it.

Awesome, thanks! "There is potentially a problem since your code will behave differently based on whether elision happens or not" — so then I can assume the code is always safe in C++17? — Brian Rodriguez, Jan 29 '20 at 21:29
I don't think the real question is just "will this invoke UB?" but also "is this guaranteed to print `5`?" I think @OP assumed that if it's not UB the value won't change on the compiler's whim, but that is not the case. Sticking to the standard, NVRO is not guaranteed, so I believe this code is allowed to print *either* `3` or `5`. It's guaranteed not to be UB but also not totally determined. — HTNW, Jan 29 '20 at 21:29
@BrianRodriguez: Your post is tagged c++17, so I answered it relative to its tag. — Nicol Bolas, Jan 29 '20 at 21:29
@BrianRodriguez If you mean "will this code always print `5` in C++17?" that's a no. The kind of elision guaranteed in C++17 is not the kind of elision you need. — HTNW, Jan 29 '20 at 21:30
@HTNW could you elaborate more on that (maybe in an answer)? — Brian Rodriguez, Jan 29 '20 at 21:30
I've clarified my question to be more explicit about what I was aiming to learn. — Brian Rodriguez, Jan 29 '20 at 21:35

score 3 · Accepted Answer · edited Jan 29 '20 at 21:36

Short answer: due to NRVO, the output of the program may be either 3 or 5. Both are valid.

For background, see first:

Guideline:

Avoid destructors that modify return values.

For example, when we see the following pattern:

T f() {
    T ret;
    A a(ret);   // or similar
    return ret;
}

We need to ask ourselves: does A::~A() modify our return value somehow? If yes, then our program most likely has a bug.

For example:

A type that prints the return value on destruction is fine.
A type that computes the return value on destruction is not fine.

_{[From https://stackoverflow.com/a/54566080/9305398 ]}

Is it safe to modify RVO values within an RAII construct?

2 Answers2