CODEWITHSUNDEEP
c++c++11move-semantics
Arda Aytekin
Arda Aytekin

Reputation: 1301

move semantics: how best to understand/use them

I am having problems in move semantics of C++11. I am using gcc 4.9.2 20150304 (prerelease) with the -std=c++11 switch, but I am having problems in move constructor's not being invoked.

I have the following source files:

#ifndef DENSEMATRIX_H_
#define DENSEMATRIX_H_

#include <cstddef>
#include <iostream>
#include <utility>

class DenseMatrix {
    private:
        size_t m_ = 0, n_ = 0;
        double *values = nullptr;
    public:
        /* ctor */
        DenseMatrix( size_t, size_t );
        /* copy ctor */
        DenseMatrix( const DenseMatrix& rhs );
        /* move ctor */
        DenseMatrix( DenseMatrix&& rhs ) noexcept;
        /* copy assignment */
        const DenseMatrix& operator=( const DenseMatrix& rhs );
        /* move assignment */
        const DenseMatrix& operator=( DenseMatrix&& rhs ) noexcept;
        /* matrix multiplication */
        DenseMatrix operator*( const DenseMatrix& rhs ) const;
        /* dtor */
        ~DenseMatrix();
};

#endif

#include "densematrix.h"

/* ctor */
DenseMatrix::DenseMatrix( size_t m, size_t n ) :
    m_( m ), n_( n ) {
    std::cout << "ctor with two arguments called." << std::endl;
    if ( m_*n_ > 0 )
        values = new double[ m_*n_ ];
}

/* copy ctor */
DenseMatrix::DenseMatrix( const DenseMatrix& rhs ) :
    m_( rhs.m_ ), n_( rhs.n_ ) {
    std::cout << "copy ctor called." << std::endl;
    if ( m_*n_ > 0 ) {
        values = new double[ m_*n_ ];

        std::copy( rhs.values, rhs.values + m_*n_, values);
    }
}

/* move ctor */
DenseMatrix::DenseMatrix( DenseMatrix&& rhs ) noexcept :
    m_( rhs.m_ ), n_( rhs.n_ ), values( rhs.values ) {
    std::cout << "move ctor called." << std::endl;
    rhs.values = nullptr;
}

/* copy assignment */
const DenseMatrix& DenseMatrix::operator=( const DenseMatrix& rhs ) {
    std::cout << "copy assignment called." << std::endl;

    if ( this != &rhs ) {
        if ( m_*n_ != rhs.m_*rhs.n_ ) {
            delete[] values;
            values = new double[ rhs.m_*rhs.n_ ];
        }

        m_ = rhs.m_;
        n_ = rhs.n_;
        std::copy( rhs.values, rhs.values + m_*n_, values);
    }

    return *this;
}

/* move assignment */
const DenseMatrix& DenseMatrix::operator=( DenseMatrix&& rhs ) noexcept {
    std::cout << "move assignment called." << std::endl;

    m_ = rhs.m_;
    n_ = rhs.n_;
    delete[] values;
    values = rhs.values;
    rhs.values = nullptr;

    return *this;
}

/* matrix multiplication */
DenseMatrix DenseMatrix::operator*( const DenseMatrix& rhs ) const {
    return DenseMatrix( this->m_, rhs.n_ );
}

/* dtor */
DenseMatrix::~DenseMatrix() {
    std::cout << "dtor called." << std::endl;
    delete[] values;
}

#include <iostream>
#include <utility>
#include "densematrix.h"

int main( int argc, char* argv[] ) {
    /* ctor */
    DenseMatrix A( 5, 10 );
    /* ctor */
    DenseMatrix B( 10, argc );
    /* copy ctor */
    DenseMatrix C = A;
    /* copy assignment */
    C = B;
    /* move ctor */
    DenseMatrix D( A*B );
    DenseMatrix E = DenseMatrix( 100, 200 );
    /* move assignment */
    D = C*D;

    return 0;
}

If I compile my program without the -fno-elide-constructors switch, I obtain the following output:

ctor with two arguments called.
ctor with two arguments called.
copy ctor called.
copy assignment called.
ctor with two arguments called.
ctor with two arguments called.
ctor with two arguments called.
move assignment called.
dtor called.
dtor called.
dtor called.
dtor called.
dtor called.
dtor called.

If, on the other hand, I compile using the -fno-elide-constructors switch, I get the following output:

ctor with two arguments called.
ctor with two arguments called.
copy ctor called.
copy assignment called.
ctor with two arguments called.
move ctor called.
dtor called.
move ctor called.
dtor called.
ctor with two arguments called.
move ctor called.
dtor called.
ctor with two arguments called.
move ctor called.
dtor called.
move assignment called.
dtor called.
dtor called.
dtor called.
dtor called.
dtor called.
dtor called.

In the second output, I am confused by the move ctor's. First, two move ctor's are called in creating D, whereas only one move ctor is called in creating E. Second, in assigning the result of the multiplication to D, another move ctor is called before the move assignment operator.

Could someone explain what is happening and/or if I have designed my class correctly? What should I do in this situation? Shall I compile the program in a normal way (without the switch) and use std::move() whenever I would like to ensure move semantics, or what?

