C++ - Making operator overload chainable

Question

So I am trying to overload two operators for my "Matrix" class (+ and +=). I am attempting to make + chainable and += non-chainable:

template <class T>
Matrix<T>& Matrix<T>::operator+=(const Matrix& M) 
{
  if (this->m_capacity != M.capacity()) 
  {
    throw std::out_of_range("Input is invalid");
  }

  for (unsigned int i = 0; i < M.rows(); i++) 
  {
    for (unsigned int j = 0; j < M.cols(); j++) 
    {
      this->m_vec[i + m_cols * j] += M(i, j);
    }
  }
  return *this;
}


template <class T>
Matrix<T> operator+(Matrix<T> M1, Matrix<T>& M2) 
{
  if (M1.capacity() != M2.capacity()) 
  {
    throw std::out_of_range("Input is invalid");
  }

  return M1 += M2;
}

It compiles just fine, no issues there, but when I try to do a unit test on this, the entire test program just crashes when attempting to chain the + operator.

Example:

TEST(add, Matrix)
{
  Matrix<int> M1 = Matrix<int>(2, 3);
  Matrix<int> M2 = { 1, 2, 3, 4 };
  Matrix<int> M3 = M2;
  Matrix<int> M4 = { 2, 4, 6, 8 };

  ASSERT_THROW(M1 + M2, std::out_of_range);
  ASSERT_EQ((M2 + M3) == M4, true);

  M2 += M3;
  M2 = M4 + M4 + M4; // As soon as this line is added, it crashes, without it, test works fine

  ASSERT_EQ(M2 == M4, true);
}

Any ideas why it crashes? How can I rewrite my operator overloads so that ´+´ is chainable (and += isn't)?

EDIT: Here is my = operator (upon request)

template <class T>
void Matrix<T>::operator=(Matrix & M){
  T*temp = new T[M.m_capacity];
  for(unsigned int  j = 0; j < M.m_capacity; j++){
    temp[j] = M.m_vec[j];
  }
  delete[] this -> m_vec;
  size_t rows = M.get_m_rows();
  size_t cols = M.get_m_cols();
  this -> m_rows = rows;
  this -> m_cols = cols;
  this -> m_vec = new T [rows*cols];
  this -> m_capacity = rows*cols;
  for(size_t i = 0;i < rows;i++){
    for(size_t j = 0;j < cols;j++){
      this -> m_vec[i*cols+j] = temp[i*cols +j];
    }
  }
  delete [] temp;
}

EDIT2: Added more context (upon request, header, constructors etc.)

Header:

template <class T>
class Matrix {
public:
   // constructor
   Matrix(unsigned int n);
   Matrix(unsigned int n, unsigned int m);
   Matrix();
   Matrix(const T n);
   Matrix(Matrix &obj);
   ~Matrix();
   Matrix(Matrix &&obj);
   Matrix(std::initializer_list<T> l);

   // operators
   void operator=(Matrix & obj);
   T& operator()(unsigned int row, unsigned int col);
   Matrix& operator=( Matrix &&obj);
   Matrix& operator+=(const Matrix& M)
   void operator+=(const T number);
   void operator-=(const T number);
   void operator-=(Matrix &obj);
   void operator*=(const T number);
   void operator*=(Matrix &obj);
   bool operator==(Matrix & rhs);

private:
   std::size_t m_rows;
   std::size_t m_cols;
   std::size_t m_capacity;
   T * m_vec;
};

Copy constructor:

template <class T>
Matrix<T>::Matrix(Matrix &obj){
  size_t rows = obj.get_m_rows();
  size_t cols = obj.get_m_cols();
  this -> m_rows = rows;
  this -> m_cols = cols;
  this -> m_vec = new T [rows*cols];
  this -> m_capacity = rows*cols;
  for(size_t i = 0;i < rows;i++){
    for(size_t j = 0;j < cols;j++){
      this -> m_vec[i*cols+j] = obj(i,j);
    }
  }
}

Destructor:

template  <class T>
Matrix<T>::~Matrix(){
  delete [] m_vec;
}

Move constructor (possibly broken)

template <class T>
Matrix<T>::Matrix(Matrix &&obj){
  size_t rows = obj.get_m_rows();
  size_t cols = obj.get_m_cols();
  this -> m_rows = rows;
  this -> m_cols = cols;
  this -> m_vec = new T [rows*cols];
  this -> m_capacity = rows*cols;
  m_vec = nullptr;
}

Move assignment (possibly broken)

template <class T>
Matrix<T>& Matrix<T>::operator=(Matrix &&obj){
  if (this !=&obj)
  {
    delete [] m_vec;
    obj.m_rows = 0;
    obj.m_cols = 0;
    obj.m_capacity = 0;
    obj.m_vec = nullptr;
  }
  return *this;
  }

Answer 1

Both your move constructor and your move assignment operator do not implement the correct semantics (and it seems you are aware of that), leading to UB somewhere later. (I didn't bother checking exactly where.)

I guess you were assuming that you don't actually call these operators, but that is wrong.

You are calling the move assignment operator at the = sign of

M2 = M4 + M4 + M4;

because the right-hand side is a prvalue (operator+ returns by-value) which can bind to a rvalue reference.

(Before C++17) You are calling the (possibly elided) move constructor at the second + in the same line to construct the first parameter of operator+, because the first + results in a prvalue.

If you intend to implement the move operations later and you are fine with using the copy implementations instead for the time being, then don't declare the move operations in your class at all. Then the compiler will choose your copy implementations instead.

Additionally, the copy constructor and the copy assignment operator should always take a const (lvalue) reference as parameter, not a non-const reference.

Answer 2

I’m not sure, but since your += operator is destructive, in that it doesn’t return a separate matrix but instead overwrites the previous one, adding m4 to itself may cause weird bugs as the “this” that is getting added and return is constantly in flux. There is no reason why the operator would be chainable as the compiler takes the chain into sequences of binary operations anyway, so the problem is most likely with your structure of the += overload. Also it would make more sense to add within the + overload and define x += x as x = x + x rather than the other way round.