1

I’m writing a simple Matrix class, and I’ve defined, among others, an operator+ overloading and a move assignment. It looks like something happens when the two of them interact, but I can’t find where I am wrong. Here’s my code (I’ve deleted everything superfluous, and just left what is needed to show the bug). The problematic line is at the end, in the main:

#include <iostream>
#define DEF -1

using namespace std;
//----- Matrix -----//
class Matrix{
private:
    float **matrixpp;
    int dim_r;
    int dim_c;

public:
    Matrix(int d_r = DEF, int d_c = 0);   
    Matrix(const Matrix&);
    Matrix(Matrix&&);
    Matrix& operator=(Matrix&&);
    ~Matrix();

    Matrix operator+(const Matrix&);

    void print();
    void fill();

};

//----- Matrix -----//
Matrix::Matrix(int d_r, int d_c){

    if(d_r == DEF){
        do{
            cout << "number of rows: ";
            cin >> dim_r;
            if(dim_r <= 0){
                cout << "ERROR" << endl;
            }
        }
        while(dim_r <= 0);

        do{
            cout << "Number of columns: ";
            cin >> dim_c;
            if(dim_c <= 0){
                cout << "ERROR" << endl;
            }
        }
        while(dim_c <= 0);


    }
    else{
        dim_r = d_r;
        dim_c = d_c;
    }

    matrixpp = new float*[dim_r];

    for(int i = 0; i < dim_r; i++){
        matrixpp[i] = new float[dim_c];
    }

    for(int r = 0; r < dim_r; r++){
        for(int c = 0; c < dim_c; c++){
            matrixpp[r][c] = 0;
        }
    }
}

Matrix::Matrix(const Matrix& tocopy)
:matrixpp(tocopy.matrixpp), dim_r(tocopy.dim_r), dim_c(tocopy.dim_c)
{

    matrixpp = new float*[dim_r];

    for(int i = 0; i < dim_r; i++){
        matrixpp[i] = new float[dim_c];
    }

    for(int r = 0; r < dim_r; r++){
        for(int c = 0; c < dim_c; c++){
            matrixpp[r][c] = tocopy.matrixpp[r][c];
        }
    }
}

Matrix::Matrix(Matrix&& tomove)
:matrixpp(tomove.matrixpp), dim_r(tomove.dim_r), dim_c(tomove.dim_c)
{
    tomove.matrixpp = nullptr;

}

Matrix& Matrix::operator=(Matrix&& tomove){

    cout << "--- MA ---" << endl;
    matrixpp = tomove.matrixpp;
    dim_r = tomove.dim_r;
    dim_c = tomove.dim_c;

    tomove.matrixpp = nullptr;
}

Matrix::~Matrix(){

    if(matrixpp != nullptr){
        for(int i = 0; i < dim_r; i++){
            delete[] matrixpp[i];
        }
        delete[] matrixpp;
    }
}

Matrix Matrix::operator+(const Matrix& m){

    if(this->dim_r == m.dim_r && this->dim_c == m.dim_c){
        Matrix new_m(m.dim_r, m.dim_c);

        for(int r = 0; r < new_m.dim_r; r++){
            for(int c = 0; c < new_m.dim_c; c++){
                new_m.matrixpp[r][c] = this->matrixpp[r][c] + m.matrixpp[r][c];
            }
        }

        return new_m;
    }
    else{
        cout << "ERROR" << endl;
    }
}

void Matrix::print(){
    int temp;

    for(int r = 0; r < dim_r; r++){
        for(int c = 0; c < dim_c; c++){
            cout << matrixpp[r][c] << " ";

        }
        cout << endl;
    }
    cout << endl;
}

void Matrix::fill(){
    float temp;

    for(int r = 0; r < dim_r; r++){
        for(int c = 0; c < dim_c; c++){
            cout << "new value: " << endl;
            this->print();
            cin >> temp;
            matrixpp[r][c] = temp;
            system("clear");
        }
    }
}

