I encountered strange side effects that I cannot explain to myself in the slightest. Probably I am missing something very obvious, but I have looked for the bug for several hours now and the code is quite simple, so I concluded that I must have some rather fundamental misunderstanding about... something.
Consider this code, that was meant to compute the product of two 2d matrices (I have changed the set()
function to add -1
to the argument cell, to make the debug output more comprehensible.
template<class T, unsigned column_count, unsigned row_count>
class Matrix
{
private:
const static unsigned row_length = column_count;
const static unsigned column_length = row_count;
using matrix_type = std::array<std::array<T, row_length>, row_count>;
matrix_type matrix;
public:
using value_type = T;
Matrix(const matrix_type& matrix) : matrix(matrix) {}
Matrix() {}
friend std::ostream& operator<<(std::ostream& o, const Matrix& rhs)
{
for (unsigned i = 0; i < column_count; ++i) {
for (unsigned j = 0; j < row_count; ++j) {
o << rhs.matrix[i][j] << ' ';
}
o << '\n';
}
return o;
}
const auto& get_rows() const { return matrix; }
const auto get_columns() const
{
std::array<std::array<T, column_length>, column_count> columns;
for (unsigned i = 0; i < row_length; ++i) {
for (unsigned j = 0; j < column_length; ++j) {
columns[i][j] = matrix[j][i];
}
}
return columns;
}
void set(unsigned i, unsigned j, T v) { matrix[i][j] = -1; }
friend Matrix operator*(const Matrix& m1, const Matrix& m2)
{
auto columns = m1.get_columns();
auto rows = m2.get_rows();
Matrix m3;
std::cout << "before:"
<< "\n";
std::cout << m1 << "\n";
std::cout << m2 << "\n";
std::cout << m3 << "\n";
unsigned i{ 0 };
for (const auto& row : rows) {
i++;
unsigned j{ 0 };
for (const auto& column : columns) {
j++;
value_type v{ 0 };
for (unsigned k = 0; k < column.size(); ++k) {
v += row[k] * column[k];
}
m3.set(i, j, v);
}
}
std::cout << "after:"
<< "\n";
std::cout << m1 << "\n";
std::cout << m2 << "\n";
std::cout << m3 << "\n";
return m3;
}
};
As you can see, the getter functions either return a copy or a constant reference. The operator*
function takes constant parameters.
I now construct two matrices like so:
std::array<int, 3> c1{ { 1, 2, 3 } };
std::array<int, 3> c2{ { 4, 5, 6 } };
std::array<int, 3> c3{ { 7, 8, 9 } };
std::array<std::array<int, 3>, 3> m1{ { c1, c2, c3 } };
std::array<std::array<int, 3>, 3> m2 = m1;
Matrix<int, 3, 3> matrix1(m1);
Matrix<int, 3, 3> matrix2(m2);
Now I invoke operator*
in different ways:
matrix1* matrix2;
result:
before:
1 2 3
4 5 6
7 8 9
1 2 3
4 5 6
7 8 9
0 0 0
0 0 0
0 0 183238709
after:
-1 -1 -1
4 5 6
7 8 9
1 2 3
4 5 6
7 8 9
0 0 0
0 -1 -1
-1 -1 -1
matrix2* matrix1;
result:
before:
1 2 3
4 5 6
7 8 9
1 2 3
4 5 6
7 8 9
0 0 0
0 0 0
0 0 -1823473620
after:
1 2 3
4 5 6
7 8 9
-1 -1 -1
4 5 6
7 8 9
0 0 0
0 -1 -1
-1 -1 -1
matrix1* matrix1;
result:
before:
1 2 3
4 5 6
7 8 9
1 2 3
4 5 6
7 8 9
1385085408 32767 401515081
1 1385085440 32767
1385085440 32767 1385085464
after:
-1 -1 -1
4 5 6
7 8 9
-1 -1 -1
4 5 6
7 8 9
1385085408 32767 401515081
1 -1 -1
-1 -1 -1
As you can see, the matrix that gets passed as first argument will be changed. Which makes no sense to me, as it is passed as a const and set()
only operates on m3
. Somehow m3
gets partly "bound" to the matrix that is the first argument of operator*
. Why?
You're writing out of bounds in your loops, since both i
and j
are incremented too early.
The code should look like this:
for (const auto& row : rows) {
// i++;
unsigned j{ 0 };
for (const auto& column : columns) {
// j++;
value_type v{ 0 };
for (unsigned k = 0; k < column.size(); ++k) {
v += row[k] * column[k];
}
m3.set(i, j, v);
j++; // <--
}
i++; // <--
}