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

classic Classic list List threaded Threaded
26 messages Options
12
Reply | Threaded
Open this post in threaded view
|

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

Hartmut Kaiser

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().

Regards Hartmut
---------------
http://boost-spirit.com
http://stellar.cct.lsu.edu





_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Eric Niebler-3
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?

--
Eric Niebler
BoostPro Computing
http://www.boostpro.com

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Hartmut Kaiser
> 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?

Regards Hartmut
---------------
http://boost-spirit.com
http://stellar.cct.lsu.edu

>
> --
> Eric Niebler
> BoostPro Computing
> http://www.boostpro.com
>
> _______________________________________________
> Unsubscribe & other changes:
> http://lists.boost.org/mailman/listinfo.cgi/boost


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Eric Niebler-3
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.

--
Eric Niebler
BoostPro Computing
http://www.boostpro.com

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Vicente Botet
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.

Best,
Vicente





_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Hartmut Kaiser
> 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_no
> des&user=267900>Mankarse
> do you a have a patch ready?
>
> I could try to implement the temporary workaround which should be less
> risky.

IMHO, this situation is not acceptable for any future release. Therefore, I
marked it (#6141) as a showstopper for 1.49.

Regards Hartmut
---------------
http://boost-spirit.com
http://stellar.cct.lsu.edu





_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Vicente Botet
In reply to this post by Vicente Botet
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
Reply | Threaded
Open this post in threaded view
|

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

Vicente Botet
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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Ion Gaztañaga
El 02/01/2012 2:50, Vicente J. Botet Escriba escribió:

> Ion,
>
> with the following patch I'm able to avoid the conflict between
> Boost.Thread and Boost.Move
> Please let me know if you want I apply this patch on trunk and if you
> prefer a better name.

If you think it's safe enough, please apply it, let's see how it affects
regression tests.

Ion

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Vicente Botet
Le 02/01/12 12:45, Ion Gaztañaga a écrit :

> El 02/01/2012 2:50, Vicente J. Botet Escriba escribió:
>
>> Ion,
>>
>> with the following patch I'm able to avoid the conflict between
>> Boost.Thread and Boost.Move
>> Please let me know if you want I apply this patch on trunk and if you
>> prefer a better name.
>
> If you think it's safe enough, please apply it, let's see how it
> affects regression tests.

Yes, I guess it is quite safe :)

  svn ci move.hpp -m "Move/Thread: Added type tait so that #6141 -
Compilation error when boost.thread and boost.move are used together -
can be solved on the Boost.Thread side"
Sending        move.hpp
Transmitting file data .
Committed revision 76271.

I will commit the Boost.Thread modifications this evening.

Thanks,
Vicente

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Hartmut Kaiser
In reply to this post by Vicente Botet
 
> 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?

Much better now!

Thanks.
Regards Hartmut
---------------
http://boost-spirit.com
http://stellar.cct.lsu.edu





_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Vicente Botet
In reply to this post by Vicente Botet
Le 02/01/12 13:19, Vicente J. Botet Escriba a écrit :

> Le 02/01/12 12:45, Ion Gaztañaga a écrit :
>> El 02/01/2012 2:50, Vicente J. Botet Escriba escribió:
>>
>>> Ion,
>>>
>>> with the following patch I'm able to avoid the conflict between
>>> Boost.Thread and Boost.Move
>>> Please let me know if you want I apply this patch on trunk and if you
>>> prefer a better name.
>>
>> If you think it's safe enough, please apply it, let's see how it
>> affects regression tests.
>
> Yes, I guess it is quite safe :)
>
>  svn ci move.hpp -m "Move/Thread: Added type tait so that #6141 -
> Compilation error when boost.thread and boost.move are used together -
> can be solved on the Boost.Thread side"
> Sending        move.hpp
> Transmitting file data .
> Committed revision 76271.
>
> I will commit the Boost.Thread modifications this evening.
>
I committed them yesterday

