CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Results 1 to 8 of 8
  1. #1
    Join Date
    Mar 2006
    Posts
    151

    is this cast to add const guaranteed to be OK?

    Hi,

    I have a class in a speed critical program which contains some data to be shared with callers outside the class. The data is reasonably large and copying it should be avoided as much as possible. However, it is imperative that the callers not be allowed to modify the data. This example program highlights the problem:

    Code:
    #include <memory>
    #include <vector>
    
    struct MyObject
    {
        int a, b, c;
    };
    
    struct MyGenerator
    {
    public:
    // No good -- allows modification of vector content.
    //    std::shared_ptr<std::vector<MyObject *>> GetData(void) const
    //    {
    //        return m_ptrvData;
    //    }
    
        std::shared_ptr<std::vector<MyObject const *> const> GetData(void) const
        {
    //        return m_ptrvData;   // C2664
            return *reinterpret_cast<std::shared_ptr<std::vector<MyObject const *> const> const *>(&m_ptrvData);
        }
    private:
        std::shared_ptr<std::vector<MyObject *>> m_ptrvData;  // The vector gets populated by other class members
    };
    
    #include <iostream>
    int main(int, char **)
    {
        MyGenerator x;
        int i = (*x.GetData())[0]->a;
    
    //    (*x.GetData())[0]->a = 5; // No! Should not be allowed!
    
        std::cout << "Done" << std::endl;
    }
    My question is, is the cast I'm making in GetData() always going to be acceptable? (Does the C-Standard address this issue?) It seems like if either vector<> or shared_ptr<> ever had a template specialization for const template arguments it could cause this to fail (catastrophically).

    Thank you,
    GeoRanger
    Last edited by GeoRanger; November 18th, 2013 at 02:44 PM. Reason: Forgot to add const to the template argument in shared_ptr

  2. #2
    Join Date
    Jan 2006
    Location
    Singapore
    Posts
    6,765

    Re: is this cast to add const guaranteed to be OK?

    I suggest a different approach: provide an iterator iterface for accessing the MyObject objects/pointers. Instead of having both iterator to non-const and iterator to const versions, you only provide the iterator to const version of begin() and end(). The iterator returned by begin() could merely be a pointer to the first MyObject pointer in the vector, or you could create an iterator class such that dereferencing the iterator results in a MyObject object rather than a MyObject pointer. Furthermore, if you ever say, change the vector to a deque, the interface does not need to change.
    C + C++ Compiler: MinGW port of GCC
    Build + Version Control System: SCons + Bazaar

    Look up a C/C++ Reference and learn How To Ask Questions The Smart Way
    Kindly rate my posts if you found them useful

  3. #3
    Join Date
    Apr 2000
    Location
    Belgium (Europe)
    Posts
    4,626

    Re: is this cast to add const guaranteed to be OK?

    If you need a cast, then "something is probably wrong in your design".

    this is even more true if you are trying to "explicitely constify" via the cast (as in this case)
    or even worse if you're trying to do the reverse.

    The reinterpret cast is particularly bad in this because it pretty much voids all the guarantees and assumptions the compiler can make about a type and you're forcibly telling the compiler that the pointer to one type needs to be accepted as being that of another type.

    I'm not saying casts have no place at all in C++ because they obviously do or they wouldn't be there. But more often than not, they're either remnants of an old design concept being "forced" into doing something it wasn't originally intended for. Or the casts are needed for interfacing your C++ design with "legacy" API's from third parties.

    Also, it's unclear what you want to achieve...
    Do you want the vector to be unmodifiable (they can't add/delete/reorganize the vector), or are the items in the vector unmodifiable. Or both ? Depending on the need, the best solution is different.
    Also, how "unmodifiable" do you want it to be. Because as you indicate here yourself, the user of your class can simply cast away the const and do the modification they wanted anyway. If you need to make that impossible, then only giving them an interface to a copy (any modification they do force has no effect) or a proxy that doesn't allow modification may be the thing you need.

  4. #4
    Join Date
    Mar 2006
    Posts
    151

    Re: is this cast to add const guaranteed to be OK?

    Thank you both for your replies!

    laserlight, I decided to use your idea and it has cured 90% of the issue. I added a small class which is able to give the appearance of being either an array or an iterator, and it only gives const access to the underlying data.

    OReubens, as you guessed, what I'm after is really the "or both" alternative. Users (which incidentally are derived classes) should not be able to modify the contents of the vector or the vector itself. My concern was that without const-ness someone could write code that would modify that data and they would never get a warning that they are writing a bug. With const-ness, they could still const_cast it away and write a bug, but at least now they are clued in that they are doing something wrong.

    The class which is doing this is a template class which is used in two different locations. laserlight's idea completely cured one of the two and made the second of the two much better than it was. There is one remaining issue with the second of the two, which I can exemplify with a modification to the toy code I posted previously:

    Code:
    struct MyObjectBase
    {
        int a, b, c;
    };
    
    struct MyGenerator
    {
    public:
        template <class T> struct MyIterator
        {
            // constructors, operators, and accessors here which only provide const access
        private:
            std::shared_ptr<std::vector<T *>> m_ptrdata;
        };
    private:
        struct MyObjectDerived : public MyObjectBase
        {
        private:
            // other stuff needed by MyGenerator but which should be invisible to outsiders
        };
    public:
        MyIterator<MyOjbectBase> GetData(void) const
        {  // Still need a cast here to up-cast the template argument?????
            return MyIterator(*reinterpret_cast<std::shared_ptr<std::vector<MyObjectBase *>> *>(&m_ptrvData);
        }
    private:
        std::shared_ptr<std::vector<MyObjectDerived *>> m_ptrvData;  // The vector gets populated by other class members
    };
    Internally in the MyGenerator class, it is maintaining a vector of pointers to MyObjectDeriveds because they all have a bunch of special meta-data, but all outsiders should perceive the objects (via the pointers to them) as just being MyObjectBase. So it seems like I'm still having to make a reinterpret_cast in order to upcast the template argument from Derived back to Base. Is there a better way to upcast the template type?

    Thanks,
    GeoRanger

  5. #5
    Join Date
    Jan 2006
    Location
    Singapore
    Posts
    6,765

    Re: is this cast to add const guaranteed to be OK?

    I don't see why you need a class template here, e.g.,
    Code:
    struct MyObjectBase
    {
        int a, b, c;
    };
    
    struct MyGenerator
    {
    private:
        struct MyObjectDerived : public MyObjectBase
        {
        private:
            // other stuff needed by MyGenerator but which should be invisible to outsiders
        };
    public:
        struct MyIterator
        {
            // constructors, operators, and accessors here which only provide const access
        private:
            std::vector<MyObjectDerived *>* m_ptrdata;
        };
    public:
        MyIterator GetData() const
        {
            return MyIterator(m_ptrvData.get());
        }
    private:
        // The vector gets populated by other class members
        std::shared_ptr<std::vector<MyObjectDerived *>> m_ptrvData;
    };
    Actually, you need more than just a pointer to the vector in the iterator object: you also need the current index, or perhaps just store an iterator from the vector (and hence you need to provide a way to obtain the "one-past-the-end" iterator). Furthermore, you don't need any extra operators and accessors in the iterator class. Rather, the iterator when dereferenced could return a pointer/reference to const MyObjectBase (or to MyObjectDerived, but I don't remember if that will work given that the definition MyObjectDerived is private).
    C + C++ Compiler: MinGW port of GCC
    Build + Version Control System: SCons + Bazaar

    Look up a C/C++ Reference and learn How To Ask Questions The Smart Way
    Kindly rate my posts if you found them useful

  6. #6
    Join Date
    Apr 2000
    Location
    Belgium (Europe)
    Posts
    4,626

    Re: is this cast to add const guaranteed to be OK?

    Quote Originally Posted by GeoRanger View Post
    Code:
        MyIterator<MyOjbectBase> GetData(void) const
        {  // Still need a cast here to up-cast the template argument?????
            return MyIterator(*reinterpret_cast<std::shared_ptr<std::vector<MyObjectBase *>> *>(&m_ptrvData);
        }
    This is not a safe cast. It's not even guaranteed to work. The above at the very least assumes that (void*)static_cast<MyOjbectBase*>(pMyDerivedObjectPtr) == (void*)pMyDerivedObjectPtr;
    And deep down, makes additional assumptions about how those are being used in a shared_ptr and a vector.

    The assumption that a derived pointer equals a parent-casted version of that pointer is typically going to work with VS, but will fail all the time on some other compilers.
    Microsoft decided to usually try and get them equal because it means they can get away with some optimisations when converting from one pointer to another (no value conversion is needed, thus less code), while other compilers forcibly make them different for added error detection.
    Also even in VS it isn't always guaranteed, among others if multiple inheritance is involved it's likely to be different.


    The only "real" and "safe" way to use reinterpret_cast is to cast a "pointer to something" into a void* (or some other pointer type) to at the place it's being used cast it back to it's original "pointer to something". If you aren't using it for that, then whatever you're doing may have all kinds of nasties involved (even if it seems to work for you in that particlular piece of code with that compiler with those compiler settings).
    Last edited by OReubens; November 20th, 2013 at 10:39 AM.

  7. #7
    Join Date
    Jan 2006
    Location
    Singapore
    Posts
    6,765

    Re: is this cast to add const guaranteed to be OK?

    Quote Originally Posted by OReubens
    The only "real" and "safe" way to use reinterpret_cast is to cast a "pointer to something" into a void* (or some other pointer type) to at the place it's being used cast it back to it's original "pointer to something".
    That doesn't make sense, but if we replace "to at the place" with "and at the place", it makes sense, so I think there's a typo there. That said, you don't need a cast at all to convert a "pointer to an object" to a "pointer to void", and you only need a static_cast to convert it back.
    C + C++ Compiler: MinGW port of GCC
    Build + Version Control System: SCons + Bazaar

    Look up a C/C++ Reference and learn How To Ask Questions The Smart Way
    Kindly rate my posts if you found them useful

  8. #8
    Join Date
    Apr 2000
    Location
    Belgium (Europe)
    Posts
    4,626

    Re: is this cast to add const guaranteed to be OK?

    Quote Originally Posted by laserlight View Post
    That doesn't make sense, but if we replace "to at the place" with "and at the place", it makes sense, so I think there's a typo there. That said, you don't need a cast at all to convert a "pointer to an object" to a "pointer to void", and you only need a static_cast to convert it back.
    "and" might make more sense. Blame it on my mother tongue where "to" makes more sense.

    I know you don't need a cast to convert to a void*. But it's not always a void* you need to get it in to. In windows you often have to cast a "pointer to something" into a DWORD (or int64) or LPARAM. which would need a cast on both ends.

    The idea remains, cast a pointer into a "generic" datatype only to cast it back into it's Original pointer type. Anything other than that may not get you what you were expecting with reinterpret_cast.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  





Click Here to Expand Forum to Full Width

Featured