-1

Thanks so much in advance! I am writing my own matrix class when using C++, I used a 1-d array to store the data, and a pointer pointing to this array to manage my data. When I am trying to overload operator=, weird thing happens... the matrix printed within operator= function outputs correct thing but it outputs weird thing outside the function, and I can not think of any reason to explain the problem, below is my code:

class Matrix2d{

    public:

        float* data;
        int rows;
        int cols;

        Matrix2d(int _rows, int _cols, float num){
            rows = _rows; cols = _cols;
            data = new float [rows * cols];
            std::fill(data, data + (rows * cols), num);
            return;
        }

        ~Matrix2d(){
            delete[] data;
        }

        Matrix2d operator+(const Matrix2d& matrix){
            if (matrix.cols != this->cols || matrix.rows != this->rows){
                std::cout << "[ERROR] Matrix size does not match." << std::endl;
                throw;
            }
            Matrix2d output(this->cols, this->rows, 0);
            float* ptr = output.data;
            for (int i = 0; i < this->rows * this->cols; ++i){
                *ptr = this->data[i] + matrix.data[i];
                ++ptr;
            }
            return output;
        }

        void operator=(const Matrix2d& matrix){
            this->rows = matrix.rows; this->cols = matrix.cols;
            delete[] this->data;
            this->data = matrix.data;
            std::cout << "Output from =" << std::endl;
            std::cout << *this << std::endl;
        }


        friend std::ostream& operator<<(std::ostream& stream, const Matrix2d& matrix){
            
            stream << "Matrix2d([";
            float* ptr = matrix.data;
            for (int i = 0; i < matrix.rows; ++i){
                stream << ((i == 0)? "[":"          [");
                for (int j = 0; j < matrix.cols; ++j){
                    stream << ptr << ((j == matrix.cols - 1)? "]":",");
                    ++ptr;
                }
                if (i != matrix.rows - 1){ stream << '\n'; }
            }
            stream << "])" << std::endl;
            return stream;
        }
};

I printed the matrix inside operator= function, it is correct, but it is not outside that function:

int main(){
    
    Matrix2d A(3, 3, 1), B(3, 3, 5);
    cout << A << endl;
    cout << B << endl;
    A = A+B;
    cout << A << endl;
    return 0;

}

Output:

Matrix2d([[1,1,1]
          [1,1,1]
          [1,1,1]])

Matrix2d([[5,5,5]
          [5,5,5]
          [5,5,5]])

Output from =
Matrix2d([[6,6,6]
          [6,6,6]
          [6,6,6]])

Matrix2d([[9.93522e-38,0,6]
          [6,6,6]
          [6,6,6]])

I do not know where first two entries came from, the memory addresses of the last two matrices are exactly the same, and I can not understand why only two values changed, addresses:

Matrix2d([[0x159fb30,0x159fb34,0x159fb38]
          [0x159fb3c,0x159fb40,0x159fb44]
          [0x159fb48,0x159fb4c,0x159fb50]])

Matrix2d([[0x159fac0,0x159fac4,0x159fac8]
          [0x159facc,0x159fad0,0x159fad4]
          [0x159fad8,0x159fadc,0x159fae0]])

Output from =
Matrix2d([[0x159bb20,0x159bb24,0x159bb28]
          [0x159bb2c,0x159bb30,0x159bb34]
          [0x159bb38,0x159bb3c,0x159bb40]])

Matrix2d([[0x159bb20,0x159bb24,0x159bb28]
          [0x159bb2c,0x159bb30,0x159bb34]
          [0x159bb38,0x159bb3c,0x159bb40]])

How can values at same addresses be changed simply after I went out of the function? Also if I use something like:

C=A+B; A=C;

this will give the correct thing.

Zongyue Lu
  • 47
  • 6

1 Answers1

0

In your operator=, you assigned this->data = matrix.data. When the temporary instance is destroyed, matrix.data is freed and you're now referencing freed memory.

With C=A+B, however, there's a special case called copy elision, where the resulting instance is created at C and no operator= is actually called.

Since you defined the operator with const Matrix2d&, it's a copy assigner. Reallocate another chunk and copy data should be correct.

Alternatively, create a move assigner (accepts a single argument of Matrix2d&&) and set matrix.data to nullptr after moving.

iBug
  • 35,554
  • 7
  • 89
  • 134
  • Thanks so much for your patience! But why only two values changed instead of all of them being freed memory. – Zongyue Lu Jun 05 '21 at 08:33
  • @ZongyueLu It doesn't make sense in asking why, when the code is totally broken due to the lack of a user-defined copy constructor and a buggy assignment operator. You need to fix those first, and then attempt to implement `operator +`. – PaulMcKenzie Jun 05 '21 at 08:35