Changeset /[76277]/ by viboes
<https://svn.boost.org/trac/boost/changeset/76277>

    Thread: #6141 <https://svn.boost.org/trac/boost/ticket/6141> -
    Compilation error when boost.thread and boost.move are …

and the trunk regressions are fine (see test_6170
http://www.boost.org/development/tests/trunk/developer/thread.html).

Note that 6170 was a duplicate of 6141. ( #6170: Bugs: boost::move
conflicts in thread upgrade mutexes (1.48.0) (closed: duplicate)
<https://svn.boost.org/trac/boost/ticket/6170>)

Ion please, could you merge 76271 to release branch so that I can merge
this showstopper?

Thanks,
Vicente


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Ion Gaztañaga
El 03/01/2012 18:44, Vicente J. Botet Escriba escribió:

> Ion please, could you merge 76271 to release branch so that I can merge
> this showstopper?

Done. Thanks.

Ion

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Vicente Botet
Le 03/01/12 23:04, Ion Gaztañaga a écrit :
> El 03/01/2012 18:44, Vicente J. Botet Escriba escribió:
>
>> Ion please, could you merge 76271 to release branch so that I can merge
>> this showstopper?
>
> Done. Thanks.
>
Thanks Ion.

Now that Boost.Move and Boost.Thread don't conflict we can adapt each
one of the Boost.Thread Movable types one by one.
I have added some movable test in trunk so that we can ensure the
refactoring works as expected.

Mankarse please, could you post here the patch for the boost.thread
class so that I can start the integration of what you have already done?

Thanks,
Vicente

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Mankarse
In reply to this post by Hartmut Kaiser
On Thu, 05 Jan 2012 07:00:27 +0100, Vicente J. Botet Escriba
wrote:>Mankarse please, could you post here the patch for the
boost.thread >class so that I can start the integration of what you
have already done?
I have attached a patch to the ticket
(https://svn.boost.org/trac/boost/ticket/6194). This patch is fairly
comprehensive in terms of code, but it does not include any tests or
documentation. In the course of writing the patch, I created some
simple tests, but I have not yet translated these into Boost.Test
testcases, or included them in the patch.
I have tested the basic functionality on a number of compilers, but
there are probably still some bugs in the patched code.
The patch contains a number of known problems that may be worth
discussing:1) Explicit move() and copy() calls:boost::thread and
boost::packaged_task both have templated constructors that take an
rvalue reference to a functor (which they move or copy as
appropriate). Templates parameters cannot be deduced to a type that is
only reachable through an conversion operator. Template constructors
can also never be explicitly instansiated. (As far as I know.) This
means that the overloads that are used in the definitions of
BOOST_COPYABLE_AND_MOVABLE types (BOOST_RV_REF(type),
BOOST_CATCH_CONST_RLVALUE(type), type&), are not matched for rvalue
objects (in C++03 mode). Example:
    //In C++11 the `CopyableAndMovableFunctor` would be    //moved
into the thread.    //In C++03 this does not compile.   
boost::thread((CopyableAndMovableFunctor()))
Here, no overload would be matched, because the `type` in the
overloads is templated. To solve this problem I modified Boost.Move to
add two member functions - `copy()` and `move()` - which forward to
the rv const& and rv& conversion operators. With these, the above
example can be rewritten as:
    //In both C++11 and C++03 the `CopyableAndMovableFunctor`   
//will be moved or copied into the thread as appropriate.   
boost::thread(CopyableAndMovableFunctor().move()),   
boost::thread(CopyableAndMovableFunctor().copy())
I can imagine the names `copy` and `move` being quite common, so this
might cause unfortunate conflicts. Is this an acceptable change to
Boost.Move? Is there another way to get the functionality that does
not require this?
2) SFINAE requiredBoost.Move requires SFINAE, and by extension
Boost.Thread now also requires SFINAE. Previously, Boost.Thread did
not require SFINAE. Is this an acceptable regression, or should more
work be put into maintaining compatibility with compilers that do not
support SFINAE?
3) Not using has_move_emulation_enabledI did not notice the
`has_move_emulation_enabled` metafunction, and instead used
`is_convertible!(T&, boost::rv!(T))`. This may mean that some
situations that `has_move_emulation_enabled` accounts for will not be
accounted for by the code in the patch. I have not yet evaluated
whether simply replacing all the occurrences will work. Any thoughts?
4) Not using Vicente's patches for #6174 or #5990By the time that I
was aware of these patches, I had already independently fixed the
issues. I have not looked at either patch in detail, but they each go
about fixing the issues in a significantly different way from my
patch. Which code should be used in each of these cases?

