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.)

Your constructor creates a local vector named `data` and initializes it. Big deal, this local variable exists only until the constructor terminates, and then disappears into that great bit bucket in the sky. Your class member, coincidentally also named `data`, remains completely uninitialized, and empty. — Sam Varshavchik, Dec 31 '18 at 19:03
@SamVarshavchik so how would I go about assigning values to the member? I thought I was initializing the member during the constructor, obviously not. Also, with what you're saying, shouldn't that mean that any operation would result in an empty/uninitialized member? addition doesn't. — Callum, Dec 31 '18 at 19:16
`int n` inside a function, whether it's a constructor or a class method, creates a local variable named `n`. The same as true with `std::vector> data;`. This creates a local variable named `data`. If you don't want a local variable named `data`, then don't declare one. What exactly is unclear to you? — Sam Varshavchik, Dec 31 '18 at 19:17
@SamVarshavchik I understood what you said, but I asked how I would intialize/assign values to the member and you've reiterated the point. My question still stands. — Callum, Dec 31 '18 at 19:25
@Callum Just remove the lines that say `std::vector> data(rows, std::vector(cols, 0.0f));`. The rest of the code looks OK (on a quick glance). — john, Dec 31 '18 at 19:26
If you have a class member `int n;`, how do you set its value? Well, you should know that, you simply set it, in a class method: `n=4;`. That's it. All class members are alike. There are no special favorites. If you have a class member named `data`, and it's a vector, just access it normally: `data.push_back(...);` or `data.resize();`. That's it. You want to initialize `data` to something? Well, go right ahead, and do it. — Sam Varshavchik, Dec 31 '18 at 19:29

ravenspoint · Accepted Answer · 2018-12-31T20:11:09.333

2

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.

edited Dec 31 '18 at 20:11

answered Dec 31 '18 at 19:18

ravenspoint

19,093
6
57
103

thanks, i've never heard of it prior and I'm using the default MS C++ compiler in VS2017...which probably is why it didn't warn me. Could you suggest the best fix? Would it be just deleting that line? (I'm going to research shadowing anyway) – Callum Dec 31 '18 at 19:29
2

@Callum You wrote this code `Rows = rows;` which is correct. You didn't write this `int Rows; Rows = rows;` which would be incorrect (because of shadowing). Why did you feel the need to treat `data` differently? – john Dec 31 '18 at 19:31
is `data.resize(...)` not resizing the member (after deleting the line(s) you pointed out)? Deleting the line would mean there's no local `data`, so calling `data.resize(rows, std::vector(cols, 0.0f))` creates an n by m matrix of zeroes. I'll just use `push_back()` i think, looking at the usage I can make it work. Thanks again. – Callum Dec 31 '18 at 20:08
You are correct. Deleting the line is fine. – ravenspoint Dec 31 '18 at 20:10

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

1 Answers1