Re: [eigen] MatrixBase::swap -- why const?

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

Thanks again for this very interesting reply !!

2009/6/5 Patrick Mihelich <patrick.mihelich@xxxxxxxxx>:
>> the problem is that c++ doesn't seem to allow to pass a non-constant
>> reference to a temporary.
>
> Right. This problem is solved by rvalue references in C++0x. I believe what
> you really want is:
>
> template<typename OtherDerived>
> void swap(MatrixBase<OtherDerived>&& other)
>
> which will bind to temporaries as well as lvalue references.

So that's yet one more place where C++0x solves our problems... (there
are already like 50 such places or so...)

> Similarly for
> lazyAssign, and anywhere else you're currently abusing const_casts to
> support temporaries.

Wait, are we doing anything bad in lazyAssign ?? I don't remember
about that. lazyAssign is defined in Assign.h and there's no
const_cast there. There are const_cast's in the coefficient mutator
methods coeffRef() that are available in all Lvalue expressions (for
example Transpose), and that's called (on the left hand side of the
assignment) by lazyAssign, but the coeffRef() methods are not
const-qualified so you can't call them on const expressions without
const_casting the expression itself, and lazyAssign doesn't do that.

> Of course rvalue references are only supported by the
> very latest compilers :(

Yes, GCC >= 4.3.

>
> Using const reference arguments and const_cast is the simplest solution, but
> has some serious drawbacks. It is indeed a breach of contract. It works in
> all the cases you want it to work, but also ones you don't!

Yes, we agree.

>
> It's possible to emulate the "move semantics" of rvalue references in pure
> C++03 to some degree [...] A drawback to emulation is you'd need to use swap on
> temporaries like this:
>
> matrix.row(i).swap( move(matrix.row(j)) )
>
> which is more verbose with the "move" call but makes permission to modify
> the object explicit.

Thanks, this is good to know. However:
 - if this is C++03 while we claim to require only C++98, it's not
clear a priori if this will work on all compilers that we try to
support;
 - if we start to require a newer standard than C++98, then in a
couple of years from now, it will appear as wiser to skip C++03 and
use directly C++0x
 - finally C++0x allows for the same API as what we currently do in
C++98 so it's most tempting to just do as you explain below:

> Another option is to stick with the current solution but use rvalue refs
> when available:
>
> #ifdef HAVE_CPP0X
> #define EIGEN_RV_REF(type) type&&
> #else
> #define EIGEN_RV_REF(type) const type&
> #endif
>
> template<typename OtherDerived>
> void swap( EIGEN_RV_REF(MatrixBase<OtherDerived>) other )
>
> That way at least the early adopters can have their type safety. Probably
> another macro is needed to deal with the const_casting; I'm too tired to
> work out the full details.

I like this approach, it seems like something that we can do at any time.

If however somebody is interested in enforcing const safety without
C++0x, then we can always add your above C++03 solution as an option
(so it would be required to use move() if some symbol
EIGEN_ENFORCE_CONSTNESS) is defined.

> swap() for heterogeneous types makes me a bit uneasy, but maybe it is OK.
> I'll have to take a closer look later.

Hm throughout Eigen we allow to mix any types of expressions together,
only making requirements on the scalar type and sizes, so this is just
about being consistent with the rest of Eigen.

Benoit