Where should I go from here? I could do the integration into the svn
trunk version, write the documentation, or update the tests to the new
version. Are there any other tasks that I can help out with? I will of
course keep the patch updated with fixes for any bugs that I find in
it.
Evan Wallace

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Vicente Botet
Mankarse wrote
On Thu, 05 Jan 2012 07:00:27 +0100, Vicente J. Botet Escriba
wrote:>Mankarse please, could you post here the patch for the
boost.thread >class so that I can start the integration of what you
have already done?
I have attached a patch to the ticket
(https://svn.boost.org/trac/boost/ticket/6194). This patch is fairly
comprehensive in terms of code, but it does not include any tests or
documentation. In the course of writing the patch, I created some
simple tests, but I have not yet translated these into Boost.Test
testcases, or included them in the patch.
Thanks Evan,

I will try to merge it on my own repository and see how it behaves. I have added some tests to check the move semantics. They are in directory libs/thread/test/threads and libs/thread/test/sync. Maybe you could check them and suggest for others.

BTw,

Should't the following
            thread_data(BOOST_RV_REF(F) f_):
                 f(boost::move(f_))

use forward instead of move?
            thread_data(BOOST_RV_REF(F) f_):
                 f(boost::forward<F>(f_))


Why the following (both) overloads are needed?

            thread_data(BOOST_RV_REF(F) f_);
            thread_data(F const& f_);


I have tested the basic functionality on a number of compilers, but
there are probably still some bugs in the patched code.
The patch contains a number of known problems that may be worth
discussing:1) Explicit move() and copy() calls:boost::thread and
boost::packaged_task both have templated constructors that take an
rvalue reference to a functor (which they move or copy as
appropriate). Templates parameters cannot be deduced to a type that is
only reachable through an conversion operator. Template constructors
can also never be explicitly instansiated. (As far as I know.) This
means that the overloads that are used in the definitions of
BOOST_COPYABLE_AND_MOVABLE types (BOOST_RV_REF(type),
BOOST_CATCH_CONST_RLVALUE(type), type&), are not matched for rvalue
objects (in C++03 mode). Example:
    //In C++11 the `CopyableAndMovableFunctor` would be    //moved
into the thread.    //In C++03 this does not compile.   
boost::thread((CopyableAndMovableFunctor()))
Hrr, this is really a bad new. Could you create a Boost.Move ticket if the limitation is not documented and a ticket doesn't exists already for this issue?

Here, no overload would be matched, because the `type` in the
overloads is templated. To solve this problem I modified Boost.Move to
add two member functions - `copy()` and `move()` - which forward to
the rv const& and rv& conversion operators. With these, the above
example can be rewritten as:
    //In both C++11 and C++03 the `CopyableAndMovableFunctor`   
//will be moved or copied into the thread as appropriate.   
boost::thread(CopyableAndMovableFunctor().move()),   
boost::thread(CopyableAndMovableFunctor().copy())
I can imagine the names `copy` and `move` being quite common, so this
might cause unfortunate conflicts. Is this an acceptable change to
Boost.Move? Is there another way to get the functionality that does
not require this?
I don't like this too much. Why doesn't boost::move works in this case? Could you give an example on which .copy() is needed?

