Re: [boost] [thread] Boost.Thread defines boost::move which conflicts with Boost.Move

Subject: Re: [boost] [thread] Boost.Thread defines boost::move which conflicts with Boost.Move
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2012-01-01 20:50:26

Le 02/01/12 01:37, Vicente J. Botet Escriba a écrit :
> Le 01/01/12 23:25, Vicente J. Botet Escriba a écrit :
>> Le 01/01/12 20:53, Eric Niebler a écrit :
>>> On 1/1/2012 11:16 AM, Hartmut Kaiser wrote:
>>>>> On 12/31/2011 1:58 PM, Hartmut Kaiser wrote:
>>>>>> VC10, SVN trunk:
>>>>>>
>>>>>> #include<boost/move/move.hpp>
>>>>>> #include<boost/thread.hpp>
>>>>>>
>>>>>> int main()
>>>>>> {
>>>>>> return 0;
>>>>>> }
>>>>>>
>>>>>> Results in:
>>>>>>
>>>>>> .../boost/thread/detail/move.hpp(28) : error C2995:
>>>>>> 'remove_reference<T>::type&&boost::move(T&&)' : function template
>>>>>> has already been defined .../boost/move/move.hpp(466) : see
>>>>>> declaration of 'boost::move'
>>>>>>
>>>>>> IMHO, Boost.Thread needs to be changed to rely on Boost.Move for 
>>>>>> move
>>>>>> semantics instead of defining its own implementation for 
>>>>>> boost::move().
>>>>> That sounds serious. Hartmut, can you file a showstopper for 1.50 
>>>>> so this
>>>>> doesn't get lost?
>>>> I already filed it: #6341. However, shouldn't it be a show stopper 
>>>> for 1.49?
>>> Ah. You said trunk. Does this also happen on release? If so, yes, this
>>> should be a showstopper for 1.49.
>>>
>> Hi,
>>
>> this was already identified one month ago (see Boost.Thread and 
>> Boost.Move collision 
>> http://boost.2283326.n4.nabble.com/Boost-Thread-and-Boost-Move-collision-tt4127995.html).
>>
>> This was introduced in 1.48 when Boost.Move was released.
>>
>> From the option Ion presented the option to adapt Boost.Thread to use 
>> Boost.Move was the more convenient. Unfortunately this will need some 
>> time. Mankarse said that is working on it, but for the moment we 
>> don't have a path ready.
>>
>> I proposed a temporary solution " What I purpose for the short term 
>> (1.49) is to let the user to choose the namespace for the move 
>> function (by default Boost.Thread will use boost::), and the user 
>> could state that it should use boost::thread:: instead. ", but nobody 
>> supported it. With this possibility, Boost.Move could force the 
>> namespace to boost::thread.
>>
>> <http://boost.2283326.n4.nabble.com/template/NamlServlet.jtp?macro=user_nodes&user=267900>Mankarse 
>> do you a have a patch ready?
>>
>> I could try to implement the temporary workaround which should be 
>> less risky.
>>
>>
> Hi again,
>
> I think that the modification I did make the problem more evident
>
> #ifndef BOOST_NO_RVALUE_REFERENCES
> template <class T>
> typename remove_reference<T>::type&&
> move(T&& t)
> {
> typedef typename remove_reference<T>::type Up;
> return static_cast<Up&&>(t);
> }
> #endif
>
> This was need to make working some move related tests..
>
> I have replaced the definition by a #include <boost/move/move.hpp> 
> conditionally
>
> #ifndef BOOST_NO_RVALUE_REFERENCES
> #include <boost/move/move.hpp>
> #endif
>
> There is yet a problem if I add the include unconditionally as the the 
> Boost.Thread prototype in boost/detail/thread.hpp
>
> template<typename T>
> typename 
> enable_if<boost::is_convertible<T&,boost::detail::thread_move_t<T> >, 
> boost::detail::thread_move_t<T> >::type move(T& t);
>
> and the Boost.Move prototype
>
> template <class T>
> typename 
> BOOST_MOVE_BOOST_NS::disable_if<has_move_emulation_enabled<T>, 
> T&>::type move(T& x);
>
> conflicts:
>
> test_5351.cpp: In function ‘int main(int, char**)’:
> test_5351.cpp:20: error: call of overloaded 
> ‘move(boost::packaged_task<int>&)’ is ambiguous
> ../../../boost/move/move.hpp:294: note: candidates are: typename 
> boost::move_detail::disable_if<boost::has_move_emulation_enabled<T>, 
> T&>::type boost::move(T&) [with T = boost::packaged_task<int>]
> ../../../boost/thread/detail/move.hpp:55: note: typename 
> boost::enable_if<boost::is_convertible<T&, 
> boost::detail::thread_move_t<T> >, boost::detail::thread_move_t<T> 
> >::type boost::move(T&) [with T = boost::packaged_task<int>]
>
> So that on compilers don't supporting Rvalue references, the collision 
> is yet there, but only when using boost::detail::thread_move_t, i.e. 
> on the Boost.Thread side
>
> Ion, could you add a temporary disabling condition in your prototype 
> so that there is no conflict
>
> template <class T>
> typename BOOST_MOVE_BOOST_NS::disable_if<has_move_emulation_enabled<T> 
> OR boost::detail::is_thread_move_t<T> , T&>::type move(T& x);
>
> for a adequate boost::detail::is_thread_move_t type trait?
>
> Sorry for the disturbance. Let me know if this is a show stopper yet 
> and I need to apply the temporary workaround changing the namespace 
> for the Boost.Thread move function.
>
> Hartmut, could you tel me if afetr updating to the Committed revision 
> 76268 this solve your issue?
>
> Best,
> Vicente
>
> _______________________________________________
> Unsubscribe & other changes: 
> http://lists.boost.org/mailman/listinfo.cgi/boost

Ion,

with the following patch I'm able to avoid the conflict between 
Boost.Thread and Boost.Move

svn diff
Index: move.hpp
===================================================================
--- move.hpp    (revision 75884)
+++ move.hpp    (working copy)
@@ -280,6 +280,10 @@
        : BOOST_MOVE_BOOST_NS::integral_constant<bool, false>
     {};

+   template <class T>
+   struct has_move_emulation_enabled_aux
+     : has_move_emulation_enabled<T> {};
+
     template <class T>
     struct has_nothrow_move
        : public BOOST_MOVE_BOOST_NS::integral_constant<bool, false>
@@ -290,8 +294,9 @@
     //                            move()
     //
     
//////////////////////////////////////////////////////////////////////////////
+
     template <class T>
-   typename 
BOOST_MOVE_BOOST_NS::disable_if<has_move_emulation_enabled<T>, T&>::type 
move(T& x)
+   typename 
BOOST_MOVE_BOOST_NS::disable_if<has_move_emulation_enabled_aux<T>, 
T&>::type move(T& x)
     {
        return x;
     }

Boost.Thread will specialize has_move_emulation_enabled_aux<X> for each 
class that uses the Boost.Thread move semantics so that the Boost.Move 
overload is not taken in account.
While this is temporary, it is not too intrusive to Boost.Move so the 
modification can stay for some time.

Please let me know if you want I apply this patch on trunk and if you 
prefer a better name.

Best,
Vicente