Re: [eigen] a few updates...

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

2010/12/22 Christoph Hertzberg <chtz@xxxxxxxxxxxxxxxxxxxxxxxx>:
> On 22.12.2010 19:33, Benoit Jacob wrote:
>> 2010/12/22 Christoph Hertzberg <chtz@xxxxxxxxxxxxxxxxxxxxxxxx>:
>>> Another possibility would be:
>>> c) Pass argument to swap() by value if it is a Map/Block-expression.
>>
>> I tried that, using a enable-if trick to select abstract expressions
>> vs. plain objects.
>>
>> The problem is that that doesn't solve the problem at all. By passing
>> the expression by value, we completely forget its constness, so we
>> can't prevent doing
>>   const Expression xpr;
>>   foo.swap(xpr);
>
> Well that's again all about bug54.

You're right, see below...

> Having, e.g., a `const ColXpr` does
> not prevent modifying its data -- the constness must be encoded directly
> in the type (as it is in `Map<const T>`). Otherwise you can do:
>
>  const MatrixXd m;
>  MatrixXd::RowXpr r(m.row(0));

Nope, that you can't do anymore with my changes, because row(Index)
const now returns a Block<const T>...

....but you're still right, because there's nothing I can do to prevent this:

const ExpressionType someConstXpr;
ExpressionType someXpr(someConstXpr);
someXpr.writeStuff();

....so, indeed, const on ExpressionType can't be enforced, the only
enforceable constness for expression types is const on template
parameters i.e. Block<const T>.

In my local changes, all const-qualified expression-returning methods
return such "inherently const" types (like Block<const T>). I just was
under the wrong impression that we could do something to ensure that
when the user writes a function like

template<typename T> void foo(const DenseBase<T>& x)
{
  ...
}

we could guarantee that x can't be written too, but thank you for
reminding me why that isn't the case.

So, thanks for reorienting this debate in the right direction:
 - this remaining const correctness issue is not specific to swap, and
not specific to Eigen, but is inherent to expression templates and
more generally, writable wrapper classes in c++.
 - there is no need for what i called 'solution b', it won't help with that..

> Back to the swap-problem I can suggest another possibility:
> d) Mark most methods of Map<T> and Block<T> as const (everything that
> does not alter the Map or Block itself). Then have three possible
> parameters for swap:
> Matrix<...> &
> const Block<T> &
> const Map<T> &

Don't worry, all of that is already done in my changes, but I didn't
have to special-case Map and Block, it works for all expressions.

I'll write about my changes as soon as I push them.

Benoit

>
> where the latter two could be combined as soon as
> direct-access-unification is done.
>
> Christoph
>
> --
> ----------------------------------------------
> Dipl.-Inf. Christoph Hertzberg
> Cartesium 0.051
> Universität Bremen
> Enrique-Schmidt-Straße 5
> 28359 Bremen
>
> Tel: (+49) 421-218-64252
> ----------------------------------------------
>
>
>