views:

337

answers:

5

I currently have a class hierarchy like

MatrixBase -> DenseMatrix
           -> (other types of matrices)
           -> MatrixView -> TransposeView
                         -> DiagonalView
                         -> (other specialized views of matrices)

MatrixBase is an abstract class which forces implementers to define operator()(int,int) and such things; it represents 2 dimensional arrays of numbers. MatrixView represents a (possibly mutable) way of looking at a matrix, like transposing it or taking a submatrix. The point of MatrixView is to be able to say something like

Scale(Diagonal(A), 2.0)

where Diagonal returns a DiagonalView object which is a kind of lightweight adapter.

Now here's the question(s). I will use a very simple matrix operation as an example. I want to define a function like

template <class T>
void Scale(MatrixBase<T> &A, const T &scale_factor);

which does the obvious thing the name suggests. I want to be able to pass in either an honest-to-goodness non-view matrix, or an instance of a subclass of MatrixView. The prototype as written above does not work for statements such as

Scale(Diagonal(A), 2.0);

because the DiagonalView object returned by Diagonal is a temporary, and Scale takes a non-const reference, which cannot accept a temporary. Is there any way to make this work? I tried to use SFINAE, but I don't understand it all that well, and I'm not sure if that would solve the problem. It is important to me that these templated functions can be called without providing an explicit template argument list (I want implicit instantiation). Ideally the statement above could work as written.


Edit: (followup question)

As sbi responded below about rvalue references and temporaries, Is there a way to define two versions of Scale, one which takes a non-const rvalue reference for non-views, and one which takes a pass-by-value view? The problem is to differentiate between these two at compile time in a way such that implicit instantiation will work.


Update

I've changed the class hierarchy to

ReadableMatrix
WritableMatrix : public ReadableMatrix
WritableMatrixView
DenseMatrix : public WritableMatrix
DiagonalView : public WritableMatrixView

The reason WritableMatrixView is distinct from WritableMatrix is that the view must be passed around by const reference, while the matrices themselves must be passed around by non-const ref, so the accessor member functions have different const-ness. Now functions like Scale can be defined as

template <class T>
void Scale(const WritableMatrixView<T> &A, const T &scale_factor);
template <class T>
void Scale(WritableMatrix<T> &A, const T &scale_factor){
    Scale(WritableMatrixViewAdapter<T>(A), scale_factor);
}

Note that there are two versions, one for a const view, and a non-const version for actual matrices. This means for functions like Mult(A, B, C), I will need 8 overloads, but at least it works. What doesn't work, however is using these functions within other functions. You see, each View-like class contains a member View of what it's looking at; for example in the expression Diagonal(SubMatrix(A)), the Diagonal function returns an object of type DiagonalView<SubMatrixView<T> >, which needs to know the fully derived type of A. Now, suppose within Scale I call some other function like it, which takes either a base view or matrix reference. That would fail because the construction of the needed View's require the derived type of the argument of Scale; information it does not have. Still working on find a solution to this.


Update

I have used what is effectively a home-grown version of Boost's enable_if to select between two different versions of a function like Scale. It boils down to labeling all my matrix and view classes with extra typedef tags indicating if they are readable and writable and view or non-view. In the end, I still need 2^N overloads, but now N is only the number of non-const arguments. For the final result, see the here (it's unlikely to get seriously revamped again).

+1  A: 

An easy way to fix this would be to use boost::shared_ptr< MatrixBase<T> > instead of a reference.

Space_C0wb0y
A: 

May be, you should use const. ?

template <class T>
void Scale(const MatrixBase<T> &A, const T &scale_factor);
Alexey Malistov
No it should not be const, Scale will modify A.
Victor Liu
@Victor: If scale modifies A, then you should not pass the temporary to it. However, if you *really* want to do it and if your platform is Windows, you can compile the code as a non-const reference with Visual Studio. But I am not sure what will happen if you try to modify the passed object in that case.
Naveen
A: 

