21

Due to this bug in Visual Studio 2013, I need to provide my own move constructor and move assignment for a derived class. However, I don't know how to call the appropriate move functions for the base class.

Here's the code:

#include <utility>

// Base class; movable, non-copyable 
class shader
{
    public:
        virtual ~shader()
        {
            if (id_ != INVALID_SHADER_ID)
            {
                // Clean up
            }
        }

        // Move assignment
        shader& operator=(shader&& other)
        {
            // Brett Hale's comment below pointed out a resource leak here.
            // Original:
            // id_ = other.id_;
            // other.id_ = INVALID_SHADER_ID;
            // Fixed:
            std::swap( id_, other.id_ );
            return *this;
        }

        // Move constructor
        shader(shader&& other)
        {
            *this = std::move(other);
        }

    protected:
        // Construct an invalid shader.
        shader()
            : id_{INVALID_SHADER_ID}
        {}

        // Construct a valid shader
        shader( const char* path )
        {
            id_ = 1;
        }

    private:
        // shader is non-copyable
        shader(const shader&) = delete;
        shader& operator=(const shader&) = delete;

        static const int INVALID_SHADER_ID = 0;

        int id_;
        // ...other member variables.
};

// Derived class
class vertex_shader final : public shader
{
    public:
        // Construct an invalid vertex shader.
        vertex_shader()
            : shader{}
        {}

        vertex_shader( const char* path )
            : shader{path}
        {}

        // The following line works in g++, but not Visual Studio 2013 (see link at top)...
        //vertex_shader& operator=(vertex_shader&&) = default;

        // ... so I have to write my own.
        vertex_shader& operator=(vertex_shader&&)
        {
            // What goes here?
            return *this;
        }

        vertex_shader(vertex_shader&& other )
        {
            *this = std::move(other);
        }

    private:
        // vertex_shader is non-copyable
        vertex_shader(const vertex_shader&) = delete;
        vertex_shader& operator=(const vertex_shader&) = delete;
};

int main(int argc, char* argv[])
{
    vertex_shader v;

    // later on
    v = vertex_shader{ "vertex_shader.glsl" };

    return 0;
}

What should the move assignment function in the derived class look like?

x-x
  • 7,287
  • 9
  • 51
  • 78
  • 2
    Interesting post on using `*this = std::move(other)` to implement the move constructor: http://stackoverflow.com/questions/17118256/implementing-move-constructor-by-calling-move-assignment-operator – goji Oct 01 '13 at 06:37
  • @Troy, thanks for the feedback. It was that Microsoft article that had me change to that style in the first place. Another reason to ignore Microsoft I suppose. :) – x-x Oct 01 '13 at 06:46
  • It works, the other question just outlines that it negates some of the efficency benefits of move assignment. Just something to think about I guess :) – goji Oct 01 '13 at 06:55
  • 2
    Your `shader` move assignment operator may have a potential resource leak. If `id_` was a valid resource, you should release it before acquiring the `other.id_` resource. Maybe we need the [QLD] tag in this question! – Brett Hale Oct 01 '13 at 10:47

1 Answers1

47

You just need to call the base class move assignment operator:

    vertex_shader& operator=(vertex_shader&& rhs)
    {
        shader::operator=(std::move(rhs));
        return *this;
    }
goji
  • 6,911
  • 3
  • 42
  • 59
  • can the compiler accept base-class directly like `vertex_shader& operator=(shader&& rhs) { ... }` ? and is that even supported? – Top-Master Dec 18 '18 at 05:36