I don't know if this point will prevent the adoption of Boost.Move by Boost.Thread. I will comeback when I do some trials.
 
2) SFINAE requiredBoost.Move requires SFINAE, and by extension
Boost.Thread now also requires SFINAE. Previously, Boost.Thread did
not require SFINAE. Is this an acceptable regression, or should more
work be put into maintaining compatibility with compilers that do not
support SFINAE?
A lot of libraries are requiring SFINAE now. Could you create a Boost.Move ticket if the limitation is not documented and a ticket doesn't exists already for this issue?

We could have a specific code for compilers that don't support SFINAE. I will see what I can do while merging your patch.

3) Not using has_move_emulation_enabledI did not notice the
`has_move_emulation_enabled` metafunction, and instead used
`is_convertible!(T&, boost::rv!(T))`. This may mean that some
situations that `has_move_emulation_enabled` accounts for will not be
accounted for by the code in the patch. I have not yet evaluated
whether simply replacing all the occurrences will work. Any thoughts?
I have no idea. I suspect that I will need to look for this in Boost.Move.

4) Not using Vicente's patches for #6174 or #5990By the time that I
was aware of these patches, I had already independently fixed the
issues. I have not looked at either patch in detail, but they each go
about fixing the issues in a significantly different way from my
patch. Which code should be used in each of these cases?
Yes, I know that some of my commits could be in conflict with the Boost.Move adoption, but I wanted to fix as much as possible as I was not sure the adoption will be in release 1.49. I will take a lot of care while doing the merge.

Where should I go from here? I could do the integration into the svn
trunk version, write the documentation, or update the tests to the new
version. Are there any other tasks that I can help out with? I will of
course keep the patch updated with fixes for any bugs that I find in
it.
Could you try to run the test on trunk related to move semantics with your version? To do that just copy
the directories threads and sync from libs/thread/test and the Jamfile whithout changing the sources and do

bjam threads sync

I will commit this evening more updates on trunk so that these tests should run better.

Could you analyze in more detail the impact of using has_move_emulation_enabled?

Thanks a lot,
Vicente
Reply | Threaded
Open this post in threaded view
|

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

Jeffrey Lee Hellrung, Jr.-2
On Mon, Jan 9, 2012 at 7:29 AM, Vicente Botet <[hidden email]>wrote:

