2

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:

  • densematrix.h
#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
  • densematrix.cpp
#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;
}
  • matrix_trial.cpp
#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!

Arda Aytekin
  • 1,231
  • 14
  • 24

1 Answers1

2

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
    {
        // ...
    }
Community
  • 1
  • 1
Praetorian
  • 106,671
  • 19
  • 240
  • 328
  • Wow! Thank you for spending your time to give me such a well explained answer combined with a live demo. Also, thanks for showing me the trick of using addresses in debug messages. Could you please elaborate more on using `std::move` in the initializer list? In what situations does that make a difference? And why would I be bothered by `vector::resize`s behavior? Is it because I will most likely be overwriting the initialized values in multiplication, and thus, I will be having an overhead of element initialization for nothing? – Arda Aytekin Apr 23 '15 at 14:27
  • @ArdaAytekin `std::move` would make a difference in cases where the data members are types that would benefit from moving. For instance, if made `values` an `std::vector` then your current implementation copies `rhs.values` while `values( std::move(rhs.values) )` would move construct it. You've understood the `vector::resize` drawback correctly, it's not a big deal in most cases, but it does add overhead. – Praetorian Apr 23 '15 at 14:34
  • Thank you again. I have a final question. When I added `DenseMatrix F = std::move( A*B )` to my code, I have observed that the `std::move` resulted in the compiler's not optimizing the code --- that is, the compiler could not generate the `A*B` in place. So, if I have understood correctly, I should design the move operations correctly (if I am using pointers) and should not interfere with the compiler's job by using `std::move`. But I should use that in my move ctor's initializer. And what if I am using `std::vector` or `std::unique_ptr`? Do I need to write move operations at all? – Arda Aytekin Apr 23 '15 at 14:43
  • @Arda `std::move( A*B )` is the same as `static_cast(A*B)`. The cast prevents the compiler from performing return value optimization, so it results in an additional move construction. If you have an expression that yields a prvalue/xvalue, then don't `move` it, as that causes the behavior you observed. `unique_ptr` is not copyable, so you must move when transferring ownership. With `vector` it depends on what your goal is, do you want the original copy as well or not? – Praetorian Apr 23 '15 at 16:29
  • Due to the space limitations in comments, I will modify my question to reflect what I have learnt through your suggestions and some reading. Could you please comment on the latest version of my question? – Arda Aytekin Apr 23 '15 at 17:15
  • Thank you, for the last time, for this question --- I guess I have understood everything. You're right; by using the containers, the code not only has (well, will have) been significantly shrunk, but also has become much easier to manage and less error-prone in terms of leakages. Best, – Arda Aytekin Apr 23 '15 at 22:31