Type Mismatch?

[Omega037]

I keep getting an error when trying to use a matrix class someone else wrote about 10 years ago in VC++ 6.0:

Optimizer.cpp:48: error: no matching function for call to 'matrix<double>::matrix(matrix<double>)'
Matrix.h:16: note: candidates are: matrix<T>::matrix(matrix<T>&) [with T = double]

The following is the code with most of the irrelevant stuff removed.

The matrix function definition looks like this:

template<class T>
matrix<T>::matrix(matrix<T> &x)
{
	}

The definition of testPredictor looks like this:

matrix<double> Predictor::testPredictor(matrix<double> _testX)
{
	matrix<double> predictY;
	return predictY;
}

The actual line of code where the error trips is:
matrix<double> trainPredictY = testPredictor(trainX);

where trainX is defined in the header as:
matrix<double> trainX;

Any ideas? Thanks.

[kbw]

It probably should be:
matrix<T>::matrix(const matrix<T> &x)

[moorecm]

This one is tricky. I recall an item like this in C++ Gotchas.

Consider what this does:

atrix<double> trainPredictY = testPredictor(trainX);

We often write this off as initialization--meaning a constructor call. If my understanding is correct, this actually creates a temporary and then uses the copy constructor (which does not exist in this case) to construct trainPredictY.

Try direct initialization, like this:

atrix<double> trainPredictY( testPredictor(trainX) );

I'm interested to see if this works or not. :P

[Omega037]

kbw, now it gives me this error:
Matrix.cpp:47: error: passing 'const matrix<double>' as 'this' argument of 'vect<T> matrix<T>::get_row(int) [with T = double]' discards qualifiers


vect is another class with similar functions as matrix, just for one dimension

moorecm, it gives me a similar error:

Optimizer.cpp:48: error: no matching function for call to 'matrix<double>::matrix(matrix<double>)'
Matrix.h:16: note: candidates are: matrix<T>::matrix(matrix<T>&) [with T = double]
Matrix.h:15: note: matrix<T>::matrix(int, int) [with T = double]
Matrix.h:14: note: matrix<T>::matrix() [with T = double]

Matrix also has this overloaded operator:

template<class T>
matrix<T> &matrix<T>::operator=(matrix<T> &x)
{
}

[kbw]

Unfortunately, we'd need to see the code, but I suspect there's lots of it.

[jsmith]

The problem looks obvious to me -- it's the "can't bind temporary to non-const reference"
problem.

The constructor for matrix needs to be declared to take a const reference, not a non-const
reference.

If you can't modify the constructor, then you have to stupidly (and inefficiently) break
the line into two:

matrix<double> trainPredictY;
trainPredictY = testPredictor(trainX);

and hope that operator= doesn't take a non-const reference too.

[RedX]

He is not returning a temp. The returned object is a copy (no & in the returned value).

I'm not sure but the template says class and not typename. Maybe that's a problem since you are passing double (not a class). Maybe you should post more code (the matrix class would be good and the optimizer).

[Duthomhas]

Unfortunately, your original matrix class was not written with const-correctness in mind. The get_row() member should be declared as a const function, but it is not, so the compiler is complaining that you are using a function that does not promise to leave the matrix unchanged. However, you are trying to use it on a const matrix...

Take a few minutes to look through the class and affix "const" to all the functions that do not modify the class. For example:

template<class T>
class matrix
  {
  ...
  public:
    vect<T> get_row(int row) const;
  ...
  };

template<class T>
vect<T> matrix<T>::get_row(int row) const
  {
  ...
  }

Hope this helps.

[Omega037]

Some of the stuff is proprietary, but I can post the Matrix class without too much concern I think.

Matrix.h:

#include "Vect.h"

template<class T>
class matrix 
{
public:
	matrix();
	matrix(int i, int j);
	matrix(matrix<T> &x);
	~matrix();
	int get_row_size(void);
	int get_col_size(void);
	vect<T> get_row(int _row);
	void set_row(int _row, vect<T> _vect);
	void set_dim(int i, int j);
	operator vect<T> ();
	T &operator()(int i, int j);
	matrix<T> &operator=(matrix<T> &x);
private:
	int rows;
	vect<T> *pt;
};

Matrix.cpp:

#include "Matrix.h"

template<class T>
matrix<T>::matrix()
{
        rows = 0;
        pt = NULL;
}

template<class T>
matrix<T>::matrix(int i, int j)
{
	rows = i;
	pt = new vect<T> [rows];
	if (pt == NULL)
	{
	   cout << "Failed to allocate array.\n";
	   exit(EXIT_FAILURE);
	}
}