>
> Mankarse wrote
> >
> > On Thu, 05 Jan 2012 07:00:27 +0100, Vicente J. Botet Escriba
> > wrote:>Mankarse please, could you post here the patch for the
> > boost.thread >class so that I can start the integration of what you
> > have already done?
> > I have attached a patch to the ticket
> > (https://svn.boost.org/trac/boost/ticket/6194). This patch is fairly
> > comprehensive in terms of code, but it does not include any tests or
> > documentation. In the course of writing the patch, I created some
> > simple tests, but I have not yet translated these into Boost.Test
> > testcases, or included them in the patch.
> >
>
> Thanks Evan,
>
> I will try to merge it on my own repository and see how it behaves. I have
> added some tests to check the move semantics. They are in directory
> libs/thread/test/threads and libs/thread/test/sync. Maybe you could check
> them and suggest for others.
>
> BTw,
>
> Should't the following
>            thread_data(BOOST_RV_REF(F) f_):
>                                f(boost::move(f_))


> use forward instead of move?
>            thread_data(BOOST_RV_REF(F) f_):
>                                f(boost::forward<F>(f_))
>

I don't think so. I haven't brushed up on the latest changes to Boost.Move
since being officially added to Boost, but prior incarnations paired
boost::forward with BOOST_FWD_REF. I think boost::move is correct when
using BOOST_RV_REF.


> Why the following (both) overloads are needed?
>
>            thread_data(BOOST_RV_REF(F) f_);
>            thread_data(F const& f_);
>

I don't know the context here, but I believe, in C++03, this would:
capture-and-copy lvalues and rvalues; and capture-and-move explicitly
created emulated rvalue references. Thus, true rvalues are captured as
lvalues :( If you don't want to compromise, you could use a different set
of overloads in C++03 and C++11:

#ifndef BOOST_NO_RVALUE_REFERENCES
template< class F > thread_data(F&& f); // and use boost::forward<F>(f)
#else // #ifndef BOOST_NO_RVALUE_REFERENCES
template< class F > thread_data(F f); // and use boost::move(f)
template< class F > thread_data(boost::rv<F>& f); // and use boost::move(f)
#endif // #ifndef BOOST_NO_RVALUE_REFERENCES

I haven't tested this, but it *should* capture-and-move both true rvalues
and emulated rvalue references in C++03. And if it's an emulated rvalue
reference, the second overload requires 1 fewer move construction.


> > I have tested the basic functionality on a number of compilers, but
> > there are probably still some bugs in the patched code.
> > The patch contains a number of known problems that may be worth
> > discussing:1) Explicit move() and copy() calls:boost::thread and
> > boost::packaged_task both have templated constructors that take an
> > rvalue reference to a functor (which they move or copy as
> > appropriate). Templates parameters cannot be deduced to a type that is
> > only reachable through an conversion operator. Template constructors
> > can also never be explicitly instansiated. (As far as I know.) This
> > means that the overloads that are used in the definitions of
> > BOOST_COPYABLE_AND_MOVABLE types (BOOST_RV_REF(type),
> > BOOST_CATCH_CONST_RLVALUE(type), type&), are not matched for rvalue
> > objects (in C++03 mode). Example:
> >     //In C++11 the `CopyableAndMovableFunctor` would be    //moved
> > into the thread.    //In C++03 this does not compile.
> > boost::thread((CopyableAndMovableFunctor()))
> >
>
> Hrr, this is really a bad new. Could you create a Boost.Move ticket if the
> limitation is not documented and a ticket doesn't exists already for this
> issue?
>

It's a known limitation in Boost.Move, AFAIK. The above change should
address this issue.


> > Here, no overload would be matched, because the `type` in the
> > overloads is templated. To solve this problem I modified Boost.Move to
> > add two member functions - `copy()` and `move()` - which forward to
> > the rv const& and rv& conversion operators. With these, the above
> > example can be rewritten as:
> >     //In both C++11 and C++03 the `CopyableAndMovableFunctor`
> > //will be moved or copied into the thread as appropriate.
> > boost::thread(CopyableAndMovableFunctor().move()),
> > boost::thread(CopyableAndMovableFunctor().copy())
> > I can imagine the names `copy` and `move` being quite common, so this
> > might cause unfortunate conflicts. Is this an acceptable change to
> > Boost.Move? Is there another way to get the functionality that does
> > not require this?
> >
>
> I don't like this too much. Why doesn't boost::move works in this case?
> Could you give an example on which .copy() is needed?
>
> I don't know if this point will prevent the adoption of Boost.Move by
> Boost.Thread. I will comeback when I do some trials.
>

See if the aforementioned solution I gave is acceptable.

[...]

> > 3) Not using has_move_emulation_enabledI did not notice the
> > `has_move_emulation_enabled` metafunction, and instead used
> > `is_convertible!(T&, boost::rv!(T))`. This may mean that some
> > situations that `has_move_emulation_enabled` accounts for will not be
> > accounted for by the code in the patch. I have not yet evaluated
> > whether simply replacing all the occurrences will work. Any thoughts?
> >
>
> I have no idea. I suspect that I will need to look for this in Boost.Move.
>

[I like the D-style template argument delimiters!]

has_move_emulation_enabled<T> should be identical to boost::is_convertible<
T&, boost::rv<T>& >, no? At least, I believe it was in previous versions of
Boost.Move...

I guess I should actually skim through the documentation and see what, if
anything, has changed in Boost.Move!

