Re: [eigen] std::swap doesn't work on Map

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

2010/4/6 Eamon Nerbonne <eamon.nerbonne@xxxxxxxxx>:
> I'm not that familiar with all the internals, so I'm probably mistaken - but
> aren't most expressions passed by reference - thus bypassing the
> copy-constructor anyhow?

Not anymore. A big change occured a few months ago (see "nesting"
threads on this list) whereby most expressions are now stored by
value, not by reference, in the expression tree. This allowed to get
rid of NestByValue and let compilers generate better code (except for
the great gcc 4.4 which doesn't seem to care).

Benoit


> --eamon@xxxxxxxxxxxx - Tel#:+31-6-15142163
>
>
> On Tue, Apr 6, 2010 at 02:10, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
>>
>> Yes, this is the problem, but I can't make this copy constructor
>> private, because it is being used when a Map expression gets nested
>> into a bigger expression...
>>
>> Right now I am looking for a minimal-effort solution as there are more
>> pressing things to do...
>>
>> Benoit
>>
>> 2010/4/5 Eamon Nerbonne <eamon.nerbonne@xxxxxxxxx>:
>> > Isn't the issue really that Eigen::Map isn't really a value?  Eigen::Map
>> > is
>> > more like a pointer, except it's used like a value, syntactically
>> > I'm not sure this is actually a good idea, but an alternative would thus
>> > be
>> > to split Eigen::Map into a "pointer" and a "value" type so that you can
>> > swap
>> > the pointers as expected and prevent swapping on values by disabling the
>> > copy-constructor (you can't really copy-construct those anyhow - which
>> > leads
>> > to this problem).
>> > Basically, Eigen::Map's copy constructor isn't really behaving like
>> > you'd
>> > expect a copy constructor to behave.  Which isn't a big issue, granted
>> > ;-).
>> > --eamon@xxxxxxxxxxxx - Tel#:+31-6-15142163
>> >
>> >
>> > On Mon, Apr 5, 2010 at 23:55, Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
>> > wrote:
>> >>
>> >> Hi Joel,
>> >>
>> >> 2010/4/5 joel falcou <joel.falcou@xxxxxx>:
>> >> > Benoit Jacob wrote:
>> >> >>
>> >> >> Normally we could consider fixing this by providing a partial
>> >> >> template
>> >> >> specialization of std::swap for Eigen types, but the big problem is
>> >> >> that std::swap takes only 1 template parameter and expects both
>> >> >> sides
>> >> >> to be of the same type, and this is not always the case with Eigen
>> >> >> swap on expressions. We could still catch the case where both sides
>> >> >> have the same type, but I'm not sure what to do then. If we let that
>> >> >> work, it's awkward that we require both sides to have the same type
>> >> >> in
>> >> >> std::swap and not in .swap(). If we want to emit an error, it's hard
>> >> >> to do because of SFINAE (so generating an error there would simply
>> >> >> discard the specialization.). I have to read back the rules of
>> >> >> SFINAE,
>> >> >> perhaps it will work to let our std::swap specialization call
>> >> >> another
>> >> >> function from where we trigger an error...
>> >> >>
>> >> >> Benoit
>> >> >>
>> >> >
>> >> > Look how boost::swap works, there is a simple trick to have your own
>> >> > eigen::swap that behaves correctly
>> >> > without tricky SFINAE. Basically :
>> >> >
>> >> > namespace eigen {
>> >> > namespace details
>> >> > {
>> >> >  template<class T1,class T2>
>> >> >  void swap( T1& a, T2& b)
>> >> > {
>> >> >   using std::swap;
>> >> >   swap(a,b); }
>> >> > }
>> >> >
>> >> > template<class T1,class T2>
>> >> > void swap( T1& a, T2& b)
>> >> > {
>> >> >  details::swap(a,b);
>> >> > }
>> >> >
>> >> > }
>> >> >
>> >> > the main swap having *two* templates arguments make it more
>> >> > specialized
>> >> > than
>> >> > the ADL found swap for type T1 and T2.
>> >> > Then the using clause in the details::swap bring std::swap in the
>> >> > game
>> >> > if
>> >> > and only if the proper swap can't be found by ADL.
>> >> >
>> >> > Using this swap then behaves like that:
>> >> > if a swap exists in the namespace of T1, it is used.
>> >> > else std::swap is used silently.
>> >> >
>> >> > Now, just tell user to use eigen::swap.
>> >>
>> >> We already have a swap() that works well in Eigen, depending on two
>> >> template parameters. It has a different calling syntax: it is a member
>> >> function. a.swap(b).
>> >>
>> >> Here the problem is with users who expect std::swap to work on the Map
>> >> expression (where it is really tricky), and discover that it doesn't.
>> >> If we decide that it's OK to tell them to use a different swap()
>> >> function, then Eigen's current solution is already good enough.
>> >>
>> >> I was wondering if we could do something to prevent the bad surprise
>> >> with std::swap from happening in the first place. I'll try to get a
>> >> static assertion to work, with an informative error message.
>> >>
>> >> Benoit
>> >>
>> >> >
>> >> > Reference here:
>> >> >
>> >> > http://www.boost.org/doc/libs/1_42_0/boost/utility/swap.hpp
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > ___________________________________________
>> >> > Joel Falcou - Assistant Professor
>> >> > PARALL Team - LRI - Universite Paris Sud XI
>> >> > Tel : (+33)1 69 15 66 35
>> >> >
>> >> >
>> >> >
>> >> >
>> >> >
>> >>
>> >>
>> >
>> >
>>
>>
>
>