template<class T>
void matrix<T>::set_dim(int i, int j)
{
	delete [] pt;
	rows = i;
	pt = new vect<T> [i];
    if (pt == NULL)
    {
       cout << "Failed to allocate array.\n";
       exit(EXIT_FAILURE);
    }
	for ( int k = 0; k < i; k++ ) {
		pt[k].set_dim(j);
	}
}

template<class T>
matrix<T>::matrix(matrix<T> &x)
{
	rows = x.rows;
	pt = new vect<T> [rows];
	for ( int i = 0; i < rows; i++ ) {
		set_row(i,x.get_row(i));
	}
}		

template<class T>
matrix<T> &matrix<T>::operator=(matrix<T> &x)
{
	delete [] pt;
	rows = x.rows;
	pt = new vect<T> [rows];
	for ( int i = 0; i < rows; i++ ) {
		set_row(i,x.get_row(i));
	}
    return *this;
}


template<class T>
matrix<T>::~matrix()
{
	delete [] pt;
}

template<class T>
int matrix<T>::get_row_size(void) 
{
	return rows;
}

template<class T>
vect<T> matrix<T>::get_row(int i)
{
        if (i < 0 || i >= rows )
        {   
           cout << "Index out of range: " << "i = " << i  <<  endl;
           exit(EXIT_FAILURE);
        }
		return pt[i];
}


template<class T>
int matrix<T>::get_col_size(void)
{
		if ( pt == NULL ) 
			return 0;
		else
	        return pt[0].get_dim();
}

template<class T>
T &matrix<T>::operator()(int i, int j)
{
        if (i < 0 || j < 0 || i >= rows || pt == NULL || j >= pt[0].get_dim())
        {   
           cout << "Index out of range: " << "i = "<< i  << " j = " << j << endl;
           exit(EXIT_FAILURE);
        }
        return pt[i](j);
}

template<class T>
void matrix<T>::set_row(int _row, vect<T> _vect)
{
		if ( _row < 0 || _row >= rows ) 
        {   
           cout << "Index out of range: " << "row = "<< _row  << endl;
           exit(EXIT_FAILURE);
        }
		else 
		{
	        pt[_row] = _vect;
		}
}

template<class T>
matrix<T>::operator vect<T> ()
{
	vect<T> tempVect(rows);
	for (int i = 0; i < rows; i++) {
		tempVect(i) = pt[i](0);
	}
	return tempVect;
}

template class matrix<double>;
template class matrix<int>;

[rollie]

jsmith is correct I believe, and the returned value IS a temp, although the second solution won't work because the assignment operator is also explicitly defined to take a non-const reference to a matrix (i.e., same problem).

You could make it work using the rather-convoluted code below

const matrix<double> & cRef = testPredictor(trainX);
matrix<double> trainPredictY = const_cast<matrix<double> &>(cRef);

The real way to fix this, though, is to change both the assignment operator and copy constructor to take a 'const matrix<T> &' as the parameter.

--Rollie

[RedX]

You should definitely make the class const-correct for one and your copy-constructor should definitely take it's parameter const.

@rollie and jsmith

anyClass somefunc(){
  anyClass some_inst;
  return some_inst;
}

definitely does not produce a temporary on return since the class will be copied on return.

//note & after anyClass on return
anyClass& somefunc(){
  anyClass other_inst;
  return other_inst;
}

This produces the "can't bind temporary to non-const reference" problem. His function (testPredictor(trainX);) does not return a reference.

[Duthomhas]

#include "Vect.h"

template<class T>
class matrix 
{
public:
	matrix();
	matrix(int i, int j);
	matrix(const matrix<T> &x);
	~matrix();
	int get_row_size(void) const;
	int get_col_size(void) const;
	vect<T> get_row(int _row) const;
	void set_row(int _row, vect<T> _vect);
	void set_dim(int i, int j);
	operator vect<T> () const;
	T &operator()(int i, int j);
	matrix<T> &operator=(matrix<T> &x);
private:
	int rows;
	vect<T> *pt;
};

Hope this helps.

[edit]
The point is that things that simply return non-modifiable information should be const.

[jsmith]

@RedX: I think you don't understand what a temporary is. Your example above is completely backwards.
The first one will produce the error; the second one won't (assuming of course that the second is one is
fixed, since it will generate a compile warning about returning a reference to a local variable).

[rollie]

@RedX,

Often the temporary is omitted by the compiler when it is bound in the initialization of another object. From man g++:

-fno-elide-constructors
The C++ standard allows an implementation to omit creating a temporary which is only used to initialize another object of the same type. Specifying this option disables that optimization, and forces G++ to call the copy constructor in all cases.

If you specify this option, write a class that outputs something in the copy constructor, you'll see that your function

anyClass somefunc(){
  anyClass some_inst;
  return some_inst;
}

