Suppose a C API provides an opaque struct with internal reference counting:

struct Opaque {
    int data;
    int refcount;
};

struct Opaque* opaque_new(int data) {
    return new Opaque {
        .data = data,
        .refcount = 1
    };
}

int opaque_data(struct Opaque* opaque) {
    return opaque->data;
}

struct Opaque* opaque_ref(struct Opaque* opaque) {
    opaque->refcount  ;
    return opaque;
}

void opaque_unref(struct Opaque* opaque) {
    opaque->refcount--;
    if (!opaque->refcount) {
        delete opaque;
    }
}

How to create a wrapper type that is copyable, movable, copy assignable and move assignable?

So far I have:

#include <algorithm>

class Wrapper {
public:
    Wrapper() : m_opaque(nullptr) {}

    explicit Wrapper(int data) : m_opaque(opaque_new(data)) {}

    Wrapper(const Wrapper&) = delete; // TODO

    Wrapper(Wrapper&& wrapper) : m_opaque(wrapper.m_opaque) {
        wrapper.m_opaque = nullptr;
    }

    Wrapper& operator=(const Wrapper&) = delete; // TODO

    Wrapper& operator=(Wrapper&& other) {
        swap(other);
        return *this;
    }

    ~Wrapper() {
        if (m_opaque) {
            opaque_unref(m_opaque);
        }
    }

    void swap(Wrapper& other) {
        std::swap(m_opaque, other.m_opaque);
    }

    int getData() const {
        if (m_opaque) {
            return opaque_data(m_opaque);
        } else {
            return 0;
        }
    }

private:
    struct Opaque* m_opaque;
};

I specifically don't want reference counting on top of reference counting, using std::shared_ptr with a custom deleter.

What's a concise way to implement the remaining methods, in particular copy assignment? Is there a better way to implement the ones I got so far?

CodePudding user response：

I'm assuming that you want a discrete copy of the Wrapper object and thus you want m_opaque to point to new data and not the same data (hence using a copy ctor/assignment). In that case you would just create a new pointer to an opaque type copying the values from the opaque type passed to the ctor/assignment-operator.

Note1: for the copy assignment, I added an if that checks that the passed wrap is not 'this' Wrapper (ie w1 = w1). If it is it just returns itself without side effects.

Note2: I also added a getRefCount() method to make it look more seemless. You can make that private/public or whatever or access it directly using ->.

eg.

class Wrapper
{
  /// Other constructors ...

  Wrapper(const Wrapper wrap)
  {
    m_opaque = new Opaque { 
                      .data = wrap.getData(), 
                      .refcount = wrap.getRefCount() 
                   };
  }

  Wrapper& operator=(const Wrapper wrap)
  {
    if (*this != wrap)
    {
      m_opaque = new Opaque { 
                        .data = wrap.getData(), 
                        .refcount = wrap.getRefCount() 
                     };
    }
    
    return *this;
  }

  int getRefCount()
  { return m_opaque->refcount; }

  /// Other methods ...
};

CodePudding user response：

Boost's intrusive_ptr was made for "internal reference counts":

#include <boost/intrusive_ptr.hpp>

// intrusive_ptr API functions
inline void intrusive_ptr_add_ref(Opaque* opaque) noexcept {
    ::opaque_ref(opaque);
}
inline void intrusive_ptr_release(Opaque* opaque) noexcept {
    ::opaque_unref(opaque);
}

struct Wrapper {
public:
    // Boost.intrusive_ptr operators are fine
    Wrapper() noexcept = default;
    Wrapper(const Wrapper&) noexcept = default;
    Wrapper(Wrapper&&) noexcept = default;
    Wrapper& operator=(const Wrapper&) noexcept = default;
    Wrapper& operator=(Wrapper&&) noexcept = default;

    // (false means don't add a refcount, since opaque_new does that)
    explicit Wrapper(int data) : m_opaque(opaque_new(data), false) {
        // if (!m_opaque) throw std::bad_alloc();
    }

    void swap(Wrapper& other) noexcept {
        m_opaque.swap(other.m_opaque);
    }
    friend void swap(Wrapper& l, Wrapper& r) noexcept {
        l.swap(r);
    }

    int getData() const {
        if (m_opaque) {
            return ::opaque_data(m_opaque.get());
        } else {
            return 0;
        }
    }
private:
    boost::intrusive_ptr<Opaque> m_opaque;
};

https://godbolt.org/z/oGEdd66Yo

You could also implement this with the "Copy-and-swap" idiom:

Wrapper(Wrapper&& other) noexcept : m_opaque(std::exchange(other.m_opaque, nullptr)) {}

Wrapper(const Wrapper& other) : m_opaque(other.m_opaque) {
    if (m_opaque) opaque_ref(m_opaque);
}

// Also works for move assign
Wrapper& operator=(Wrapper copy) noexcept {
    this->swap(copy);
    return *this;
}

Unrelated, but your C function opaque_new should not throw C exceptions (when new fails). You can use new (std::nothrow) Wrapper { ... } so that it returns nullptr instead of throwing an exception. Or you could have it abort instead of throwing.
And would it really hurt to have a nullptr check inside the functions?