70

I have a series of factories that return unique_ptr<Base>. Under the hood, though, they are providing pointers to various derived types, i.e unique_ptr<Derived>, unique_ptr<DerivedA>, unique_ptr<DerivedB>etc.

Given DerivedA : Derived and Derived : Base we'd have:

unique_ptr<Base> DerivedAFactory() {
    return unique_ptr<Base>(new DerivedA);
}

What I need to do is to "cast" the pointer from the returned unique_ptr<Base> to some derived level (not necessarily the original internal one). To illustrate in pseudo code:

unique_ptr<Derived> ptr = static_cast<unique_ptr<Derived>>(DerivedAFactory());

I'm thinking of doing this by releasing the object from the unique_ptr, then using a function that casts the raw pointer and reassigns that to another unique_ptr of the desired flavor (the release would be explicitly done by the caller prior to the call):

unique_ptr<Derived> CastToDerived(Base* obj) {
    return unique_ptr<Derived>(static_cast<Derived*>(obj));
}

Is this valid, or is / will there be something funky going on?

PS. There is an added complication in that some of the factories reside in DLLs that are dynamically loaded at run-time, which means I need to make sure the produced objects are destroyed in the same context (heap space) as they were created. The transfer of ownership (which typically happens in another context) must then supply a deleter from the original context. But aside from having to supply / cast a deleter along with the pointer, the casting problem should be the same.

Praetorian
  • 106,671
  • 19
  • 240
  • 328
d7samurai
  • 3,086
  • 2
  • 30
  • 43
  • 1
    I would let `CastToDerived` take a `unique_ptr&&`. The reason why there are no casts equivalent to `static_pointer_cast` for `shared_ptr` is that casts typically do not modify their argument. But for `unique_ptr`, you'd have to *move* the pointer from the argument to the object returned by the cast. – dyp Jan 16 '14 at 23:09
  • @dyp I assume that `swap` could be a good option too in this case – user2485710 Jan 16 '14 at 23:11
  • @user2485710 Could you elaborate? – dyp Jan 16 '14 at 23:12
  • @d7samurai (continuing..) Your `CastToDerived` could be called via `CastToDerived(my_ptr.get())` (which is an error) and `CastToDerived(my_ptr.release())` (which is correct). To prevent the former, I suggest using something like `CastToDerived( std::move(my_ptr) )` which is explicit and maybe a bit less error prone. Alternatively, make it explicit in the name, like `move_static_cast(my_ptr)`. – dyp Jan 16 '14 at 23:14
  • So `unique_ptr&&` (or rather `unique_ptr&&`, as templated types cannot be exposed across DLL boundaries) would implicitly perform the move / transfer? Could I then just do such a move operation directly without the need for a function? And would such a move retain the deleter from the original unique_ptr? – d7samurai Jan 16 '14 at 23:31
  • Yes, the way it is sketched implies that the caller does a my_ptr.release(). The reason for that is of course that the caller must then be aware of the release. – d7samurai Jan 16 '14 at 23:33
  • @d7samurai Yes, but IMHO that's error prone, as the caller might as well use `my_ptr.get()` instead. Therefore either enforce the use of a `std::move` at the call site or let the cast function move the pointer, but then the name of the cast function needs to convey that it alters its argument. – dyp Jan 16 '14 at 23:35

1 Answers1

50

I'd create a couple of function templates, static_unique_ptr_cast and dynamic_unique_ptr_cast. Use the former in cases where you're absolutely certain the pointer is actually a Derived *, otherwise use the latter.

template<typename Derived, typename Base, typename Del>
std::unique_ptr<Derived, Del> 
static_unique_ptr_cast( std::unique_ptr<Base, Del>&& p )
{
    auto d = static_cast<Derived *>(p.release());
    return std::unique_ptr<Derived, Del>(d, std::move(p.get_deleter()));
}

template<typename Derived, typename Base, typename Del>
std::unique_ptr<Derived, Del> 
dynamic_unique_ptr_cast( std::unique_ptr<Base, Del>&& p )
{
    if(Derived *result = dynamic_cast<Derived *>(p.get())) {
        p.release();
        return std::unique_ptr<Derived, Del>(result, std::move(p.get_deleter()));
    }
    return std::unique_ptr<Derived, Del>(nullptr, p.get_deleter());
}

