Most efficient safe way to cast std::map<int, std::shared_ptr<Base>> to std::map<int, std::shared_ptr<Derived>>

Question

We currently store several different collections of data models like so:

std::map<std::string, std::map<int64_t, std::shared_ptr<DataObject>>> models;

The string maps to a known list of types and this is all dealt with by serialization. The nested map contains a collection of "Object Id" and associated (deserialized) std::shared_ptr<DataObject>

DataObject is a base class which we have several types derived from.

We have a method to get all DataObjects of a given type:

static std::map<int64_t, std::shared_ptr<DataObject>> *getAll(std::string type);

This simply returns a pointer to the map at a given "type" key.

Today I encountered a code review to add the following which I believe invokes UB but seems to function. This has me a little nervous and looking for an efficient solution:

template <typename M>
static std::map<int64_t, std::shared_ptr<M>> *getAll(const std::string &type) {
    auto castObjectMap = reinterpret_cast<std::map<int64_t, std::shared_ptr<M>>*>(getAll(type));
    return castObjectMap;
}

The benefit to casting like this is that it involves no looping or copying (as far as I can tell), it's a simple pointer cast. That's fantastic. But I don't think it's portable or safe, though I don't know what the best portable and safe alternative would be.

I have a simplified version of a toy version of this question below, it does actually spit out the correct output as expected:

#include <iostream>
#include <memory>
#include <string>
#include <map>

using namespace std;

struct Base {
    Base(){}
    virtual ~Base(){}

    Base(int u):x(u){}

    int x = 0;
};

struct Derived : public Base {
    Derived(){}
    Derived(int u, int v):Base(u),y(v){}
    int y = 0;
};

int main() {
    map<int, shared_ptr<Base>> test {
        {0, make_shared<Derived>(2, 3)},
        {1, make_shared<Derived>(4, 5)},
        {2, make_shared<Derived>(6, 7)}
    };

    map<int, shared_ptr<Derived>> *castVersion = reinterpret_cast<map<int, shared_ptr<Derived>>*>(&test);

    for(auto&&kv : *castVersion){
        cout << kv.first << ": " << kv.second->x << ", " << kv.second->y << std::endl;
    }
    return 0;
}

My question is if there's a good way to do this which doesn't involve a lot of copying, or if there's a clean way to do so at least. We are using C++17 presently.

Answer 1

It won't give exactly the same interface, but a similar but safer idea that comes to mind is using boost::transform_iterator to create iterators that transparently handle converting the shared_ptr pointers within the map.

#include <memory>
#include <utility>
#include <type_traits>
#include <boost/iterator/transform_iterator.hpp>

template <class Derived, class Iterator>
class downcast_pair_iterator
    : public boost::transform_iterator<
        std::pair<
            typename std::iterator_traits<Iterator>::value_type::first_type,
            const std::shared_ptr<Derived>
        > (*)(Iterator),
        Iterator>
{
public:
    using base_value_type = typename std::iterator_traits<Iterator>::value_type;
    using key_type = const typename base_value_type::first_type;
    using base_mapped_type = typename base_value_type::second_type;
    using mapped_type = const std::shared_ptr<Derived>;
    using value_type = std::pair<key_type, mapped_type>;

private:
    template <typename T>
    static T* shared_to_raw(const std::shared_ptr<T>&); // undefined
    static_assert(std::is_base_of_v<
        std::remove_pointer_t<
            decltype(shared_to_raw(std::declval<base_mapped_type&>()))>,
        Derived>);

    static value_type convert(const base_value_type& pair_in)
    {
        return value_type(pair_in.first,
            std::static_pointer_cast<Derived>(pair_in.second));
    }
public:
    explicit downcast_pair_iterator(Iterator iter)
        : transform_iterator(iter, &convert) {}
};

template <class Derived, class Iterator>
auto make_downcast_pair_iter(Iterator iter)
{
    return downcast_pair_iterator<Derived, Iterator>(iter);
}

template <class Derived, class Range>
class downcast_pair_range
{
public:
    explicit downcast_pair_range(Range& c)
        : source_ref(c) {}

    auto begin() const {
        using std::begin;
        return make_downcast_pair_iter<Derived>(begin(source_ref));
    }
    auto end() const {
        using std::end;
        return make_downcast_pair_iter<Derived>(end(source_ref));
    }

private:
    Range& source_ref;
};

template <class Derived, class Range>
auto make_downcast_pair_range(Range& r)
{
    return downcast_pair_range<Derived, Range>(r);
}
template <class Derived, class Range>
auto make_downcast_pair_range(const Range &r)
{
    return downcast_pair_range<Derived, const Range>(r);
}

Then your example main could become:

int main() {
    std::map<int, std::shared_ptr<Base>> test {
        {0, std::make_shared<Derived>(2, 3)},
        {1, std::make_shared<Derived>(4, 5)},
        {2, std::make_shared<Derived>(6, 7)}
    };

    for (auto&& kv : make_downcast_pair_range<Derived>(test)){
        std::cout << kv.first << ": "
                  << kv.second->x << ", " << kv.second->y << std::endl;
    }
    return 0;
}

This avoids making any second container object, and does not involve undefined behavior when used correctly. Using the transforming iterators will mostly result in the same machine code as the unsafe cast, except that a dereference does create a new shared_ptr<Derived> object, which will involve a little reference-counting overhead. See the full working program on coliru.

In addition to using make_downcast_pair_range<Derived>(some_map) as shown in the range-based for above, make_downcast_pair_iterator<Derived> can be used directly to get transforming iterators for other purposes, for example from the result of a map's find(k). And given a transforming iterator, you can get back to an iterator for the real map using iter.base(), for example to pass to the map's erase(iter).

Of course, using the result of std::static_pointer_cast is still undefined behavior if the pointers aren't actually pointing at Derived objects. If there's any concern someone might use the wrong "derived" template argument when getting objects, or that the maps might somehow end up containing pointers to the wrong derived object types, you could change the downcast_pair_iterator<D, I>::convert private function to use std::dynamic_pointer_cast instead, then throw or abort if the result is a null pointer.

Answer 2

You cannot directly use the std::map constructor taking a pair of iterators because the conversion is attempting to cast from Base to Derived which cannot be done implicitly, but you can run an std::transform safely. This does involve copying but has the benefit of not being undefined behavior.

template <typename M>
static std::map<int64_t, std::shared_ptr<M>> getAll(const std::string &type) {
    auto* source = getAll(type);
    std::map<int64_t, std::shared_ptr<M>> castMap;
    std::transform(source->begin(), source->end(), std::inserter(castMap, castMap.end()), [](auto& kv) {
        return std::pair<const int, std::shared_ptr<M>>(kv.first, std::static_pointer_cast<M>(kv.second));
    });
    return castMap;
}