Pass by reference if possible, by value otherwise

Question

I would like to create an instance of my class Matrix using a transformation on another matrix in a template function.

Matrix<T> m(A.tri_lo());

The transformation, here tri_lo() returns a new value, so here my code throws an error :

error C2662: 'Matrix<long double> Matrix<long double>::tri_lo(bool)' : cannot convert a 'this' pointer from 'const Matrix<long double>' to 'Matrix<long double> &'

I tried overloading the constructor for pass-by-value but I couldn't get it to work. Here are my constructors :

Matrix() : data{ {T{}} } {}; // Implemented
Matrix(std::vector<std::vector<T>> _data) : data{ _data } {}; // Implemented
Matrix(unsigned int const lines, unsigned int const cols) { // Implemented
    for (unsigned int i = 0; i < lines; i++) { this->data.push_back(std::vector<T>(cols, T())); }
};
template<class T2> Matrix(Matrix<T2> const& other) : data{ other.data } {}; // Implemented
template<class T2> Matrix(Matrix<T2> const other) : data{ other.data } {} // Implemented

Where am I going wrong ?

EDIT : here is the context.

template<class T>
template<class T2>
auto Matrix<T>::operator-(Matrix<T2> const& other) {
    assert(this->lines() == other.lines());
    assert(this->cols() == other.cols());

    decltype(std::declval<T>() - std::declval<T2>()) T3;

    Matrix<T3> res(this->lines(), this->cols());

    for (unsigned int const i = 0; i < this->lines(); i++) {
        for (unsigned int const j = 0; j < this->cols(); i++) {
            res[i][j] -= other[i][j];
        }
    }

    return res;
}

Here is the full pastebin. Feel free to include a small code review if needed !

Answer 1

Main issues

There are a lot of issues with your code that Visual Studio didn't catch, but that still break the code.

For example, on lines 86 and 87 of your pastebin file:

decltype (std::declval<T>()*std::declval<T2>()) T3;
Matrix<T3> result = Matrix<T3>::gen_full(this->lines(), other.cols());

You declare a variable called T3, and then try to use it as a template parameter for Matrix. What it should be is:

// Declare T3 as a type
using T3 = decltype (std::declval<T>()*std::declval<T2>());
// Now we can use T3
Matrix<T3> result = Matrix<T3>::gen_full(this->lines(), other.cols());

Or here, in gen_full:

template<class T>
Matrix<T> Matrix<T>::gen_full(unsigned int lines, unsigned int cols, T value){
    for(unsigned int i = 0; i < lines; i++) {
        std::vector<T> line;
        for(unsigned int j = 0; j < cols; j++) {
            line.push_back(value);
        }
        this->data.push_back(line); // Error here
    }
};

You're using this, but gen_full is a static function so this isn't available.

We can rewrite it as:

template<class T>
Matrix<T> Matrix<T>::gen_full(unsigned int lines, unsigned int cols, T value){
    Matrix<T> m; 
    for(unsigned int i = 0; i < lines; i++) {
        std::vector<T> line;
        for(unsigned int j = 0; j < cols; j++) {
            line.push_back(value);
        }
        m.data.push_back(line); // Error here
    }
    return m; 
};

You have the same issue on lines 346 and 348 that you had on 86 and 87:

decltype(std::declval<T>() - std::declval<T2>()) T3;

Matrix<T3> res(this->lines(), this->cols());

We can fix it the same way we did there (with using T3 = decltype(...))

On line 350, you declare i as const, and then you increment it. We can just remove the const and it works.

Other issues

Once we got through the main issues, there are still a few other issues that we can only catch by trying to instantiate the class.

For example, we can use a dummy function to get the compiler to check this for us:

void foo() {
    // Forces the compiler to instantiate Matrix<double>
    Matrix<double> A;
    Matrix<double> B(A.tri_lo()); 
}

When we try to do this, we get a few cryptic errors, such as here on line 260:

Matrix<T> res(this->lines(), this->cols());

Gcc gives me the error

<source>: In instantiation of 'Matrix<T> Matrix<T>::tri_lo(bool) const [with T = double]':
<source>:365:31:   required from here
<source>:262:15: error: passing 'const Matrix<double>' as 'this' argument discards qualifiers [-fpermissive]
  262 |     Matrix<T> res(this->lines(), this->cols());
      |               ^~~

What this means is that you're trying to use functions that aren't const (such as lines() and cols()) in a const context (since tri_lo is const)

We can fix this by marking lines() and cols() as const:

// On line 32 and 33
unsigned int cols() const; // Implemented
unsigned int lines() const; // Implemented

And here as well:

// Lines 71 to 75
template<class T>
unsigned int Matrix<T>::cols() const { return this->data.size(); };

template<class T>
unsigned int Matrix<T>::lines() const { return this->data[0].size(); };

What was causing the original problem?

As far as I can tell, the original problem occurred because lines() and cols() weren't marked const.

Conclusion

There were a lot of errors that Visual Studio didn't catch. It's a good idea to use a separate compiler, like gcc or clang, which will catch errors sooner and faster. You can use them online at https://godbolt.org, or you can install them locally.

Here is the original version of your code, along with the errors shown by gcc: https://godbolt.org/z/5eiRNw

And here's the updated version of your code, with the errors fixed (including the one described in your original post): https://godbolt.org/z/vFlyvk

You still need to add an implementation of Matrix<T>::gen_uninitialized, and on line 226, clang warns you that std::vector<T> diag(); is interpreted as the forward-declaration of a function named diag (remove the parenthesis), but everything else looks good!