- Jeff

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Vicente Botet
Le 09/01/12 22:31, Jeffrey Lee Hellrung, Jr. a écrit :

> On Mon, Jan 9, 2012 at 7:29 AM, Vicente Botet<[hidden email]>wrote:
>
>> Mankarse wrote
>>> On Thu, 05 Jan 2012 07:00:27 +0100, Vicente J. Botet Escriba
>>> wrote:>Mankarse please, could you post here the patch for the
>>> boost.thread>class so that I can start the integration of what you
>>> have already done?
>>> I have attached a patch to the ticket
>>> (https://svn.boost.org/trac/boost/ticket/6194). This patch is fairly
>>> comprehensive in terms of code, but it does not include any tests or
>>> documentation. In the course of writing the patch, I created some
>>> simple tests, but I have not yet translated these into Boost.Test
>>> testcases, or included them in the patch.
>>>
>> Thanks Evan,
>>
>> I will try to merge it on my own repository and see how it behaves. I have
>> added some tests to check the move semantics. They are in directory
>> libs/thread/test/threads and libs/thread/test/sync. Maybe you could check
>> them and suggest for others.
>>
>> BTw,
>>
>> Should't the following
>>             thread_data(BOOST_RV_REF(F) f_):
>>                                 f(boost::move(f_))
>
>> use forward instead of move?
>>             thread_data(BOOST_RV_REF(F) f_):
>>                                 f(boost::forward<F>(f_))
>>
> I don't think so. I haven't brushed up on the latest changes to Boost.Move
> since being officially added to Boost, but prior incarnations paired
> boost::forward with BOOST_FWD_REF. I think boost::move is correct when
> using BOOST_RV_REF.
>
>
>> Why the following (both) overloads are needed?
>>
>>             thread_data(BOOST_RV_REF(F) f_);
>>             thread_data(F const&  f_);
>>
> I don't know the context here, but I believe, in C++03, this would:
> capture-and-copy lvalues and rvalues; and capture-and-move explicitly
> created emulated rvalue references. Thus, true rvalues are captured as
> lvalues :( If you don't want to compromise, you could use a different set
> of overloads in C++03 and C++11:
>
> #ifndef BOOST_NO_RVALUE_REFERENCES
> template<  class F>  thread_data(F&&  f); // and use boost::forward<F>(f)
> #else // #ifndef BOOST_NO_RVALUE_REFERENCES
> template<  class F>  thread_data(F f); // and use boost::move(f)
> template<  class F>  thread_data(boost::rv<F>&  f); // and use boost::move(f)
> #endif // #ifndef BOOST_NO_RVALUE_REFERENCES
>
> I haven't tested this, but it *should* capture-and-move both true rvalues
> and emulated rvalue references in C++03. And if it's an emulated rvalue
> reference, the second overload requires 1 fewer move construction.
I have not used Boost.Move yet. I will take this in account while
applying the patch.

>
>>> I have tested the basic functionality on a number of compilers, but
>>> there are probably still some bugs in the patched code.
>>> The patch contains a number of known problems that may be worth
>>> discussing:1) Explicit move() and copy() calls:boost::thread and
>>> boost::packaged_task both have templated constructors that take an
>>> rvalue reference to a functor (which they move or copy as
>>> appropriate). Templates parameters cannot be deduced to a type that is
>>> only reachable through an conversion operator. Template constructors
>>> can also never be explicitly instansiated. (As far as I know.) This
>>> means that the overloads that are used in the definitions of
>>> BOOST_COPYABLE_AND_MOVABLE types (BOOST_RV_REF(type),
>>> BOOST_CATCH_CONST_RLVALUE(type), type&), are not matched for rvalue
>>> objects (in C++03 mode). Example:
>>>      //In C++11 the `CopyableAndMovableFunctor` would be    //moved
>>> into the thread.    //In C++03 this does not compile.
>>> boost::thread((CopyableAndMovableFunctor()))
>>>
>> Hrr, this is really a bad new. Could you create a Boost.Move ticket if the
>> limitation is not documented and a ticket doesn't exists already for this
>> issue?
>>
> It's a known limitation in Boost.Move, AFAIK. The above change should
> address this issue.
>
Could you point where in the doc it is described or to the Trac ticket?

>>> Here, no overload would be matched, because the `type` in the
>>> overloads is templated. To solve this problem I modified Boost.Move to
>>> add two member functions - `copy()` and `move()` - which forward to
>>> the rv const&  and rv&  conversion operators. With these, the above
>>> example can be rewritten as:
>>>      //In both C++11 and C++03 the `CopyableAndMovableFunctor`
>>> //will be moved or copied into the thread as appropriate.
>>> boost::thread(CopyableAndMovableFunctor().move()),
>>> boost::thread(CopyableAndMovableFunctor().copy())
>>> I can imagine the names `copy` and `move` being quite common, so this
>>> might cause unfortunate conflicts. Is this an acceptable change to
>>> Boost.Move? Is there another way to get the functionality that does
>>> not require this?
>>>
>> I don't like this too much. Why doesn't boost::move works in this case?
>> Could you give an example on which .copy() is needed?
>>
>> I don't know if this point will prevent the adoption of Boost.Move by
>> Boost.Thread. I will comeback when I do some trials.
>>
> See if the aforementioned solution I gave is acceptable.
It is acceptable if the user interface doesn't changes.

Thanks for all your comments,
Vicente


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Jeffrey Lee Hellrung, Jr.-2
On Mon, Jan 9, 2012 at 2:17 PM, Vicente J. Botet Escriba <
[hidden email]> wrote:
[...]

>
>>  I have tested the basic functionality on a number of compilers, but
>>>> there are probably still some bugs in the patched code.
>>>> The patch contains a number of known problems that may be worth
>>>> discussing:1) Explicit move() and copy() calls:boost::thread and
>>>> boost::packaged_task both have templated constructors that take an
>>>> rvalue reference to a functor (which they move or copy as
>>>> appropriate). Templates parameters cannot be deduced to a type that is
>>>> only reachable through an conversion operator. Template constructors
>>>> can also never be explicitly instansiated. (As far as I know.) This
>>>> means that the overloads that are used in the definitions of
>>>> BOOST_COPYABLE_AND_MOVABLE types (BOOST_RV_REF(type),
>>>> BOOST_CATCH_CONST_RLVALUE(**type), type&), are not matched for rvalue
>>>> objects (in C++03 mode). Example:
>>>>     //In C++11 the `CopyableAndMovableFunctor` would be    //moved
>>>> into the thread.    //In C++03 this does not compile.
>>>> boost::thread((**CopyableAndMovableFunctor()))
>>>>
>>>>  Hrr, this is really a bad new. Could you create a Boost.Move ticket if
>>> the
>>> limitation is not documented and a ticket doesn't exists already for this
>>> issue?
>>>
>>>  It's a known limitation in Boost.Move, AFAIK. The above change should
>> address this issue.
>>
>>  Could you point where in the doc it is described or to the Trac ticket?
>

