13
votes

Using the return value of operator* from a "dead" unique_ptr is bad.

The following code compiles but results of course in Undefined Behavior:

auto& ref = *std::make_unique<int>(7);
std::cout << ref << std::endl;

Why didn't the standard make the return type of operator* for an rvalue of std::unique_ptr an rvalue of the internal value, instead of an lvalue, like this:

// could have been done inside unique_ptr
T& operator*() & { return *ptr; }
T&& operator*() && { return std::move(*ptr); }

In which case this would work fine:

std::cout << *std::make_unique<int>(7) << std::endl;

But the code at the beginning would not compile (cannot bind an rvalue to an lvalue).


Side note: of course someone could still write bad code like the below, but it is saying "I'm UB" more verbosely, IMHO, thus less relevant for this discussion:

auto&& ref = *std::make_unique<int>(7);
std::cout << ref << std::endl;

Is there any good reason for operator* on an rvalue of std::unique_ptr to return an lvalue ref?

3
Why: Because the standard lacks a r-value ref qualified overload of operator*. Why wasn't the ref qualified version added? Perhaps an oversight, maybe a good idea for a paper.AndyG
Of course, it would have to be T&& operator*() && { return ::std::move(*ptr); }, otherwise it wouldn't even compileAngew is no longer proud of SO
With your proposed change, would e.g. auto ptr = std::make_unique<int>(7); auto& ref = *ptr; still work?Some programmer dude
"it is saying "I'm UB" more verbosely, thus not so relevant:" I fail to see why. auto&& doesn't mean "I'm UB".Nicol Bolas
@AmirKirsh: My point is that it's not "saying 'I'm UB'" at all. That is, it isn't more explicitly or obviously a declaration of UB than the lvalue reference version.Nicol Bolas

3 Answers

10
votes

Your code, in terms of the value categories involved and the basic idea, is the equivalent of this:

auto &ref = *(new int(7));

new int(7) results in a pointer object which is a prvalue expression. Dereferencing that prvalue results in an lvalue expression.

Regardless of whether the pointer object is an rvalue or lvalue, applying * to a pointer will result in an lvalue. That shouldn't change just because the pointer is "smart".

4
votes

Good question!

Without digging into the relevant papers and design discussions, I think there are a few points that are maybe the reasons for this design decision:

  1. As @Nicol Bolas mentioned, this is how a built-in (raw) pointer would behave, so "do as int does" is applied here as "do as int* does".

    This is similar to the fact that unique_ptr (and other library types) don't propagate constness (which in turn is why we are adding propagate_const).

  2. What about the following code snippet? It doesn't compile with your suggested change, while it is a valid code that shouldn't be blocked.

class Base { virtual ~Base() = default; };
class Derived : public Base {};
void f(Base&) {}

int main()
{
    f(*std::make_unique<Derived>());
}

(godbolt - it compiles if our operator* overloadings are commented out)


For your side note: I'm not sure auto&& says "I'm UB" any louder. On the contrary, some would argue that auto&& should be our default for many cases (e.g. range-based for loop; it was even suggested to be inserted automatically for "terse-notation range-based for loop" (which wasn't accepted, but still...)). Let's remember that rvalue-ref has similar effect as const &, extension of the lifetime of a temporary (within the known restrictions), so it doesn't necessarily look like a UB in general.

2
votes

std::cout << *std::make_unique<int>(7) << std::endl; already works as the temporary dies at the end of the full expression.

T& operator*() & { return *ptr; }
T&& operator*() && { return std::move(*ptr); }

wouldn't avoid the dangling reference, (as for your example)

auto&& ref = *std::make_unique<int>(7); // or const auto&
std::cout << ref << std::endl;

but indeed, would avoid binding a temporary to a non-const lvalue reference.

Another safer alternative would be:

T& operator*() & { return *ptr; }
T operator*() && { return std::move(*ptr); }

to allow the lifetime extension, but that would do an extra move constructor not necessarily wanted in the general case.