Weird behavior when using std::move shared_ptr with conditional operator

Question

I was working on some C++ code using std::move on shared_ptr and got really weird output. I've simplified my code as below

int func(std::shared_ptr<int>&& a) {
    return 0;
}

int main() {
    std::shared_ptr<int> ptr = std::make_shared<int>(1);

    for (int i = 0; i != 10; ++i) {
        func(i == 9 ? std::move(ptr) : std::shared_ptr<int>(ptr));
    }

    if (ptr) {
        std::cout << "ptr is not null: " << *ptr << "\n";
    } else {
        std::cout << "ptr is null\n";
    }

    return 0;
}

And I got output

ptr is null

As I expected, my ptr will be moved (cast to std::shared_ptr<int>&&) at last loop, and since func never steals memory in a, my ptr outside will be non-null (which turns out to be null actually). And if I replace

func(i == 9 ? std::move(ptr) : std::shared_ptr<int>(ptr));

with if-else statement

if (i == 9) func(std::move(ptr));
else func(std::shared_ptr<int>(ptr));

the output will be

ptr is not null: 1

I am so confused about this behavior of compiler.

I've tried GCC and clang with different std version and optimization level, and got same output. Could someone explain for me why and where the data under ptr was stolen ?

Multiped

Isn't UB to use an object after it has been moved? – Amieva

I think the common type of the two branches of your conditional operator is std::shared_ptr<int> so in the first case a move constructed temporary instance is constructed and passed to func which steals ownership of ptr before the function is called. – Surprising

it makes sense. You are moving into func and func just destroys the temporary parameter. The last for iteration will move the shared_ptr into the function. and at the end of func the 'moved--in' shared_ptr gets destroyed – Arliearliene

@Arliearliene The argument for func is a reference. Only the reference's lifetime enda at the end of func, not the referred object (unless the reference happens to be extending the lifetime of a materialized temporary object). – Surprising

The loop seems to add unnecessary complexity here to demonstrate the problem. You could just remove that part entirely godbolt.org/z/qsTncvGvT – Simmie

the type of the conditional expression above is std::shared_ptr<int>, so a temporary object is constructed in the first case when condition is hit, check godbolt.org/z/44EM6YPad – Hyades

@FrançoisAndrieux Don't just think. I am certain that's the case. Conditional operator always returns the common types. This snippet just shows the preference of value over reference. – Coonan

Answer 1

Because the second and third operand to the conditional operator don't have the same value category (i.e., std::move(ptr) is an xvalue, while std::shared_ptr<int>(ptr) is a prvalue), this conditional expression falls under [expr.cond]/7:

Lvalue-to-rvalue, array-to-pointer, and function-to-pointer standard conversions are performed on the second and third operands. After those conversions, one of the following shall hold:

• The second and third operands have the same type; the result is of that type and the result object is initialized using the selected operand.
• [...]

std::shared_ptr<int>(ptr) is already a prvalue, so the lvalue-to-rvalue conversion (which is really the glvalue-to-prvalue conversion) does nothing to it.

std::move(ptr) is converted to a prvalue and that prvalue is used to initialize the result object. The initialization of the result object uses the move constructor (because that's the constructor that initializes a std::shared_ptr<int> from an xvalue of that type, which is what std::move(ptr) is). The move constructor "steals" the value from ptr. The result object is a temporary object that is bound to the parameter a and then later destroyed. Note that all of this only happens in the case where std::move(ptr) is actually evaluated (which requires i == 9 to be true).

Disconsolate

But, I guess this is a potential DR. common type of value and its refernce should be a reference IMHO. – Coonan

@Coonan That would change the meaning of existing code, and it would force implementations to introduce a hidden runtime flag that determines whether or not there is a temporary object that needs to be destroyed, since the temporary object would only get created along one of the branches of the conditional expression. – Disconsolate

If the result is used, it would bind to a reference. No need for heuristics, just life extend the temporary - reguardless of whether it was used. – Coonan

@Coonan I can't figure out what you mean. If your proposal is that the result of the whole conditional expression should be an xvalue, then it means when the second operand gets selected, it becomes the result (no temporary) and when the third operand gets selected, a temporary is materialized. So you would get conditional creation of a temporary. In the first case, there is no temporary to lifetime-extend. – Disconsolate

The 2nd operand is rvalue reference, the 3rd is rvalue. Result should be an rvalue reference. 3rd operand gets life-extednded to the duration of whatever captures the result-regardless of the condition. If the condition is deduced at compile-time to be true, then the operation can be short circuited and the 3rd operand be destructed immediately. – Coonan

@Coonan You can't lifetime-extend the third operand if the third operand isn't evaluated in the first place. The conditional operator only evaluates the second or third operand, not both. – Disconsolate

Yes, that's the dead end. – Coonan

@Coonan So you agree that the runtime flag is needed, so that the program knows whether or not it needs to call a destructor for a lifetime-extended temporary once the lifetime of the reference ends? – Disconsolate

If both parameters are not evaluated, then a flag would be necessary; not the optimal solution. A runtime branch is not ideal. A destructor function pointer could also be created, but again too much complication is not good. – Coonan

Answer 2

Called C++ functions are not required to move out a value, even if they get called with an rvalue reference. In particular, your func() doesn't even touch its argument a, so ptr remains unchanged in this call: func(std::move(ptr)).

On the contrary, your expression

func(i == 9 ? std::move(ptr) : std::shared_ptr<int>(ptr));

always creates a temporary of type std::shared_ptr<int> before calling func() with an rvalue reference to that temporary. What happens under the hood is:

{
    std::shared_ptr<int> temp = i == 9 ? std::move(ptr) : std::shared_ptr<int>(ptr));
    func(std::move(temp));
}

When i is 9, the assignment temp = std::move(ptr) moves ptr to temp and leaves ptr empty. Then, when the end of the block is reached and the temporary gets out of scope, it is destructed.

You can avoid that behaviour by changing your line to either of the following two variants:

func(i == 9 ? std::move(ptr) : std::move(std::shared_ptr<int>(ptr)));

or:

func(std::move(i == 9 ? ptr : std::shared_ptr<int>(ptr)));

Both variants ensure that the temporary created will be of a reference type (namely std::shared_ptr<int>&& in the first case and std::shared_ptr<int>& in the second case), so that nothing needs to be copied.

You might ask, why the original code creates a temporary of type std::shared_ptr<int> (and not std::shared_ptr<int>&&). Well, the reason is the well known return value optimization (RVO), which was introduced into C++ decades ago: functions, that return an object, are actually called in a way that the calling function already allocates space for that object and passes a pointer to that object to the called function. The called function then writes the return value directly into that space. Because of this, many direct assignments like:

auto x = function(a, b, c);

save the hassle of first creating and then immediately destructing again a temporary value. But in your case of the ternary operator, that otherwise hidden temporary suddenly comes to life.

Neptunium

I thought (and tested) in the second variant, the common type of ternary is std::shared_ptr<int> rather than std::shared_ptr<int>&, which causes copy-construct at last loop. And the first variant is OK. Thank you for improvment advice! – Multiped