//-------- Main -------//
int main(){

    Matrix m0;
    m0.fill();
    Matrix m1(0);

    m1 = m0+m0; // problematic line

    //m1.print();

}

It gives me the error when the move assignment is called to move the result of m0+m0 in m1.

If I compile my code with g++ on a Mac, the error given is just “Illegal instruction: 4”, nothing more, nothing less. If I do it on a Linux, the error given is:

In member function ‘Matrix Matrix::operator+(const Matrix&)’: p.cpp:90:16: error: use of deleted function ‘constexpr Matrix::Matrix(const Matrix&)’ return new_m; ^~~~~ p.cpp:9:7: note: ‘constexpr Matrix::Matrix(const Matrix&)’ is implicitly declared as deleted because ‘Matrix’ declares a move constructor or move assignment operator class Matrix{ ^~~~~~

Thank you in advance!

10
  • Your second do-while loop inside Matrix::Matrix contains a bug. You input dim_c and check for dim_r <= 0 (notice c vs r). It won't probably fix your issue, but that's a bug nonetheless. That's the risk of copy-pasting the code. Besides, what happens if I do Matrix m(2, -2);? Commented Oct 17, 2018 at 10:42
  • Please copy and paste full error printed by the compiler. Commented Oct 17, 2018 at 10:43
  • Edited, and thanks for the bug Commented Oct 17, 2018 at 10:46
  • 4
    The rule of three/five/zero Commented Oct 17, 2018 at 10:47
  • 1
    Functions that have a non-void return type must return something on every path. Your compiler should be warning you about this. Commented Oct 17, 2018 at 10:49

2 Answers 2

4

The problem is, when using pre-C++17 standard, there is no copy elision guarantee, so returning a value from a function follows copy initialization semantics, which requires move-or-copy constructor to be defined. Defining your own move assignment operator disables automatic constructor generation (Class copy CTOR generation rules). In fact the compiler provided constructor is defined, but deleted. To solve this problem, you have to define a move or copy constructor, or both of them according to the rule of five (Rule of three/five/zero).

Also, I suspect that the original problem (illegal instruction) is caused by a lot of undefined behaviours in your code (e.g. no return statement in your assignment operator, and no return in operator+ else branch).

Sign up to request clarification or add additional context in comments.

8 Comments

As I said, here is a semplified version of my whole program. There, I had defined a move and copy and everything constructor and assignment, but the problem still remained.
Well. the error message says something else. ¯\_(ツ)_/¯
I still get the “Illegal instruction: 4” error message. Anyway, about the elses without a return, I get a warning when I compile.
These warnings are really important, you should always return something in functions with non-void return type.
Yes I know that. But right now I was just practicing with move constructor and move assignment. Do you think that can be the cause of the problem? Otherwise, do you have any idea of what it could be that causes the bug?
|
0

the main problem is your operator = didnt have any return.

overload the operator + like this:

Matrix operator+(const Matrix &A ,const Matrix &m)
{
if(A->dim_r == m.dim_r && A->dim_c == m.dim_c){
    Matrix new_m(m.dim_r, m.dim_c);

    for(int r = 0; r < new_m.dim_r; r++){
        for(int c = 0; c < new_m.dim_c; c++){
            new_m.matrixpp[r][c] = A->matrixpp[r][c] + m.matrixpp[r][c];
        }
    }

    return new_m;
}
else{
    cout << "ERROR" << endl;
}

and make it friend of your class. and in the end of operator = add return statement like this :

Matrix& Matrix::operator=(Matrix&& tomove){
        if(this != &tomove) { 
        cout << "--- MA ---" << endl;
        matrixpp = tomove.matrixpp;
        dim_r = tomove.dim_r;
        dim_c = tomove.dim_c;

        tomove.matrixpp = nullptr;
    }
    return *this;
}

2 Comments

And it should work like this? Anyway, why isn’t working my way instead?
Why? Member operator overloads are perfectly fine when both arguments are of the same type. And this won't solve a problem with missing copy-constructor.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.