4

If I have the following:

class Animal {};
class Penguin : public Animal {};
class Snake : public Animal {};

class Zoo
{
    std::vector<std::shared_ptr<Animal>> animals;

public:

    const std::vector<std::shared_ptr<Animal>>& GetAnimals() { return animals; }

    std::shared_ptr<Penguin> AddPenguin()
    {
        auto result = std::make_shared<Penguin>();
        animals.push_back(result);
        return result;
    }

    std::shared_ptr<Snake> AddSnake()
    {
        auto result = std::make_shared<Snake>();
        animals.push_back(result);
        return result;
    }
};

I'd like to keep const correctness, and be able to add the following method:

    const std::vector<std::shared_ptr<const Animal>>& GetAnimals() const
    {
        return animals;
    }

However, that doesn't compile because the return type doesn't match animals. As the const is embedded deep in the type, const_cast isn't able to convert.

However, this compiles and appears to behave:

    const std::vector<std::shared_ptr<const Animal>>& GetAnimals() const
    {
        return reinterpret_cast<const std::vector<std::shared_ptr<const Animal>>&>(animals);
    }

I realize reinterpret_casts can be dangerous, but are there any dangers to use it in this scenario? Do the types being cast between have the same memory layout? Is the only effect of this to prevent the caller from then calling any non-const methods of Animal?


Update I've realized this isn't entirely const correct. The caller could call .reset() on one of the elements of the vector, which would still modify the zoo. Even so, I'm still intrigued what the answer is.


Update on the update I got that wrong, the code I was trying accidentally copied the shared_ptr and so the shared_ptr in the vector can't be reset when the vector is const.

