Why don't static_pointer_cast and similar functions employ perfect forwarding?

[eyebrow...@gmail.com]

I ran into an issue with something similar to this today:

class A
{
};

class B : A
{
};

std::vector<std::shared_ptr<A>> vector;

std::shared_ptr<B> movedPointer = std::static_pointer_cast<B>(std::move(vector[0]));
// do something with movedPointer
movedPointer.reset();

This ended up with some mysterious behavior, because I was expecting the pointer in the vector to be moved-from (nullptr), and for the object to deallocate when I reset my pointer. As it turns out though, static_pointer_cast's signature only takes a const ref to a shared pointer, it doesn't have an overload that takes an rvalue reference. Hence, my pointer was copied, not moved. It is specified this way even in the standard. Is there a reason for this decision, or was this just an oversight?

Thanks,

Jackson

[Michael Witten]

On Wed, 18 Apr 2018 15:18:02 -0700 (PDT), Jackson wrote:

I ran into an issue with something similar to this today:

class A
{
};

class B : A
{
};

std::vector<std::shared_ptr<A>> vector;

std::shared_ptr<B> movedPointer = std::static_pointer_cast<B>(std::move(vector[0]));
// do something with movedPointer
movedPointer.reset();

This ended up with some mysterious behavior, because I was expecting the pointer in the vector to be moved-from (nullptr), and for the object to deall­ocate when I reset my pointer. As it turns out though, static_pointer_cast's signature only takes a const ref to a shared pointer, it doesn't have an overload that takes an rvalue reference. Hence, my pointer was copied, not moved. It is specified this way even in the standard. Is there a reason for this decision, or was this just an oversight?

It looks like an oversight.

As described below, it looks like the solution is 2-fold:

• Introduce to std::shared_ptr an "aliasing" constructor with move semantics.

• Implement std::static_pointer_cast (and similar functions) with perfect for­warding.

What do you think?

---

Here is a version of your code that actually compiles and runs without error (at least on my system):

#include <vector>  // std::vector
#include <memory>  // std::shared_ptr, std::static_pointer_cast
#include <utility> // std::move

class A
{
};

class B : public A
{
};

int main()
{
  std::vector<std::shared_ptr<A>> vector(1);

  std::shared_ptr<B> movedPointer = std::static_pointer_cast<B>(std::move(vector[0]));
  // do something with movedPointer
  movedPointer.reset();
}

I suppose you intended for this line:

std::shared_ptr<B> movedPointer = std::static_pointer_cast<B>(std::move(vector[0]));

to produce side effects equivalent to the following:

std::shared_ptr<B> movedPointer = std::static_pointer_cast<B>(vector[0]);
vector[0].reset();

Unfortunately, that's not fully equivalent to your intention, because it has the overhead of calling the reset member; according to the Standard, section [util.smartptr.shared.mod], the line:

vector[0].reset();

is equivalent to:

std::shared_ptr<A>().swap(vector[0]);

If that's how reset is implemented, then the overhead includes the construction of a temporary object, whose contents are swapped out, and then whose destructor is called, all of which I'm sure you wish to avoid.

Essentially, the problem is that your case isn't a matter of simply swapping data; your static cast involves calculating a new pointer value, and using that instead of the old one:

static_cast<B*>(vector[0].get())

Alas, there is currently no means by which to use this new pointer value without engaging in aliasing semantics, which implies the need to revoke the ownership held by vector[0]; currently, there is no way to say "Move these data and also change this one datum."

I suppose this identifies the oversight which affects your case: The template std::shared_ptr is lacking an aliasing constructor with move semantics:

template<class Y> shared_ptr(shared_ptr<Y>&& r, element_type* p) noexcept;

Given that additional aliasing constructor, static_pointer_cast can be implem­ented with move semantics:

template<class T, class Y> 
std::shared_ptr<T> static_pointer_cast(std::shared_ptr<Y>&& r) noexcept
{
    using new_type = std::shared_ptr<T>;
    using old_type = std::shared_ptr<Y>;
    using pointer = typename new_type::element_type*;

    pointer alias = static_cast<pointer>(r.get());
    return new_type(std::forward<old_type>(r), alias);
}

That fixes the problem; with that, your code does indeed work as you intended, a fact that I've verified by making the necessary changes in my system's im­plem­entation.

For instance, consider the following program:

#include <vector>   // std::vector
#include <memory>   // std::shared_ptr, std::static_pointer_cast
#include <utility>  // std::move
#include <iostream> // std::cout

class A
{
};

class B : public A
{
};

using String = const char*;

template <class T>
static void print(const String name, const std::shared_ptr<T>& p)
{
  std::cout << name << ": " << p.get() << ' ' << p.use_count() << '\n';
}

static void blank_line()
{
  std::cout << '\n';
}

static constexpr String v = "vector[0]";
static constexpr String m = "movedPointer";

#define print_v0() \
  print(v,v0); \
  blank_line();

#define print_end() \
  print(v,v0); \
  print(m,movedPointer);

#define print_both() \
  print_end(); \
  blank_line();

int main()
{
  std::vector<std::shared_ptr<A>> vector(1);

  auto& v0 = vector[0];                print_v0();
  v0.reset(new A);                     print_v0();
  std::shared_ptr<B> movedPointer =
    std::static_pointer_cast<B>
    (
      std::move(v0)
    );                                 print_both();
  v0.reset(new A);                     print_both();
  v0 = movedPointer;                   print_both();
  movedPointer.reset();                print_both();
  v0.reset();                          print_end();
}

This produced the following output on my system:

vector[0]: 0 0

vector[0]: 0xabef90 1

vector[0]: 0 0
movedPointer: 0xabef90 1

vector[0]: 0xabefc0 1
movedPointer: 0xabef90 1

vector[0]: 0xabef90 2
movedPointer: 0xabef90 2

vector[0]: 0xabef90 1
movedPointer: 0 0

vector[0]: 0 0
movedPointer: 0 0

Just for the sake of fun, you can simulate the behavior without modifying your system's implementation. In your simple case, the new pointer value (the alias) is the same as the old pointer value; that is, the following is true:

vector[0].get() == static_cast<B*>(vector[0].get())

So, you can exploit the move constructor of std::shared_pointer to manually perform the move and cast simultaneously:

std::shared_ptr<B> movedPointer(reinterpret_cast<std::shared_ptr<B>&&>(vector[0]));

Lastly, I'll note that if your code requires downcasting (from base class A to der­ived class B), then that suggests there is a flaw in the design of your pro­gram; there's no guarantee at run-time that this will work, and thus you may end up shooting yourself in the foot if you ever stuff an object into vector that is in­compatible with such casting. The whole point of a type system is to give the compiler enough information to write code for you (especially code that protects your feet), or to fail explicitly if it cannot do so with the intended guarantees.

I suspect that all of these problems are best solved by avoiding them in the first place.

Sincerely,
Michael Witten