Pitfalls of "making" unique_ptr implicitly convertible to T*?

Question

I just read

Add implicit conversion from unique_ptr<T> to T*

which focuses on the how-to-do-it part, not on should-I-do-it. I'm also not asking whether or not you think it's a good idea, but I am asking for concrete pitfalls I might stuble onto, or into, if I subclass unique_ptr to add the functionality of implicit conversion to a raw pointer (so I could just pass it directly to functions taking pointers, without using get()).

There's not much difference to using `get()` All the pitfalls applying to the latter will also apply to the implicit conversion. — πάντα ῥεῖ, Nov 07 '16 at 21:54
@πάνταῥεῖ: But what about the other direction? Perhaps something like unexpected template instantiation choices with STL data structures? I don't know, something diabolical. — einpoklum, Nov 07 '16 at 23:17

Dale Wilson · Answer 1 · 2016-11-07T21:51:37.413

3

consider this slightly contrived case:

X* px;
{
    unique_ptr<X> ux = new X;
    px = ux; // implicit conversion
}
px->method();

When presented like this it is obvious that the call to px->method will fail (or not, which would be worse), but if this code were more complex (say a call to a method taking X* as a parameter, obscure bugs could go uncaught.

For example replace px = ux with a call to functionWithMemory(ux);

void functionWithMemory(X * px)
{
   static X* oldX = nullptr;
   if(oldX)
   {
      oldX->goodbye();
   }
   px->hello();
   oldX = px;
}

still contrived, but if you keep extending this it starts to become plausible.

edited Nov 07 '16 at 21:51

answered Nov 07 '16 at 21:44

Dale Wilson

9,166
3
34
52

Well, I don't know... assigning a `unique_ptr` to anything else is itself a red flag for potential leaks and segmentation violations. In fact, one could argue that this stands out _more_ than having `px = ux.get()`. – einpoklum Nov 07 '16 at 21:47
1

@einpoklum: One could argue that, sure. That doesn't make you *right*. – Nicol Bolas Nov 07 '16 at 21:51
@einpoklum Yep. that's why I said "it's obvious". (although considered in isolation the statement `px = ux` doesn't raise flags. You have to go back and check the type of `ux` before you start to worry.) – Dale Wilson Nov 07 '16 at 21:54
Considering your second example - why wouldn't such a function be a problem with a plain pointer as well? Also - if I write code with a unique_ptr on the RHS of an assignment I usually go "hmm... should I really be doing that?" I typically expect to just pass `.get()`s of my unique_ptr and have it die at end-of-scope/ – einpoklum Nov 07 '16 at 21:59

T.C. · Answer 2 · 2016-11-07T21:59:25.943

3

An implicit conversion would allow all kinds of nonsense:

{
    unique_ptr<int> p(new int);

    p + 10; // OK with implicit conversion
    p[3];   // ditto
    bool x = p; // ditto

    unique_ptr<Derived> d(new Derived);
    unique_ptr<Base> b(d); // OK with implicit conversion...oops
                           // These two unique_ptr's own the same object!

}    // double deletion here

edited Nov 07 '16 at 21:59

answered Nov 07 '16 at 21:53

T.C.

133,968
17
288
421

These all seem completely legit and reasonable, except for the `bool` assignment, but that's pretty nonsensical even with a regular pointer. – einpoklum Nov 07 '16 at 21:55
1

@einpoklum You think it's completely legit to allow pointer arithmetic and subscripting on a `unique_ptr` for a single object? – T.C. Nov 07 '16 at 21:56
Yes - that is, as much as it would be with a plain pointer. You only know it's a single object because you know how the unique_ptr was initialized. – einpoklum Nov 07 '16 at 21:57
@einpoklum Nope. `unique_ptr` is for single objects; `unique_ptr` is for array-like things. Different use cases, different interface. – T.C. Nov 07 '16 at 21:58
1

@einpoklum Also I fail to see how double deletion is "completely legit". – T.C. Nov 07 '16 at 21:58
2

@einpoklum The fact that you think it's completely legit is EXACTLY why implicit conversion should be disallowed! If you think it's legit then you are likely to code it that way and if you do the airplane that is running your program in it's autopilot will fall out of the sky and make a huge crater in Western Kansas. – Dale Wilson Nov 07 '16 at 22:02
@DaleWilson, T.C. - With due respect, you are missing the point. The point is whether there are pitfalls of using pointers or pitfalls of implicit conversion of `unique_ptr`'s - and it's the latter, not the former. You have to watch out for double deletions just the same when initializing a the `unique_ptr` with a regular pointer. – einpoklum Nov 07 '16 at 23:16
@einpoklum So you are happy debugging a problem where you forgot a `std::move` in `unique_ptr b(std::move(d));` and instead of a compile-time error you get undefined behavior at run time. Fine. Count me out. – T.C. Nov 07 '16 at 23:18
2

@ein unique ptr is not "raw pointer with RAII". It also semantically guards agains common errors. You can still use pointer arithmetic, double-delete, etc; but you have to say `.get()` first. The implicit conversion to pointer is *full* of pitfalls, and requiring `.get()` fixes piles of bugs before they happen. The cost is 6 characters when you want to convert to pointer, the benefit is an everest of avoided bugs. You miss a `move` and your code double deletes! unique ptrs are *easy* to use, it is hard to screw them up. Add implicit conversion and they become almost as bad as raw ones! – Yakk - Adam Nevraumont Nov 08 '16 at 00:51
1

@Yakk I would argue that they would be worse than raw ones, because you get an illusion of safety :) – T.C. Nov 08 '16 at 19:55
@T.c. `auto_ptr ` – Yakk - Adam Nevraumont Nov 08 '16 at 19:59

Pitfalls of "making" unique_ptr implicitly convertible to T*?

2 Answers2