Scott Langham
  • 58,735
  • 39
  • 131
  • 204
  • regarding const correctness `auto GetAnimals() { return animals;}` is just fine. Did you mean to make the method `const` ? – 463035818_is_not_an_ai Dec 12 '22 at 18:48
  • @463035818_is_not_a_number oops, yes I've updated the question. – Scott Langham Dec 12 '22 at 18:50
  • 1
    I don't think there is any guarantee that compiler doesn't specialize `const` version, it's not likely though. – apple apple Dec 12 '22 at 18:56
  • 1
    re-update: you cannot reset a `const std::shared_ptr` – apple apple Dec 12 '22 at 18:58
  • 1
    strictly undefined behaviour as the two types are unrelated, you might get lucky, you might not. Easier just to copy to a new temporary vector – Alan Birtles Dec 12 '22 at 18:58
  • 1
    or just write a wrapper for `shared_ptr` whose const-qualified methods return const qualified types (ie, `T const* operator->() const`, `T const& operator*() const`, ...). Then constness will propogate through just fine. – Useless Dec 12 '22 at 19:04
  • @appleapple Yes, a const std::shared_ptr can't be reset, but I get a compiler warning telling me that the standard prohibits storing const types in a vector! – Scott Langham Dec 12 '22 at 19:08
  • 1
    This is undefined behavior. – Sam Varshavchik Dec 12 '22 at 19:12
  • 1
    *"the standard prohibits storing const types in a vector!"* Right! If you insert or erase members from the vector, the others have to be moved around. Can't do that if they are all const. – BoP Dec 12 '22 at 19:13
  • 1
    What is the use case for getting the *entire* zoo? Why not add a member for looking at *one* animal at a time, and have that return a const reference/pointer? – BoP Dec 12 '22 at 19:15
  • 2
    Might be a use case for [`std::experimental::propagate_const`](https://en.cppreference.com/w/cpp/experimental/propagate_const)? – joergbrech Dec 12 '22 at 19:27
  • @BoP The use case is being able to iterate and use std algorithms, eg std::transform. I guess I might have to write some kind of adapter with a custom iterator that `const_pointer_cast`s each while iterating. I was wondering if I could save the effort of working out how to do that. – Scott Langham Dec 12 '22 at 19:50
  • These are not interchangeable: `vector` and `vector`. These are not interchangeable: `shared_ptr` and `shared_ptr` (the latter can be made from the former with shared control block, but not vice versa). Using `reinterpret_cast` to force them is a recipe for disaster (it's **undefined behavior**). – Eljay Dec 12 '22 at 20:51
  • Related: https://stackoverflow.com/questions/2079750/c-smart-pointer-const-correctness – joergbrech Dec 12 '22 at 23:26

2 Answers2

4

std::shared_ptr<Animal> and std::shared_ptr<const Animal> are fundamentally different types. Messing with reinterpret_cast can lead to very strange bugs down the road (mostly due to optimizations, I would imagine). You have two options: create a new std::shared_ptr<const Animal> for each std::shared_ptr<Animal>, or return a complex proxy type (something like a view of the vector).

That said, I question the need for GetAnimals. If Zoo is meant to be a collection of pointers to animals, can't you provide access functions like size, operator[], and perhaps iterators? This does involve more effort, but if all you want is a function that returns the whole vector, why have a Zoo class in the first place? If Zoo contains other data and manages more than just a vector of animals, then I would make a separate class to take care of that part, AnimalList or something. That class can then provide appropriate access functions.

Something else you might try is to keep a std::shared_ptr<std::vector<Animal>> instead that you can easily convert into a std::shared_ptr<const std::vector<Animal>>. That may or may not be relevant depending on the reason you need shared pointers.

Nelfeal
  • 12,593
  • 1
  • 20
  • 39
  • My example was simplified. Zoo could contain other data besides Animal's. I considered `std::shared_ptr>`, but that has a slicing problem if a data member gets added to Penguin. Yes, I think I might be going down the proxy route with a class that has `size` `operator[]` `begin` `end`. – Scott Langham Dec 12 '22 at 19:56
  • @ScottLangham Perhaps you should have a `AnimalList` class to take care of the `std::vector>`. Then you don't need a proxy. – Nelfeal Dec 12 '22 at 20:06
2

You can probably solve your problem with std::experimental::propagate_const. It is a wrapper for pointer-like types that properly propagates const-correctness.

A const std::shared_ptr<Animal> holds a mutable Animal. Retrieving a mutable reference to the mutable animal is legal, because the pointer itself is not changed. Vice versa, a std::shared_ptr<Animal const> will always hold a const Animal. You would have to explicitly cast away constness to mutate the held element which is ugly to say the least. Dereferencing a std::experimental::propagate_const<std::shared_ptr<Animal>> on the other hand returns a Animal const& if it is const, and a Animal& if it is not const.

If you wrap your shared pointers in std::experimental::propagate_const you can equip Zoo with a const and a non-const getter for your animals vector and have const-correctness (or you could make animals a public data member, since the getters don't do anything special. This would make your API more transparent):

#include <vector>
#include <memory>
#include <experimental/propagate_const>
#include <type_traits>

class Animal {};
class Penguin : public Animal {};
class Snake : public Animal {};

class Zoo
{
    template <typename T>
    using pointer_t = std::experimental::propagate_const<std::shared_ptr<T>>;

    std::vector<pointer_t<Animal>> animals;

public:

    // const-getter
    auto const& GetAnimals() const 
    { 
        return animals; 
    }

    // non-const getter
    auto& GetAnimals() 
    { 
        return animals; 
    }

    std::shared_ptr<Penguin> AddPenguin()
    {
        auto result = std::make_shared<Penguin>();
        animals.push_back(result);
        return result;
    }

    std::shared_ptr<Snake> AddSnake()
    {
        auto result = std::make_shared<Snake>();
        animals.push_back(result);
        return result;
    }
};

int main() {

    Zoo zoo;
    zoo.AddSnake();

    // non-const getter will propagate mutability through the pointer
    {
        auto& test = zoo.GetAnimals()[0];
        static_assert(std::is_same<Animal&, decltype(*test)>::value);
    }

    // const-getter will propagate const through the pointer
    {
        Zoo const& zc = zoo;
        auto& test = zc.GetAnimals()[0];
        static_assert(std::is_same<Animal const&, decltype(*test)>::value);
    }

    return 0;
}

https://godbolt.org/z/1rd8YraMc

The only downsides I can think of are the discouraging "experimental" namespace and the fact that afaik MSVC hasn't implemented it yet, so it is not as portable as it could be....

If that bothers you, you can write your own propagate_const wrapper type, as @Useless suggested:

template <typename Ptr>
class propagate_const
{
public:
    using value_type = typename std::remove_reference<decltype(*Ptr{})>::type;

    template <
        typename T,
        typename = std::enable_if_t<std::is_convertible<T, Ptr>::value>
    >
    constexpr propagate_const(T&& p) : ptr{std::forward<T>(p)} {}

    constexpr value_type& operator*() { return *ptr; }
    constexpr value_type const& operator*() const { return *ptr; }

    constexpr value_type& operator->() { return *ptr; }
    constexpr value_type const& operator->() const { return *ptr; }

private:
    Ptr ptr;
};

https://godbolt.org/z/eGPPPxef4

joergbrech
  • 2,056
  • 1
  • 5
  • 17