Looking through the documentation, in short, no I can't find any mention of
this limitation. Let me see if I understand it correctly, though. One has
the following constructor overloads:

template< class F > thread(BOOST_RV_REF( F ) f);
template< class F > thread(BOOST_CATCH_CONST_RVALUE( F ) f); // what is
BOOST_CATCH_CONST_RVALUE?
template< class F > thread(F& f);

and wishes

thread((CopyableAndMovableFunctor()));

to, ideally, bind the temporary functor to an emulated rvalue reference.
But I gather the above won't even compile since 1) the template parameter F
cannot be deduced in the first 2 constructor overloads (the compiler won't
apply conversions); and 2) a temporary cannot bind to the F& overload.

If the above is all accurate, then I wonder where the above 3 constructor
overloads came from in the first place...?

- Jeff

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Mankarse
>Should't the following
>            thread_data(BOOST_RV_REF(F) f_):
>                f(boost::move(f_))
>
>use forward instead of move?
>            thread_data(BOOST_RV_REF(F) f_):
>                f(boost::forward<F>(f_))
>
>Why the following (both) overloads are needed?
>
>            thread_data(BOOST_RV_REF(F) f_);
>            thread_data(F const& f_);
>

When I submitted the patch, I thought that my handling of the forwarding was a
bit awkward. As I will  describe later in this email, I have submitted a new
patch which uses the technique suggested by Jeffrey Lee Hellrung. That said,
this discussion about the `thread_data` constructors is mostly irrelevant.
`thread_data` is an internal type, and the code in the `thread` constructors
already performs the lvalue/rvalue matching and calls the appropriate
`thread_data` constructor.