The functions are taking an rvalue reference to ensure that you're not pulling the rug out from underneath the caller's feet by stealing the unique_ptr passed to you.

Praetorian
  • 106,671
  • 19
  • 240
  • 328
  • 6
    You need to extract the deleter from the source object and inject it into the destination object. The type is most probably not sufficient. – David Rodríguez - dribeas Jan 16 '14 at 23:26
  • @DavidRodríguez-dribeas Good point, thanks. I've updated the answer. Any idea whether `get_deleter()` must return a valid result after you've called `release()`? – Praetorian Jan 16 '14 at 23:31
  • 1
    @Praetorian [unique.ptr.single.asgn] for example says: "*Effects:* Transfers ownership from `u` to `*this` as if by calling `reset(u.release())` followed by an assignment from `std::forward(u.get_deleter())`." So I think it's well-defined, although the deleter doesn't occur in the postconditions of `release` – dyp Jan 16 '14 at 23:39
  • @Praetorian should it be `...result != dynamic_cast<...` ? – d7samurai Jan 16 '14 at 23:47
  • @d7samurai No, I'm assigning the result of the `dynamic_cast` to `result` and checking whether it is `nullptr`, all in one expression. But now it seems unnecessary to do that. I'll update the answer. – Praetorian Jan 16 '14 at 23:49
  • Indeed you are :) I wasn't looking closely enough. My own implementation of the same check (the one that enables me to subsequently use static_cast) compares the result of the dynamic_cast against a nullptr, since the compiler said something about implicit conversion to bool).. I was seeing my code instead of yours, hence the misread. – d7samurai Jan 16 '14 at 23:54
  • 1
    Btw. the deleter can be move-only, this makes the final return statement a bit difficult (you can't copy, you shouldn't move either, and maybe you can't default-construct). Maybe if it's move-only, you could *require* `DefaultConstructible`; another way is to throw an exception instead of returning. – dyp Jan 16 '14 at 23:57
  • @dyp Thanks, you just made me realize I could've been moving the deleter all along. Maybe you could SFINAE using `is_move_constructible` to a version that throws when the `dynamic_cast` fails. But I think I prefer the compilation failure in this case to silently switching between versions that throw and do not throw. Edit: I didn't mean add the `move` to the last one. – Praetorian Jan 17 '14 at 00:08
  • Sorry for all the trouble ;) – dyp Jan 17 '14 at 00:24
  • @dyp Not any trouble at all, constructive criticism is always welcome. – Praetorian Jan 17 '14 at 00:26
  • Why did you delete [that answer](http://stackoverflow.com/questions/21195217/elegant-way-to-remove-all-elements-of-a-vector-that-are-contained-in-another-vec#21196215)? All I wanted to note is that you can't just use *any random* StdLib's `set_difference` without paying attention if it provides the additional guarantee to work on overlapping ranges. The `copy` could be tricky, though. (sry for hijacking this question) – dyp Jan 17 '14 at 21:36
  • @dyp Because he *is* trying to do it in-place. And the one I posted doesn't quite cut it in that case. It contains potential self-assignments, which would invalidate iterators (I think) and a call to `std::copy` which again requires non-overlapping ranges. – Praetorian Jan 17 '14 at 21:45
  • 5
    @Praetorian Hi, I've been trying to use this and am able to get it to work when I use a custom deleter while initializing the unique_ptr(http://pastebin.com/EABRT9G3). But, am not sure of how to do it with the default deleter (http://pastebin.com/BNDBivff) - In this case, the compiler complains about there not being a viable conversion between the derived/base class deleters. Is it possible to use this code with default delteters? If so, could you look at my second paste and let me know how I need to call the cast function with the right template arguments? Thanks :) – georgemp Jul 31 '14 at 16:21
  • 1
    @georgemp That won't work because both the `unique_ptr` cast functions above make a copy of the deleter as is, so in your last example, the return value from the function is of type `unique_ptr>`, and you're trying to assign it to a `unique_ptr>`. Just use `auto d = dynamic_unique_ptr_cast(std::move(b));` instead. And `~Base` **must** be `virtual` for the second example to work. – Praetorian Jul 31 '14 at 18:47
  • What happens if after `p.release()`, in the `unique_ptr` constructor, an exception is thrown? Will the pointer be leaked? Can you call `p.release()` after the `unique_ptr` constructor (and before `return`) to avoid this? Since you are moving the deleter during the constructor you should never end up with two `unique_ptr` objects deleting the same thing, right? – Malvineous Aug 01 '15 at 11:07
  • 2
    @Malvineous `unique_ptr` constructors are `noexcept`. But what you suggest, first constructing the second `unique_ptr`, and then `release()`ing ownership of the original, would be the more foolproof option if copying/moving the deleter can throw. – Praetorian Aug 02 '15 at 01:09
  • Shouldn't the function take the argument unique_ptr by value, because it will take ownership of p? See the discussion here: http://stackoverflow.com/questions/21174593/downcasting-unique-ptrbase-to-unique-ptrderived – Knitschi Nov 10 '15 at 07:30
  • @Knitschi Maybe you meant to link to [this answer](http://stackoverflow.com/a/8114913/241631)? If yes, I agree, taking the argument by value isn't a bad idea either. But there is some debate on the subject, for example Scott Meyers argues it should be taken by rvalue reference [here](http://scottmeyers.blogspot.com/2014/07/should-move-only-types-ever-be-passed.html). – Praetorian Nov 10 '15 at 20:04
  • Yes I wanted to link to that answer. Thanks for informing me that this topic is still under debate. Now I don't know how to write c++ anymore :-(. – Knitschi Nov 11 '15 at 09:48
  • If moving the deleter throws the code above leaks? Second, is there any way to detect if both the default deleter is being used and it is sufficient? (Ie virtual dtor) Also dynamic version double-deletes? – Yakk - Adam Nevraumont Jan 26 '17 at 15:37
  • @Yakk Yes, if moving the deleter throws there's a leak, I don't see a way around that, do you? I suppose you could tag dispatch to handle `default_delete` being used, but is it worth it since copying/moving that is cheap? I don't know how you determine whether `default_delete` is *sufficient*, but that would solve the throwing move problem in some cases. About double deletion, are you asking about the return statement where `nullptr` is returned? The deleter should never be called for the returned `unique_ptr` in that case. – Praetorian Jan 26 '17 at 17:16
  • @praet no, I just somehow missed the `p.release();` line. In that case you do end up eith a moved-from unique ptr that did not end up empty, which may surprise. I do not know how to avoid that. – Yakk - Adam Nevraumont Jan 26 '17 at 17:19
  • To fix the throw problem, first create retval temporary. Then release source pointer. Then return temporary. – Yakk - Adam Nevraumont Jan 26 '17 at 17:20
  • I don't know which compiler you are using but without using the auto keyword, this cannot compile if you are trying to put the result of the static/dynamic cast in a unique_ptr as the op stated. http://rextester.com/QNOP12810. The link I provided shows which type you would have to carry around for this static cast to work. This is less that ideal. Any idea how to make it work with a real unique_ptr ? – Jean-Simon Brochu Aug 28 '17 at 19:22
  • 1
    I think I might have fixed it with some enable_if: http://rextester.com/KFSF97862 – Jean-Simon Brochu Aug 28 '17 at 19:52
  • 1
    @Jean-SimonBrochu why should this fix anything? The deleter still has the same type and your example does not compile. The lean `static_cast` is just replaced by an expensive `dynamic_cast`, which is unnecessary. I am also looking for a solution, this answer needs a unique_ptr as return type whose first template argument fits `Derived` but we are looking for `unique_ptr`, not anything else. Or is there no solution? – IceFire Oct 10 '19 at 07:59
  • Ok, since no one answers here anymore, maybe consider this follow up question which clarifies a bit more: https://stackoverflow.com/questions/58318716/why-is-static-downcasting-unique-ptr-unsafe – IceFire Oct 11 '19 at 07:59
  • @IceFire It does compile ( as you can verify by following the link I provided ) but it does not help with the dynamic_cast as you mentioned. I just tried to fix the code that was not compiling and tried to understand what the answer fixed... which I don't think is anything. – Jean-Simon Brochu Nov 21 '19 at 15:13
  • @Jean-SimonBrochu yeah, it does now. I was looking at a time where the code did not... anyways, never mind. The answer to my follow up question under the link helped me at least a bit, maybe it also is of value for you. – IceFire Nov 21 '19 at 15:40
  • This one simply doesn't work when the deleters are different. Period. If someone claims otherwise, get a link to godbolt. I had no success whatsoever making this work for any non-trivial type. – Kuba hasn't forgotten Monica May 31 '20 at 16:59