Updated on 2019/07/03 (long overdue) after this thread on reddit.

My post from last week contained a HUGE bug: optionals don’t move their content lose their value on move, so destructor on moved handles is still invoked (thus causing multiple destructions).

In the brief discussion on reddit, I proposed to create a optional with move semantics as solution (basically, an hybrid between a unique_ptr and an optional).

To fit it in the grand scheme, I needed to make it both copyable and non-copiable (see previous post for the implementation of Copyable<bool COPYABLE>).

template<class T, bool COPYABLE>
class moveable_optional : Copyable<COPYABLE> {
    T value_;
    bool valid_;
public:
    moveable_optional() : valid_(false) {}
    explicit moveable_optional(const T &t) : value_(t), valid_(true) {}
    moveable_optional(const moveable_optional&) = default;
    moveable_optional(moveable_optional&& other) : value_(std::move(other.value_)), valid_(other.valid_)
    {
        other.valid_ = false;
    }
    moveable_optional& operator = (const moveable_optional&) = default;
    moveable_optional& operator = (moveable_optional &&other) {
        value_ = std::move(other.value_);
        valid_ = other.valid_;
        other.valid_ = false;
        return *this;
    }

    ~moveable_optional() = default;

    bool has_value() const { return valid_; }

    const T &value() const { return value_; }
};

This solution works, and I don’t need to inherit BaseHandle from Copyable<COPYABLE> anymore (the mere problem of the moveable_optional member do the same trick). You can find the full code here, I left in the comments the original code in lines 126 and 130, as well as a test case to see it explode when accessing a function from an object that has been moved away, in line 194).

I then realized I could have modified the BaseHandle alone, keeping the optional where it was, and just handling correctly the move constructor and assignment (and defaulting the copy constructor and assignment).

Here’s the shorter, and probably more elegant solution. Just adding these four members to BaseHandle (and maintain inheritance from Copyable<COPYABLE>):

BaseHandle(const BaseHandle &) = default;

BaseHandle& operator = (const BaseHandle &) = default;

BaseHandle(BaseHandle &&other) : _handle(std::exchange(other._handle, {})) {}

BaseHandle& operator = (BaseHandle &&other) {
    _handle = std::exchange(other._handle, {});
    return *this;
}

You can see it in action here.

Leave a Reply

This site uses Akismet to reduce spam. Learn how your comment data is processed.