Re: [Boost-users] [multi_array] std::swap and subarray

Subject: Re: [Boost-users] [multi_array] std::swap and subarray
From: Sergey Mitsyn (svm_at_[hidden])
Date: 2010-02-20 07:13:13

Hello alfC,

Thanks for explanation!

Yes, the code you provided works perfectly with my compiler :).

The actual problem I've faced is that std::random_shuffle makes a mess 
with multi_array in msvc 2008. The interesting fact is that 
std::iter_swap, which is used by std::random_shuffle, works in gcc 
without one more overload in std:

//std::swap( a[0], a[1] );
std::iter_swap( a.begin(), a.begin()+1 );

Looks like msvc's std::iter_swap always uses std::swap, while gcc's one 
uses it only when iterator_traits<...>::reference is the same as 
value_type&.

Ok, now I see I don't really understand how stl handles types with 
reference semantics. Looks like std::swap should be overloaded. Maybe 
stl algorithms shouldn't be used with such types at all?

Cheers,
        Sergey Mitsyn.

On 20.02.2010 8:26, alfC wrote:
> Hi Sergey,
>
> Ultimately, the problem is the compiler I think.
>
>>  I've got somehow unexpected behaviour of std::swap with multi_array's
>>  subarray. For example, given a 2x2 multi_array:
>>   ...
>>           std::swap( a[0], a[1] );
>>
>>  With msvc 2008 + boost 1.42, one of the rows overwrites the other and
>>  the output is:
>>
>>  21   22
>>  21   22
>
> The problem is that a[0] and a[1] are temporary objects,  and they
> cannot be swapped in the std::swap sense (since it takes two reference
> to non-const).
> fortunately gcc 4 catches this problem and your original code doesn't
> compile.
>
> Then using your hack doesn't work either (i.e. doesn't compile in gcc
> 4)
>
>>  template<typename TValue, int K>
>>  void swap( boost::detail::multi_array::sub_array<TValue, K>  &lhs,
>>            boost::detail::multi_array::sub_array<TValue, K>  &rhs)
>
> ...because of the same reason.
>
>>  Well, probably that's not a problem for now, but I dunno if it's a bug
>>  or std::swap is not supported - coundn't find anything about this.
>
> it is a (conceptual) bug in your compiler that allows you to take a
> temporary as a non-const reference. (gcc wins for the conceptual
> superiority this time).
>
> Now, to solve your problem with the smallest change possible you can
> try with
>
> template<typename TValue, boost::detail::multi_array::size_type /*was
> int*/ K>
> void swap(
>    boost::detail::multi_array::sub_array<TValue, K>  lhs,
>    boost::detail::multi_array::sub_array<TValue, K>  rhs)
> {
>    ... same code here
> }
>
> note two changes, 1) we must use
> boost::detail::multi_array::size_type instead of int, otherwise the
> overload template is not found in cases where the size_t is not int
> for example).
> 2) use plain variable type as argument, you may say: What a waste to
> pass by value!! well is not that bad because sub_arrays are really
> references already, elements are not copied.
>
> It works in gcc (actually this is the only variant that compiles with
> gcc), Let me know your results in VS2008.
>
> I think this solves the problem,  (except for the fact that there it
> may be a more efficient way by swapping element by element).
>
> Good luck,
> Alfredo