Copy constructor required for proper execution of operator+

[2beagles]

So, I have not written c++ in quite a while and am trying to dig back in to it. I am running in to a weird situation and I am hoping someone might be able to explain it.

I have a class representing a three dimensional matrix of integers in which I am trying to implement the operator+ for addition of an integer to all elements of the matrix. The class signature looks like this,

class Matrix
{
    public:
        Matrix();
 
        Matrix(const Matrix& _object);
        Matrix(const unsigned int _iMax, const unsigned int _jMax, const unsigned int _kMax);
 
        ~Matrix();
 
        Matrix& operator=(const Matrix& rhs);
 
        Matrix& operator=(const int& rhs);
 
        Matrix operator+(const int& rhs) const;
};
 

When I first implemented this class I did not include the copy constructor. Without the copy constructor, if I ran the simple test code below I consistently saw an error indicating a memory violation. What was happening is when the object mat3 was destroyed the memory used internally to store the integer matrix had already been cleaned up.

int main(int argc, char* argv[])
{
    Matrix mat(5, 5, 5);
    mat = 0;
 
    Matrix mat3;
    mat3 = mat + 1;
 
    return 0;
}
 

I managed to track this down in the debugger to determine that the temporary object returned by operator+(int) actually used the same address for storage of the internal array of data that the object mat3 did. So, when the temporary object was cleaned up it deallocated the memory.

On a whim, I added the copy constructor to see if the behavior was different. Sure enough, when I define the copy constructor the compiler is converting the line,

mat3 = mat + 1
 

to

mat3.operator=(new Matrix(mat.operator+(1)))
 

thus inserting the call to the constructor. In this case everything works with no problems at all.

What I am wondering is if this should be required, or if I have a problem in the implementation of my + and = operators. I really dislike having yet another class instantiation and copy here since the internal matrix could be big. Hate to use the memory as well as have to copy the data.

My operators + and = are implemented as shown below,

Matrix& Matrix::operator=(const Matrix& rhs)
{
    iMax = rhs.iMax;
    jMax = rhs.jMax;
    kMax = rhs.kMax;
 
    if (data!= 0)
    {
        delete [] data;
    }
 
    data = new int[iMax * jMax * kMax];
    for (unsigned int ijk = 0; ijk < iMax * jMax * kMax; ijk++)
    {
        data[ijk] = rhs.data[ijk];
    }
 
    return *this;
}
 
 
Matrix Matrix::operator+(const int& rhs) const
{
    Matrix newMat(iMax, jMax, kMax);
 
    for (unsigned int ijk = 0; ijk < iMax * jMax * kMax; ijk++)
    {
        newMat.data[ijk] = data[ijk] + rhs;
    }
 
    return newMat;
}
 

I appreciate any help on this.

[Banfa]

I assume that data is a member of your class that holds the pointer to the matrix data which you appear to allocate from the heap (using new).

In this case it is almost guaranteed that you will require an explicitly written copy constructor and assignment operator.

The default copy constructor does a shallow copy. That is it just copies the values of the members from one object to the other. For an object with allocated memory (like yours) that means that both objects end up pointing to the same allocated data and when one object is deleted and releases the memory it leaves the other object with an invalid pointer. This is likely to cause memory errors or segmentation faults, which is what you are seeing.

In this case (and what your assignment operator does) you require a deep copy. A deep copy is one that has some knowledge of what it is copying and when it comes accross an allocated buffer rather than copying the pointer it allocates a new buffer for the receiving object and copies the data in the buffer not the pointer to the buffer.

As far as I can tell C++ does tend to do these hiden allocations and secret object instatiations. I can't say I like it but that's how it is. Sometimes it is possible to write your code to reduce them, for instance if you are using the increment operator (++) on an object (rather than a built in type) then pre-increment tends to be more efficient than post-increment because post-increment requires that a copy of the current object is taken and returned where as pre-increment can perform the entire operation in the current object and return it. Obviously this is true for decrement.

In place operations also tend to be more efficient due to the reduction of object copying, e.g. a += 1; is more efficient than a = a + 1;

[weaknessforcats]

The default copy constructor does a shallow copy. That is it just copies the values of the members from one object to the other.

Not quite true. The default copy constructor does memberwise copy. That means a copy constructor call for every member that is a struct/class. Only pointers and the fundamental types have their values copied.

That means a deep copy where copy constructors are called.

I appreciate any help on this.

Your assgnment operators should check for no self-assignment. Otherwise, you will delete the data you are about to assign. You need a check like:

[code=cpp]
Matrix& Matrix::operato r=(const Matrix& rhs)
{
if (this == &rhs) return *this; //no self-assignment

iMax = rhs.iMax;
jMax = rhs.jMax;
kMax = rhs.kMax;

if (data!= 0)
{
delete [] data;
}
etc....
}

[2beagles]

Banfa,

Thanks for clarifying this. Indeed data is a member of the Matrix class that is a pointer to an array of ints. It is allocated/deallocated using new int[] and delete []. The shallow copy of this pointer clearly explains why I was seeing the behavior I was seeing. What I do not understand is why the compiler feels the need to call the copy constructor to create a new object here rather than just pass the temporary object that is returned from operator+ to the operator= function.

Clearly I am going to have to be pretty careful with how I implement and use the operators for the matrix class to minimize the data allocation and copies. I need to lean hard on the in place operators I guess. Learn something new every day!!

Thanks for the information.

[2beagles]

Your assgnment operators should check for no self-assignment. Otherwise, you will delete the data you are about to assign. You need a check like:

Thanks! I had not thought of that. I'd probably have spent a few evenings with the debugger trying to track that down...

[weaknessforcats]

Look at this code:

Matrix Matrix::operato r+(const int& rhs) const
{
Matrix newMat(iMax, jMax, kMax);

for (unsigned int ijk = 0; ijk < iMax * jMax * kMax; ijk++)
{
newMat.data[ijk] = data[ijk] + rhs;
}

return newMat;
}

Here you see newMat created as a local variable in the function. When you return newMat, you cannot return the local variable since it will be destroyed. So, the compiler calls your copy constructor to make an object to return.

This is one reason why copy constructors are so important.

[2beagles]

Look at this code:


Here you see newMat created as a local variable in the function. When you return newMat, you cannot return the local variable since it will be destroyed. So, the compiler calls your copy constructor to make an object to return.

This is one reason why copy constructors are so important.

Yeah, I ran across a discussion of this today in an article on optimization. It is pretty obvious now why the call to the copy constructor is required. Unfortunate, but required.

The article I was looking at is, http://www.tantalon.com/pete/cppopt/main.htm. This is a fairly nice article by the way. The discussion of this I was looking at is in the section Final Optimization, "Avoid temporary objects: the return value optimization."

Thanks for the help.