magras
magras

Reputation: 1741

reinterpret_cast template class from non-const to const version

I have template class ImageView<Pixel> which stores a non-owning pointer to data and image size.

I'd like to have const correctness, so I'm working with both Pixel and const Pixel:

std::byte * data;
ImageView<char> img(data, width, height);
std::byte const* cdata;
ImageView<char> img(cdata, width, height); // compile error
ImageView<const char> cimg(cdata, width, height);

But of course it leads to a problem with a code like this:

void foo(ImageView<const char> const&);

ImageView<char> view;
foo(view); // conversion from ImageView<char> to ImageView<const char> const& required

Obvious solution is to add implict conversion using constructor:

template <class = std::enable_if_t<std::is_const_v<Pixel>>>
constexpr ImageView(ImageView<std::remove_const_t<Pixel>> const& other) noexcept
    : m_data(other.GetData())
    , m_stride(other.GetStride())
    , m_width(other.GetWidth())
    , m_height(other.GetHeight())
{}

But it has drawback of creating temporary on each conversion and ImageView is 24 bytes on most 64 bit platforms. This temporaries differ from original only by type — they have exactly the same layout. So I started to think about using reinterpret_cast and const reference conversion operator:

template <class = std::enable_if_t<!std::is_const_v<Pixel>>>
constexpr operator ImageView<std::add_const_t<Pixel>> const&() const noexcept
{
    using ConstImageView = ImageView<std::add_const_t<Pixel>>;
    return *reinterpret_cast<ConstImageView const*>(this);
}

It seems to work, but I'm not sure about the correctness of the last snippet.


There is simplified (only some additional non-virtual functions omitted) version of a whole class:

template <class Pixel>
class ImageView
{
    template <class T, class U>
    using copy_const_qualifier =
        std::conditional_t<
            std::is_const_v<T>,
            std::add_const_t<U>,
            std::remove_const_t<U>>;

    using Byte = copy_const_qualifier<Pixel, std::byte>;

public:

    constexpr ImageView(Byte * data, unsigned w, unsigned h, std::size_t s) noexcept
        : m_data(data)
        , m_stride(s)
        , m_width(w)
        , m_height(h)
    {}

    constexpr Byte * GetData() const noexcept { return m_data; }
    constexpr std::size_t GetStride() const noexcept { return m_stride; }
    constexpr unsigned GetWidth() const noexcept { return m_width; }
    constexpr unsigned GetHeight() const noexcept { return m_height; }

protected:
    Byte * m_data;
    std::size_t m_stride; // in bytes
    unsigned m_width; // in pixels
    unsigned m_height; // in pixels
};

Upvotes: 2

Views: 406

Answers (3)

magras
magras

Reputation: 1741

Indeed it's possible to achieve desired behavior using @YCS's idea, but it requires more complicated code and const_casts to work with m_data. const_cast here is safe, because constructor of non-const ImageView accepts pointers to non-const data only.

So for now, I'll keep version with conversion constructor or operator. If I'll notice significant impact of temporaries on performance I'll return to this code:

template <class Pixel>
struct ImageView : public ImageView<const Pixel>
{
    constexpr ImageView(Pixel * data) noexcept
        : ImageView(data)
    {}

    constexpr Pixel * GetData() const noexcept
    {
        const Pixel * data = ImageView<const Pixel>::GetData();
        return const_cast<Pixel*>(data);
    }
};

template <class Pixel>
struct ImageView<const Pixel>
{
    constexpr ImageView(const Pixel * data) noexcept
        : m_data(data)
    {}

    constexpr const Pixel * GetData() const noexcept
    {
        return m_data;
    }

private:
    const Pixel * m_data;
};

int main()
{
    int * data = nullptr;
    const int * cdata = nullptr;

    ImageView<int> img(data);
    //ImageView<int> img1(cdata); // compile error
    ImageView<const int> cimg(data);
    ImageView<const int> cimg1(cdata);

    auto img2 = img;
    auto cimg2 = cimg;

    ImageView<const int> cimg3(img);
    ImageView<const int> cimg4 = static_cast<ImageView<const int>>(img);
    ImageView<const int> cimg5 = img;

    img.GetData();
    cimg.GetData();

    return 0;
}

Upvotes: 0

Rakete1111
Rakete1111

Reputation: 48938

Yes, that reinterpret_cast is invalid, you can't just cast an object to another object of unrelated type. Well you can, but don't access it aftwards please.

You can add a conversion operator instead of disabling away the implicit constructor, which can't work because you're using SFINAE in a non-overload resolution context (there are workarounds, like making the condition dependent which would achieve the same goal). But using the conversion operator is cleaner IMO:

operator ImageView<const Pixel>() { return {m_data, m_width, m_height, m_stride}; }

You don't have to worry about doing copies, compilers are smart! :) And 24 bytes are really nothing to worry about.

Look at the assembly here yourself. gcc generates the same code on -O1 and above for passing a ImageView<const char> and ImageView<char> to foo, and for clang above -O2. So no difference at all if you compile with optimization.

Upvotes: 4

YSC
YSC

Reputation: 40060

Even though a const char is implicitly convertible to a char, ImageView<char> and ImageView<const char> are completely unrelated types, I learn you nothing here. But it's a shame since, in a sense, a ImageView<const char> is a ImageView<char> on which modification is possible.

Fortunately, we have a tool to tel the compiler something is a something else. This is the definition (according to the Liskov rule) of inheritance. And this is it. Making ImageView<const char> inherit from ImageView<char> resolve most of your problems, and it makes sense:

template<class T>
struct ImageView {};

template<class T>
struct ImageView<const T> : ImageView<T>
{};

void f(ImageView<char>&) {}
void f_const(ImageView<const char>&) {}

int main()
{
    ImageView<char> d1;
    ImageView<const char> d2;

    f(d1);
    f(d2);
    //f_const(d1); // error: invalid initialization of reference of type 'ImageView<const char>&' from expression of type 'ImageView<char>'
    f_const(d2);
}

DEMO

Upvotes: 1

Related Questions