...

anyClass myobj

Actually calls the copy constructor twice - once when the function returns (when creating the temporary anyClass), and once when initializing the object using the return value.

--Rollie

[RedX]

I re-read the jsmiths post and see that i misunderstood what he was saying.

@rollie
I didn't know about that. Thank you for enlightening me.

[Omega037]

Duoas's solution will work I think, though now I gotta do the same for the Vect class. The following is the source code for that class.

Vect.h:

template<class T>
class vect
{
//	friend vect operator*(TYPE c, const vect &v);		// scalar 

public:
	vect();
	vect(int elements);
	vect(vect<T> &a);
	~vect();
	void set_dim(int _n);
	void clear();
	int get_dim();
	double mag();
	vect<T> &operator=(vect<T> &v);
	T &operator[](int i);
	T &operator()(int i);
	vect<T> operator-();
	vect<T> operator*(T c);	// scalar
	vect<T> operator+(vect<T> &v);
	vect<T> operator-(vect<T> &v);
	operator T();
	T operator*(vect<T> &v);	// dot
	vect<double> norm();				// Normalize
	vect<double> htan();
	T dot(vect<T> &u,vect<T> &v);		// dot
	T sum();

private:
	T *p;	// Address of memory to hold data
	int   n;	// Number of elements in vector
};

Vect.cpp:

#include "Vect.h"

template<class T>
vect<T>::vect()
{
	n = 0;
	p = NULL;
//	cout << "vector object created: " << p << endl;
}

template<class T>
vect<T>::vect(int elements)
{
	n = elements;
	p = new T[n];
//	cout << "vector object created: " << p << endl;
	if (p == NULL)
	{
	  cout << "Cannot allocated vector: \n";
	  exit(EXIT_FAILURE);
	}
	for ( int i = 0; i < n; i++ ) {
		p[i]=0;
	}
}

template<class T>
vect<T>::vect(vect<T> &v)
{
	n = v.n;
	p = new T[n];

	if (p == NULL)
        {
          cout << "Cannot allocated vector: \n";
          exit(EXIT_FAILURE);
        }
	
	for ( int i = 0; i < n; i++ ) {
		p[i]=v[i];
	}
}

template<class T>
vect<T>::~vect()
{
//	cout << "Destroyed vector object: " << p << endl;
	delete [] p;
}
 
template<class T>
void vect<T>::set_dim(int _n)
{
	delete [] p;
	n = _n;
        p = new T[n];

        if (p == NULL)
        {
          cout << "Cannot allocated vector: \n";
          exit(EXIT_FAILURE);
        }
		for ( int i = 0; i < n; i++ ) {
			p[i]=0;
		}
}

template<class T>
double vect<T>::mag()
{
	double magn  = 0;
	T x = 0;
	T *pt = p;
	T *pt_end = pt + n;
	while (pt < pt_end)
	{
		x = *pt++;
	   	magn += x*x;
	}
	return sqrt(magn);
}
		
template<class T>
vect<double> vect<T>::norm()
{
	T magn = mag();
	vect<double> w(n);
	for (int i=0;i<n;i++)
	{
		w(i)=p[i]/magn;
	}
	return w;
}

template<class T>
vect<T>& vect<T>::operator=(vect<T> &v)
{
	delete [] p;
        n = v.n;
        p = new T[n];

        if (p == NULL)
        {
          cout << "Cannot allocated vector: \n";
          exit(EXIT_FAILURE);
        }

    for ( int i = 0; i < n; i++ ) {
		p[i]=v[i];
	}
	return *this;
}

template<class T>
T& vect<T>::operator[](int i)
{
	if ( i < 0 || i >= n)
	{
		cout << "vect: index OOB: " << i << endl;
		exit(EXIT_FAILURE);
	}
	return *(p+i);
}

template<class T>
T& vect<T>::operator()(int i)
{
	if ( i < 0 || i >= n)
	{
		cout << "vect: index OOB: " << i << endl;
		exit(EXIT_FAILURE);
	}
	return *(p+i);
}

template<class T>
vect<T> vect<T>::operator-()
{
	vect<T> w(n);
	T *pt_w = w.p;
	T *pt_v = p;
	T *pt_end = p + n;
	while (pt_v < pt_end)
	   *pt_w++ = -(*pt_v++);
	return w;
}	

//vect vect::operator*(TYPE c, const vect &v)
//{
//	return v*c;
//}

template<class T>
T vect<T>::dot(vect<T> &u, vect<T> &v)		//dot
{
	if (u.n != v.n)
	{
	  cout << "Cannot dot vectors u*v:\n"
	       << "\tdifferent numbers of elements\n";
	  exit(EXIT_FAILURE);
	}
	T c = 0;
	T *pt_u = u.p;
	T *pt_v = v.p;
	T *pt_end = pt_u + u.n;
	while (pt_u < pt_end)
	   c += (*pt_u++) * (*pt_v++);
	return c;
}

