1

I have a template class, and code for its assignment operator that accepts a const lvalue reference to a two dimensional array is shown below.

template<typename T> 
class matrix{
  ...
template< size_t Nr, size_t Nc>
    matrix& operator=(const T(&arg)[Nr][Nc]) :rows(Nr), cols(Nc) {
        const T* p = &arg[0][0];
        const T* pEnd = p + (Nr * Nc);
        vdata.reserve(sizeof(T) * Nr * Nc);
        std::copy(p, pEnd, std::back_inserter(vdata));
        return *this;
    }
...
size_t rows;
size_t cols;
std::vector<T> vdata;
};

I can invoke this assignment operator using the following syntax,

matrix<double>& mweights = get reference ...
mweights = {{{ 20.0, 20.0,-10.0},
             {-20.0,-20.0, 30.0}}};

However, my first try

mweights = {{ 20.0, 20.0,-10.0},
            {-20.0,-20.0, 30.0}};

resulted in compiler (MSVC2019 with C++17 standard selected) error message "matrix::operator =': constructor initializer lists are only allowed on constructor definitions" All the different ways of object construction and initialization in Modern C++ can be a bit confusing (to be polite), and I was not sure exactly what the compiler was trying to tell me. But I gathered that I had made it very unhappy...

In this case I was expecting the compiler to consider my braced initializer as an rvalue reference to two dimensional array of doubles, and the const lvalue reference argument of the assignment operator to bind to it. Obviously I am missing something (or perhaps a lot of things if this just the tip of the iceberg..)

My questions are 1) Why the first syntax works ? and 2) Why the second does not ?

Update !

Thanks Ted for noticing my copy paste error :rows(),cols() which is what the compiler message was all about.

So the same class also has a templated constructor that takes the same kind of argument, as well as a move assignment operator. So when I added the extra braces,I bypassed the assignment operator (so the compiler stopped complaining because the template function was no longer being instantiated), and instead made a temporary using the templated constructor and passed it to the move assignment operator!!

Removing the illegal member initializers from the assignment got rid of the compiler error and the call with just two braces works fine.

I paste the other two functions and the corrected (only partly because the UB noted below still exists) assignment operator below for reference.

// ctor for initializing a matrix with syntax 
// auto m = matrix<T>({{1,2,3},{4,5,6}}); or
// auto m = matrix<T>{{{1,2,3},{4,5,6}}};
template< size_t Nr, size_t Nc>
matrix(const T(&arg)[Nr][Nc]) :rows(Nr), cols(Nc) {
    const T* p = &arg[0][0];
    const T* pEnd = p + (Nr * Nc);
    vdata.reserve(sizeof(T) * Nr * Nc);
    std::copy(p, pEnd, std::back_inserter(vdata));
}

//move assignment (cannot be template)
matrix& operator = (matrix&& arg)
{
    // std::cout << "\n matrix move assignment to " << this;
    rows = arg.rows;
    cols = arg.cols;
    vdata = std::move(arg.vdata);
    arg.rows = 0;
    arg.cols = 0;
    return *this;
}
// assignment from arrays.
template< size_t Nr, size_t Nc>
matrix& operator=(const T(&arg)[Nr][Nc]) {
    rows = Nr;
    cols = Nc;   
    const T* p = &arg[0][0];
    const T* pEnd = p + (Nr * Nc);
    vdata.clear();
    vdata.reserve(Nr * Nc);
    std::copy(p, pEnd, std::back_inserter(vdata));
    return *this;
}

Many Thanks !

rakeshdn
  • 135
  • 1
  • 7
  • 2
    Do note that `std::copy(p, pEnd, std::back_inserter(vdata));` is actually undefined behavior since iterating past the end of the first row of data invalidates the pointer even though there is valid memory after it. – NathanOliver Nov 18 '19 at 21:43
  • 1
    `matrix& operator=(const T(&arg)[Nr][Nc]) :rows(Nr), cols(Nc) {` can't be in the program when you say that it works. – Ted Lyngmo Nov 18 '19 at 21:43
  • 1
    Alternative: https://godbolt.org/z/SQ-qqe (although, note what @NathanOliver-ReinstateMonica wrote). – Ted Lyngmo Nov 18 '19 at 21:48
  • 1
    @Ted Thanks. That was copy paste from a templated constructor of the similar nature. But the program did compile. and I did check the value of the assignment. I deleted the rows(Nr),cols(Nc) after I saw your comment.!!. I will report back on this. – rakeshdn Nov 18 '19 at 21:50
  • @NathanOliver-ReinstateMonica Could you please elaborate or provide a link with more information. My mental picture of a two dimensional array is contiguous area of memory of size (sizeof(T)*number of rows* number of columns) and T* to be valid inside that range. – rakeshdn Nov 18 '19 at 22:00
  • 2
    @rakeshdn see: https://stackoverflow.com/a/58699176/4342498 – NathanOliver Nov 18 '19 at 22:02
  • 1
    I just noticed that this is probably not what you want: `vdata.reserve(sizeof(T) * Nr * Nc);`. I think you mean `vdata.reserve(Nr * Nc);` – Ted Lyngmo Nov 18 '19 at 22:24
  • Ok. I better stop coding... seriously how many bugs can I pack into 6 lines of code.. :) – rakeshdn Nov 18 '19 at 22:31
  • :-) I go lay on the couch for a while when that starts happening. – Ted Lyngmo Nov 18 '19 at 22:34

1 Answers1

0

The answer is already in the update part of the question.

For completeness, (and not to mess with SO bookkeeping) I will post a response. The first syntax using {{{},{}}} was actually not using the template assignment operator at all and in the process hiding the syntax errors the compiler originally complained about.

Instead compiler generates code to create a temporary using a templated constructor that takes same kind of argument as the assignment operator, and then use move assignment (if not available then the copy assignment would have been used) to perform the same task.

The second syntax {{},{}} works once the assignment operator is fixed.

Also please note there are (at-least) two more issues in code above as noted in comments.

  1. UB in pointer access to two dimensional array.
  2. vdata.reserve call should not have sizeof(T).

Thanks again for all the comments!

rakeshdn
  • 135
  • 1
  • 7