Search code examples
c++pointersshared-ptr

Replacing char* with shared_ptr<char>


I have a struct as follows:

struct P
{
    enum {INT, FLOAT, BOOLEAN, STRING, ERROR} tag;
    union
    {
        int a;
        float b;
        bool c;
        const char* d;
    };
};

I'm using cereal library to serialize this and cereal does not support raw pointers. I'm replacing const char* d with const shared_ptr<char> d. I'm facing 3 issues:

  1. Converting char* to shared_ptr:

    char* x = //first element of char array
    d = shared_ptr<char> (x); // is this the right way?
    
  2. Handling assignments like:

    string s = "hello";
    d = s.c_str(); // how to convert the c_str() to shared_ptr<char>?
    
  3. From what I've read, shared_ptr seems to handle pointers very different from raw pointers. Will I be able to use this shared_ptr as a character array safely without any side effects?


Solution

  • First thing to say is that you're using a union. Unions in c++ are really hard to get right. Do you really need a union?

    If you really need a union, use boost::variant instead. It solves all the complexity for you.

    Next, we're using C++ - not C. Let's act like it. Get rid of that const char *. It's a landmine. That's why cereal does not support it. They're doing the right thing. Replace it with what it is. A std::string.

    EDIT:

    OK. You asked for it. Here is a solution using a discriminated union.

    Now, remember I said that unions are hard to get right in c++?

    I've been writing c++ almost every day for the past 15 (20?) years. I'm an avid follower of the progress of the standard, I always use the latest tools and I demand that people in my team know the language and the standard library inside out... and I am not yet sure that this solution is fully robust. I would need to spend a day writing tests to be really sure... because discriminated unions are really hard to get right.

    EDIT2:

    fixed the 'construct from const char*' bug (told you it was hard...)

    Are you sure you would rather not use boost::variant?

    No? ok then:

    #include <iostream>
    #include <string>
    
    struct error_type {};
    static constexpr error_type as_error = error_type {};
    
    struct P
    {
        enum {
            INT, FLOAT, BOOLEAN, STRING, ERROR
        } _tag;
    
        union data
        {
            data() {}
            ~data() {} // define a destructor that does nothing. We need to handle destruction cleanly in P
    
            int a;
            double b;   // use doubles - all calculation are performed using doubles anyway
            bool c = false;  // provide a default constructor
            std::string d;  // string or error
        } _data;
    
        // default constructor - we must initialised the union and the tag.
    
        P() : _tag { BOOLEAN }, _data {} {};
    
        // offer constructors in terms of the various data types we're storing. We'll need to descriminate
        // between strings and errors...
    
        P(int a) : _tag (INT) {
            _data.a = a;
        }
    
        P(double b) : _tag (FLOAT) {
            _data.b = b;
        }
    
        P(bool c) : _tag (BOOLEAN) {
            _data.c = c;
        }
    
        P(std::string s) : _tag(STRING)
        {
            new (std::addressof(_data.d)) std::string(std::move(s));
        }
    
        // provide a const char* constructor... because const char* converts to bool
        // more readily than it does to std::string (!!!)
        P(const char* s) : P(std::string(s)) {}
    
        P(std::string s, error_type) : _tag(ERROR)
        {
            new (std::addressof(_data.d)) std::string(std::move(s));
        }
    
        // destructor - we *must* handle the case where the union contains a string
        ~P() {
            destruct();
        }
    
        // copy constructor - we must initialise the union correctly
    
        P(const P& r)
        : _tag(r._tag)
        {
            copy_construct(r._data);
        }
    
        // move constructor - this will be particularly useful later...
    
        P(P&& r) noexcept
        : _tag(r._tag)
        {
            steal_construct(std::move(r._data));
        }
    
        // assignment operator in terms of constructor
        P& operator=(const P& p)
        {
            // this line can throw
            P tmp(p);
    
            // but these lines will not
            destruct();
            steal_construct(std::move(tmp._data));
            return *this;
        }
    
        // move-assignment in terms of noexcept functions. Therefore noexcept
        P& operator==(P&& r) noexcept
        {
            destruct();
            _tag = r._tag;
            steal_construct(std::move(r._data));
            return *this;
        }
    
        // don't define swap - we have a nothrow move-assignment operator and a nothrow
        // move constructor so std::swap will be optimal.
    
    private:
    
        // destruct our union, using our tag as the type switch
        void destruct() noexcept
        {
            using namespace std;
            switch (_tag) {
                case STRING:
                case ERROR:
                    _data.d.~string();
                default:
                    break;
            }
        }
    
        /// construct our union from another union based on our tag
        void steal_construct(data&& rd) noexcept
        {
            switch(_tag) {
                case INT:
                    _data.a = rd.a;
                    break;
                case FLOAT:
                    _data.b = rd.b;
                    break;
                case BOOLEAN:
                    _data.c = rd.c;
                    break;
                case STRING:
                case ERROR:
                    new (std::addressof(_data.d)) std::string(std::move(rd.d));
                    break;
            }
        }
    
        // copy the other union's data based on our tag. This can throw.
        void copy_construct(const data& rd)
        {
            switch(_tag) {
                case INT:
                    _data.a = rd.a;
                    break;
                case FLOAT:
                    _data.b = rd.b;
                    break;
                case BOOLEAN:
                    _data.c = rd.c;
                    break;
                case STRING:
                case ERROR:
                    new (std::addressof(_data.d)) std::string(rd.d);
                    break;
            }
        }
    
    public:
    
        // finally, now all that union boilerplate malarkey is dealt with, we can add some functionality...
    
        std::string report() const {
            using namespace std::string_literals;
            using std::to_string;
    
            switch (_tag)
            {
                case INT:
                    return "I am an int: "s + to_string(_data.a);
                case FLOAT:
                    return "I am a float: "s + to_string(_data.b);
                case BOOLEAN:
                    return "I am a boolean: "s + (_data.c ? "true"s : "false"s);
                case STRING:
                    return "I am a string: "s + _data.d;
                case ERROR:
                    return "I am an error: "s + _data.d;
            }
        }
    
    
    };
    
    int main()
    {
        P p;
        std::cout << "p is " << p.report() << std::endl;
    
        auto x = P("hello");
        std::cout << "x is " << x.report() << std::endl;
    
        auto y = P("goodbye", as_error);
        std::cout << "y is " << y.report() << std::endl;
    
        auto z = P(4.4);
        std::cout << "z is " << z.report() << std::endl;
    
    
        return 0;
    }
    

    expected results:

    p is I am a boolean: false
    x is I am a string: hello
    y is I am an error: goodbye
    z is I am a float: 4.400000