template<class T>
inline vect<T> vect<T>::operator+(vect<T> &v)
{
	if (this->n != v.n)
	{
	  cout << "Cannot add vectors:\n"
	       << "\tdifferent numbers of elements\n";
	  exit(EXIT_FAILURE);
	}
	vect<T> w(this->n);
	T *pt_w = w.p;
	T *pt_this = this->p;
	T *pt_v = v.p;
	T *pt_end = pt_w + this->n;
	while (pt_w < pt_end)
	   *pt_w++ = *pt_this++ + *pt_v++;
	return w;
}

template<class T>
inline vect<T> vect<T>::operator-(vect<T> &v)
{
	if (this->n != v.n)
	{
	  cout << "Cannot subtract vectors:\n"
	       << "\tdifferent numbers of elements\n";
	  exit(EXIT_FAILURE);
	}
	vect<T> w(this->n);
	T *pt_w = w.p;
	T *pt_this = this->p;
	T *pt_v = v.p;
	T *pt_end = pt_w + this->n;
	while (pt_w < pt_end)
	   *pt_w++ = (*pt_this++) - (*pt_v++);
	return w;
}

template<class T>
inline vect<T> vect<T>::operator*(T c) 
{
	vect<T> w(n);	
	T *pt_w = w.p;
	T *pt_v = p;
	T *pt_end = p + n;
	while (pt_v < pt_end)
	   *pt_w++ = (*pt_v++) * c;
	return w;
}

template<class T>
inline T vect<T>::operator*(vect<T> &v)	//dot
{
	if (this->n != v.n)
	{
	  cout << "Cannot dot vectors u*v:\n"
	       << "\tdifferent numbers of elements\n";
	  exit(EXIT_FAILURE);
	}
	T c = 0;
	T *pt_this = this->p;
	T *pt_v = v.p;
	T *pt_end = pt_this + this->n;
	while (pt_this < pt_end)
	   c += (*pt_this++) * (*pt_v++);
	return c;
}

template<class T>
int vect<T>::get_dim() 
{
	return n;
}

template<class T>
T vect<T>::sum()
{
	T sum = 0;
	for ( int i = 0; i < n; i++) {
		sum += p[i];
	}
	return sum;
}

template<class T>
vect<T>::operator T()
{
	// Allows cast to type T, returns first value in vect
	if ( p != NULL) {
		return p[0];
	}
	else {
		return 0;
	}
}

template<class T>
vect<double> vect<T>::htan()
{
	vect<double> tempvect(n);
	// applies htan soft-limiter to each element of vector
	for (int i=0;i<n;i++)
	{
		tempvect[i]=tanh(p[i]);
	}
	return tempvect;
}

template<class T>
void vect<T>::clear()
{
	delete [] p;
	n = 0;
}

template class vect<double>;
template class vect<int>;

Should I add const to just the following or more:
vect(vect<T> &a);
int get_dim();
vect<T> &operator=(vect<T> &v);
operator T();

[kbw]

Now that stl is ubiquitous, why don't you replace it with std::vector?

[Omega037]

I've been heading in that direction, but my last dealings with it didn't go so well. I think I am going to break it all down into flowchart/pseudocode and then think about how to do it. One other concern though is speed, and I know that a hand written vector and matrix class in C with only what you need is likely faster than importing and using STL.

[kbw]

stl's vector class is as fast as a general purpose C equivalent, so speed shouldn't be a problem.

[jsmith]

+1 kbw.

But, to answer OP's question, any parameter passed by (non-const) pointer or by
(non-const) reference that the function does not intend to modify in the caller
should be passed by const pointer or const reference. The following member
functions should be fixed:

	vect(vect<T> &a);
	vect<T> &operator=(vect<T> &v);
	vect<T> operator+(vect<T> &v);
	vect<T> operator-(vect<T> &v);
	T operator*(vect<T> &v);	// dot
	T dot(vect<T> &u,vect<T> &v);		// dot 

Any member function that does not modify any of *this's data members should be
declared const. The following member functions fit that description:

	int get_dim();
	double mag();
	vect<T> operator-();
	vect<T> operator*(T c);	// scalar
	vect<T> operator+(vect<T> &v);
	vect<T> operator-(vect<T> &v);
	operator T();
	T operator*(vect<T> &v);	// dot
	vect<double> norm();				// Normalize
	vect<double> htan();
	T dot(vect<T> &u,vect<T> &v);		// dot
	T sum();