std::vector size not being set and data isn't assigned properly -> subscript range error

Question

Debug Assertion Failed!

Program: File: c:\program files (x86)\microsoft visual studio\2017\community\vc\tools\msvc\14.15.26726\include\vector Line: 1742

Expression: vector subscript out of range

For information on how your program can cause an assertion failure, see the Visual C++ documentation on asserts.

(Press Retry to debug the application) Sandbox.exe has triggered a breakpoint.

I'm trying to write a Variable dimension (as in 1 type to encompass any n by m matrix) matrix library for another project, so i want to create and modify "2D" arrays of numbers. To do this i used std::vector of std::vectors.

I understand the error is saying that the index I am trying to access is out of range. i.e. trying to access value[5][8] of a 3x1 array. Looking at the debug information, the Multiply function flags the error and the issue itself could be with the assignment of the '2D vectors' data and out.data which could mean it is the constructor's fault, right? (But the matrices are created properly, initially..?)

Here's my code (ignore the lack of clean namesapces/ typedefs - I'll sort out all that when I'm refactoring, I like to leave it like this so I know where/what everything is)

//Maths.h

#pragma once
#include "ckpch.h" // C/C++ standard libraries
#include "Core.h" // API  (__declspec(im/export) = CK_API)

namespace Maths {

    struct CK_API Matrix {

        int Rows, Cols;

        std::vector<std::vector<float>> data;

        Matrix(int rows, int cols);

        Matrix(int rows, int cols, float *values);

        ~Matrix();

        Matrix Multiply(const Matrix& mat) const ;
        friend CK_API Matrix operator*(Matrix& left, const Matrix& right);

        friend CK_API std::ostream& operator<<(std::ostream& os, const Matrix& mat);
    };

}

//Maths.cpp

 #include "ckpch.h"
 #include "Maths.h"
 #include "Log.h" // Loads log info

namespace Maths {

    Matrix::Matrix(int rows, int cols) {
        Rows = rows;
        Cols = cols;

        std::vector<std::vector<float>> data(rows, std::vector<float>(cols, 0.0f));
        data.resize(rows, std::vector<float>(cols, 0.0f));

    }

    Matrix::Matrix(int rows, int cols, float* values) {

        Rows = rows;
        Cols = cols;
        std::vector<std::vector<float>> data(rows, std::vector<float>(cols, 0.0f));
        data.resize(rows, std::vector<float>(cols, 0.0f));
        for (int i = 0; i < rows; i++) {
            for (int j = 0; j < cols; j++) {
                data[i][j] = values[j + i * cols];
            }
        }

    }

    Matrix::~Matrix() {
        this->data.clear();

    }

    Matrix Matrix::Multiply(const Matrix& mat) const {
        int inR1 = this->Rows; // Matrix 1 Rows
        int inR2 = mat.Rows;   // Matrix 2 Rows
        int inC1 = this->Cols; // Matrix 1 Columns 
        int inC2 = mat.Cols;   // Matrix 2 Columns

        // (n x m) * (n' x m') --> (n x m')

        int outR = this->Rows;
        int outC = mat.Cols;

        Matrix out = Matrix(outR, outC);

        if (this->Cols == mat.Rows) {
            for (int i = 0; i < inR1; i++) {
                for (int j = 0; j < inC2; j++) {
                    float sum = 0.0f;
                    for (int off = 0; off < inR1; off++) {
                        sum += this->data[off][j] * mat.data[i][off];
                    }
                    out.data[i][j] = sum;
                }
            }
            return out;
        } else {
            CK_WARN("Matrix 1 Column and Matrix 2 Row dimension mismatch! ({0} =/= {1})", Cols, mat.Rows);
        }
    }

    Matrix operator*(Matrix& left, const Matrix& right) { return left.Multiply(right); }

    std::ostream& operator<<(std::ostream& os, const Matrix& mat){

        os << mat.Rows << " x " << mat.Cols << " - Matrix: " << std::endl;

        for (int i = 0; i < mat.Rows; i++) {
            for (int j = 0; j < mat.Cols; j++) {
                os << mat.data[i][j] << ", ";
            }
            os << std::endl;
        }
        return os;
    }
}

// SandboxApp.cpp

float val1[2] = {
                1.0f, 2.0f
    };
    Maths::Matrix mat1 = Maths::Matrix(1, 2, val1);

    float val2[4] = {
                        1.0f, 2.0f,
                        -1.0f, 3.0f
    };
    Maths::Matrix mat2 = Maths::Matrix(2, 2, val2);

    Maths::Matrix mat3 = mat1 + mat2;
    Maths::Matrix mat4 = mat1 * mat2;

    std::cout << mat1 << std::endl;
    std::cout << mat2 << std::endl;
    std::cout << mat3 << std::endl;
    std::cout << mat4 << std::endl;

There's all the code I have that I think is relevant due to the functions/methods etc being called - I removed everything else (overloads for the functions, adding, subtracting, etc.)

Answer 1

This line

std::vector<std::vector<float>> data(rows, std::vector<float>(cols, 0.0f));

creates a new local variable called data which is the same name as your attribute Matrix::data

This is called shadowing and is very bad, because snything you do with this local does not touch the attribute.

A decent compiler will raise a warning telling you about this.

Could you suggest the best fix? Would it be just deleting that line?

Yes.

If, instead of assignment you use std::vector::push_back() then you do not need to resize. This will be a bit slower for very large data sets.