you are limiting the type of the first argument of Scale, but you can let the compiler figure out what type would be appropriate on its own, like this:

template <class M,class T>
void Scale(M A, const T &scale_factor);
catwalk
Yes, I could do that, but that leads to another problem. I also have VectorBase, Vector, VectorView, etc. And Scale must work differently for vectors than for matrices. Perhaps you can suggest how to incorporate this additional complication.
Victor Liu
Additional problems: A cannot be passed in by value; it could potentially be a huge copy. Second, if it's passed in by reference, then we're back to the problem where ordinary matrices are passed in fine, but views are temporaries on the stack so it wouldn't work.
Victor Liu
A: 

Don't use references, pass by value.

Let copy elision do the optimization for you, if needed.

Edouard A.
+6  A: 

This has nothing to do with templates. Your example

Scale(Diagonal(A), 2.0);

could be generalized to

f(g(v),c);

In C++03, this requires the first parameter to f() to either be passed per copy or per const reference. The reason is that g() returns a temporary, an rvalue. However, rvalues only bind to const references, but not to non-const references. This is independent of whether templates, SFINAE, TMP or whatnot are involved. It's just the way the language (currently) is.

There's also a rationale behind that: If g() returns a temporary, and f() modifies that temporary, then nobody has a chance to "see" the modified temporary. Thus the modification is done in vain and the whole thing is most likely an error.

As far as I understood you, in your case the result of g() is a temporary that's a view onto some other object (v), so modifying it would modify v. But if that's the case, in current C++, the result of g() must either be const (so that it can be bound to a const reference or it must be copied. Since const "smells" wrong to me, making that view cheap to copy would probably be the best thing.

However, there's more to this. C++1x will introduce what's called rvalue references. What we know as "references" will then be divided into either lvalue reference or rvalue references. You will be able to have functions take rvalue references and even overload based on "l/rvalue-ness". This was thought out to allow class designers to overload copy ctor and assignment for rvalue right-hand sides and having them "steal" the right-hand side's values, so that copying rvalues will e cheaper. But you could probably use it to have Scale take an rvalue and modify that.

Unfortunately your compiler very likely doesn't support rvalue references yet.


Edit (followup question):

You cannot overload f(T) with f(T&) to achieve what you want. While only the former will be used for rvalues, lvalues can bind to either argument equally well, so invoking that f with an lvalue is ambiguous and results in a compile-time error.

However, what's wrong with having an overload for DiagonalView:

template <class T>
void Scale(MatrixBase<T> &matrix, const T &scale_factor);

template <class T>
void Scale(DiagonalView<T> view, const T &scale_factor);

Is there anything I'm missing?


Another edit:

I would need a ridiculously large number of overloads then, since there are currently more than 5 views, and there are several dozen functions like Scale.

Then you would need to group together those types that can be handled in the same way. You could use some simple template-meta stuff to do the grouping. Off the top of my head:

template<bool B>
struct boolean { enum { result = B }; };

template< typename T >
class some_matrix {
  public:
    typedef boolean<false> is_view;
  // ...
};

template< typename T >
class some_view {
  public:
    typedef boolean<true> is_view;
  // ...
};

namespace detail {
  template< template<typename> class Matrix, typename T >
  void Scale(Matrix<T>& matrix, const T& scale_factor, boolean<true>)
  {
    /* scaling a matrix*/
  }
  template< template<typename> class Matrix, typename T >
  void Scale(View<T>& matrix, const T& scale_factor, boolean<true>)
  {
    /* scaling a view */
  }
}

template< template<typename> class Matrix, typename T >
inline void Scale(Matrix<T>& matrix, const T& scale_factor)
{
  detail::Scale( matrix, scale_factor, typename Matrix<T>::is_view() );
}

This particular setup/grouping might not exactly fit your needs, but you can setup something like this in ways that fit for yourself.

sbi
I understand this point now fully, but I have added another followup question.
Victor Liu
I would need a ridiculously large number of overloads then, since there are currently more than 5 views, and there are several dozen functions like Scale.
Victor Liu