Reputation: 103
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!
Upvotes: 1
Views: 926
Reputation: 151
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).
Upvotes: 4
Reputation: 327
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;
}
Upvotes: 0