>Hrr, this is really a bad new. Could you create a Boost.Move ticket if the
>limitation is not documented and a ticket doesn't exists already for this
>issue?

As the issue has been resolved by an alternative technique, this is probably not
necessary.

>Could you try to run the test on trunk related to move semantics with your
>version?

I will do this later tonight.

>Could you analyze in more detail the impact of using
>has_move_emulation_enabled?

I investigated further, and while I still do not fully understand the impact, it
seems that it is harmless to use it in the particular case where I am using it.
In the name of consistency, I have changed the code to use
has_move_emulation_enabled in version 1 of the patch.

>> Why the following (both) overloads are needed?
>>
>>            thread_data(BOOST_RV_REF(F) f_);
>>            thread_data(F const& f_);
>
>I don't know the context here, but I believe, in C++03, this would:
>capture-and-copy lvalues and rvalues; and capture-and-move explicitly
>created emulated rvalue references. Thus, true rvalues are captured as
>lvalues :(

`thread_data` is an internal class. Its constructors are only ever called with
explicitly created rvalues (which are forwarded from the rvalues that are
matched in the constructor of `thread`), or lvalues. Thus the above behaviour is
not problematic.

>If you don't want to compromise, you could use a different set
>of overloads in C++03 and C++11:
> [snip]
>I haven't tested this, but it *should* capture-and-move both true rvalues
>and emulated rvalue references in C++03. And if it's an emulated rvalue
>reference, the second overload requires 1 fewer move construction.

Thankyou for the idea!

>It's a known limitation in Boost.Move, AFAIK. The above change should
>address this issue.

It does. I have submitted a new version of the patch that uses those overloads
in the constructors of `thread` and `packaged_task`.

>>[snip... explicit move() and copy() ...]
>
> I don't like this too much. Why doesn't boost::move works in this case?
> Could you give an example on which .copy() is needed?

boost::move will not work on rvalues. `.copy()` is needed to allow a constructor
to be matched from a const rvalue.

>> I don't know if this point will prevent the adoption of Boost.Move by
>> Boost.Thread. I will comeback when I do some trials.
>See if the aforementioned solution I gave is acceptable.
It is. Thankyou!

>[I like the D-style template argument delimiters!]
I have no confidence in my mail client whatsoever (you can see what it did to
the formatting of my message), and I was trying to avoid the template being
argument delimiters being seen as HTML tags and stripped. Note also - there was
an error in my original posting, what I meant was
`is_convertible!(T&, boost::rv!(T)&)`.

>has_move_emulation_enabled<T> should be identical to
>boost::is_convertible<T&, boost::rv<T>& >, no?

It isn't identical, but in the particular situation in which it is being used, I
believe it has the same effect.

>If the above is all accurate, then I wonder where the above 3 constructor
>overloads came from in the first place...?

The above is all accurate. The 3 constructor overloads originally came from the
constructors that all BOOST_COPYABLE_AND_MOVABLE types have. I thought that
using the same overloads would also be appropriate for a template. I was wrong
(for the reasons that you gave).

-- Evan Wallace


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
12