Thank you!

Edit due to the comments of @Praetorian:
The final version of my densematrix.h implementation is then as follows:

#ifndef DENSEMATRIX_H_
#define DENSEMATRIX_H_

#include <cstddef>
#include <stdexcept>
#include <utility>
#include <vector>

class DenseMatrix {
    private:
        size_t m_ = 0,  /* number of rows */
               n_ = 0;  /* number of columns */
        /* values of the matrix in column major order */
        std::vector< double > values_;
    public:
        /* ctor */
        DenseMatrix( size_t m, size_t n,
            std::vector< double > values = std::vector< double >() ) :
            m_( m ),
            n_( n ),
            values_( values )
        {
            if ( m_*n_ == 0 )
                throw std::domain_error( "One of the matrix dimensions is zero!" );
            else if ( m_*n_ != values.size() && values_.size() != 0 )
                throw std::domain_error( "Matrix dimensions do not match with the number of elements" );
        }
        /* copy ctor */
        DenseMatrix( const DenseMatrix& rhs ) :
            m_( rhs.m_ ),
            n_( rhs.n_ ),
            values_( rhs.values_ ) { }
        /* move ctor */
        DenseMatrix( DenseMatrix&& rhs ) noexcept :
            m_( std::move( rhs.m_ ) ),
            n_( std::move( rhs.n_ ) ),
            values_( std::move( rhs.values_ ) ) { }
        /* copy assignment */
        const DenseMatrix& operator=( const DenseMatrix& rhs ) {
            if ( this != &rhs ) {
                m_ = rhs.m_;
                n_ = rhs.n_;
                /* trust std::vector<>'s optimized implementation, i.e.,
                 * no need to check the vectors' sizes to decrease the
                 * heap access */
                values_ = rhs.values_;
            }

            return *this;
        }
        /* move assignment */
        const DenseMatrix& operator=( DenseMatrix&& rhs ) noexcept {
            m_ = std::move( rhs.m_ );
            n_ = std::move( rhs.n_ );
            values_ = std::move( rhs.values_ );

            return *this;
        }
        /* matrix multiplication */
        DenseMatrix operator*( const DenseMatrix& rhs ) const {
            /* do dimension checking */
            DenseMatrix temp( this->m_, rhs.n_ );

            /* do the necessary calculations */

            return temp;
        }
        /* dtor not needed in this case */
};

#endif

Now, hopefully, I have implemented the semantics correctly. What do you think? Now, I am depending on the container class I am using when it comes to copying and/or moving vectors of different sizes.

Thank you again for your help and comments!

Upvotes: 2

Views: 260

Answers (1)

Praetorian
Praetorian

Reputation: 109119

The two move operations whenever you use operator* are because you've asked the compiler not to perform copy/move elision. This forces it to construct a temporary inside operator* (2 argument constructor call), then move this temporary into the return value (move constructor call) and finally move the return value to the destination object (move constructor/move assignment calls in your example).

I've made your example a bit noisier by also printing the addresses of the objects involved in the print statements. For instance

std::cout << "move ctor called. " << this << std::endl;

Live demo

Let's look at what happens here:

/* move ctor */
DenseMatrix D( A*B );
std::cout << "&D " << &D << std::endl;

The relevant output statements are

ctor with two arguments called. 0x7fff7c2cb1d0  <-- temporary created in the 
                                                    return statement of operator*
move ctor called. 0x7fff7c2cb2b0                <-- temporary moved into the return value
dtor called. 0x7fff7c2cb1d0                     <-- temporary from step 1 destroyed
move ctor called. 0x7fff7c2cb230                <-- return value moved into D
dtor called. 0x7fff7c2cb2b0                     <-- return value destroyed
&D 0x7fff7c2cb230                               <-- address of D

The output of D = C*D; is identical except the second move construction is replaced by move assignment.

You haven't done anything wrong in your class implementation, just don't use -fno-elide-constructors to compile the code (why would you?).

This doesn't make a difference in your case, but typically in the move constructor the source object's data members are std::moved in the mem-initializer

DenseMatrix::DenseMatrix( DenseMatrix&& rhs ) noexcept :
    m_( std::move(rhs.m_) ), n_( std::move(rhs.n_) ), values( std::move(rhs.values) ) {
//..
}

Finally, you may want to change values from double* to std::vector<double> and avoid all the new and delete calls. The only caveat to doing that is that the vector will value initialize the new elements it adds whenever you vector::resize, but there are workarounds for that too.

Update to address the code added in your last edit.

Not only is the destructor definition no longer necessary, but you don't need any of the copy/move constructors/assignment operators either. The compiler will implicitly declare these for you, and they do exactly what your handwritten versions do. So all you need in your class are the constructor definition and that for operator*. Therein lies the true beauty of not managing memory manually.

I'd make a couple of changes to the constructor definition

    DenseMatrix( size_t m, size_t n,
        std::vector< double > values = {} ) : // <-- use list initialization, 
                                              //     no need to repeat type name
        m_( m ),
        n_( n ),
        values_( std::move(values) )          // <-- move the vector instead of copying
    {
        // ...
    }

Upvotes: 2

Related Questions