[config] clang deleted functions bugged

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[config] clang deleted functions bugged

Vicente Botet
Hi,

Clang has a bug with deleted functions (see
http://stackoverflow.com/questions/7754564/clang-error-message-when-using-c0x-call-to-deleted-constructor-of).

Please, could we comment these two lines until clang fixes the bug

//#if !__has_feature(cxx_deleted_functions)
#  define BOOST_NO_CXX11_DELETED_FUNCTIONS
//#endif

Thanks,
Vicente

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

Re: [config] clang deleted functions bugged

Peter Dimov-2
Vicente J. Botet Escriba wrote:
> Hi,
>
> Clang has a bug with deleted functions (see
> http://stackoverflow.com/questions/7754564/clang-error-message-when-using-c0x-call-to-deleted-constructor-of).

This is not a clang bug. It was a bug in boost::shared_ptr, now fixed.


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

Re: [config] clang deleted functions bugged

Vicente Botet
Le 28/10/13 00:47, Peter Dimov a écrit :
> Vicente J. Botet Escriba wrote:
>> Hi,
>>
>> Clang has a bug with deleted functions (see
>> http://stackoverflow.com/questions/7754564/clang-error-message-when-using-c0x-call-to-deleted-constructor-of).
>
> This is not a clang bug. It was a bug in boost::shared_ptr, now fixed.
>
Maybe you are right, but there are other cases when the constructor is
deleted, that is when we use explicitly = delete.
In this case the clang compiler report an error similar to the error
reported by the preceding link.

bjam toolset=clang-3.4xl -l 300 variant=debug  -j6 ts_container
warning: mismatched versions of Boost.Build engine and core
warning: Boost.Build engine (bjam) is 2011.12.01
warning: Boost.Build core (at /Users/viboes/boost/trunk/tools/build/v2)
is 2013.05-svn
Performing configuration checks

     - lockfree boost::atomic_flag : yes (cached)
...patience...
...patience...
...found 1416 targets...
...updating 23 targets...
clang-darwin.compile.c++
../../../bin.v2/libs/thread/build/clang-darwin-3.4xl/debug/link-static/threading-multi/pthread/once.o
clang-darwin.compile.c++
../../../bin.v2/libs/thread/build/clang-darwin-3.4xl/debug/threading-multi/pthread/once.o
clang-darwin.compile.c++
../../../bin.v2/libs/thread/test/container__thread_vector_p_lib.test/clang-darwin-3.4xl/debug/threading-multi/threads/container/thread_vector_pass.o
In file included from threads/container/thread_vector_pass.cpp:8:
In file included from ../../../boost/thread/thread.hpp:12:
In file included from ../../../boost/thread/thread_only.hpp:17:
In file included from ../../../boost/thread/pthread/thread_data.hpp:10:
In file included from ../../../boost/thread/exceptions.hpp:20:
In file included from /usr/include/c++/v1/string:439:
In file included from /usr/include/c++/v1/algorithm:627:
/usr/include/c++/v1/memory:1641:31: error: call to deleted constructor
of 'boost::thread'
             ::new((void*)__p) _Up(_VSTD::forward<_Args>(__args)...);
                               ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/c++/v1/memory:1568:18: note: in instantiation of function
template specialization
'std::__1::allocator<boost::thread>::construct<boost::thread, const
boost::thread &>' requested here
             {__a.construct(__p, _VSTD::forward<_Args>(__args)...);}
                  ^
/usr/include/c++/v1/memory:1449:14: note: in instantiation of function
template specialization
'std::__1::allocator_traits<std::__1::allocator<boost::thread>
 >::__construct<boost::thread, const boost::thread &>' requested here
             {__construct(__has_construct<allocator_type, pointer,
_Args...>(),
              ^
/usr/include/c++/v1/memory:1531:17: note: in instantiation of function
template specialization
'std::__1::allocator_traits<std::__1::allocator<boost::thread>
 >::construct<boost::thread, const boost::thread &>' requested here
                 construct(__a, _VSTD::__to_raw_pointer(__end2-1),
_VSTD::move_if_noexcept(*--__end1));
                 ^
/usr/include/c++/v1/vector:839:21: note: in instantiation of function
template specialization
'std::__1::allocator_traits<std::__1::allocator<boost::thread>
 >::__construct_backward<boost::thread *>' requested here
     __alloc_traits::__construct_backward(this->__alloc(),
this->__begin_, this->__end_, __v.__begin_);
                     ^
/usr/include/c++/v1/vector:1488:9: note: in instantiation of member
function 'std::__1::vector<boost::thread,
std::__1::allocator<boost::thread> >::__swap_out_circular_buffer'
requested here
         __swap_out_circular_buffer(__v);
         ^
threads/container/thread_vector_pass.cpp:56:13: note: in instantiation
of member function 'std::__1::vector<boost::thread,
std::__1::allocator<boost::thread> >::reserve' requested here
     threads.reserve(10);
             ^
../../../boost/thread/detail/thread.hpp:165:33: note: function has been
explicitly marked deleted here
       BOOST_THREAD_MOVABLE_ONLY(thread)
                                 ^
../../../boost/thread/detail/move.hpp:240:28: note: expanded from macro
'BOOST_THREAD_MOVABLE_ONLY'
   BOOST_THREAD_NO_COPYABLE(TYPE) \
                            ^
../../../boost/thread/detail/delete.hpp:58:35: note: expanded from macro
'BOOST_THREAD_NO_COPYABLE'
     BOOST_THREAD_DELETE_COPY_CTOR(CLASS) \
                                   ^
../../../boost/thread/detail/delete.hpp:24:7: note: expanded from macro
'BOOST_THREAD_DELETE_COPY_CTOR'
       CLASS(CLASS const&) = delete; \
       ^
1 error generated.

The concerned code is (compile error in lines (1) and (2))

   typedef std::vector<boost::thread> thread_vector;
   {
     thread_vector threads;
     threads.reserve(10); // (1)
     for (int i = 0; i < 10; ++i)
     {
       boost::thread th(&increment_count);
       threads.push_back(std::move(th)); // (2)
     }
     join_all(threads);
   }

Boost.Thread defines a private constructor when
BOOST_NO_CXX11_DELETED_FUNCTIONS is defined and in this case there is no
compiler error.
Note that libc++ doesn't make use of =delete to implement movable only
classes, so the compiler bug is not visible when we push_back a
std::thread on a vector of std::threads.

clang-3.4xl is the latest from svn. The same behavior for clang-3.1 and 3.2

Best,
Vicente


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

Re: [config] clang deleted functions bugged

Jonathan Wakely-2
On 28 October 2013 00:12, Vicente J. Botet Escriba wrote:
>
> Boost.Thread defines a private constructor when
> BOOST_NO_CXX11_DELETED_FUNCTIONS is defined and in this case there is no
> compiler error.

That might be true, but that is not sufficient to conclude that clang
doesn't support deleted functions!

The stackoverflow question you linked to is years old and Howard's
answer is wrong (but Johannes's comments on that answer are correct,
and agree with what Peter already said.)

The stack trace shows that std::vector is trying to copy the element,
not move it.

My guess would be that boost::thread's move constructor is not
noexcept, so the libc++ std::vector refuses to use a move when
inserting, and falls back to a copy instead, which fails. That is the
correct, standard behaviour for std::vector. If my guess is correct
you should make the move constructor noexcept.

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

Re: [config] clang deleted functions bugged

Jonathan Wakely-2
On 28 October 2013 01:45, Jonathan Wakely wrote:

> On 28 October 2013 00:12, Vicente J. Botet Escriba wrote:
>>
>> Boost.Thread defines a private constructor when
>> BOOST_NO_CXX11_DELETED_FUNCTIONS is defined and in this case there is no
>> compiler error.
>
> That might be true, but that is not sufficient to conclude that clang
> doesn't support deleted functions!
>
> The stackoverflow question you linked to is years old and Howard's
> answer is wrong (but Johannes's comments on that answer are correct,
> and agree with what Peter already said.)
>
> The stack trace shows that std::vector is trying to copy the element,
> not move it.
>
> My guess would be that boost::thread's move constructor is not
> noexcept, so the libc++ std::vector refuses to use a move when
> inserting, and falls back to a copy instead, which fails. That is the
> correct, standard behaviour for std::vector. If my guess is correct
> you should make the move constructor noexcept.

Yep, here's your bug:

        thread(BOOST_THREAD_RV_REF(thread) x)
        {
            thread_info=BOOST_THREAD_RV(x).thread_info;
            BOOST_THREAD_RV(x).thread_info.reset();
        }

That is missing a BOOST_NOEXCEPT exception-specification.

Why not simply do x.thread_info.swap(thread_info) to avoid ref-count
updates in that constructor?

Also, there's a bogus copyright date in boost/thread/detail/thread.hpp:

// (C) Copyright 20011-2012 Vicente J. Botet Escriba

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

Re: [config] clang deleted functions bugged

Jonathan Wakely-2
On 28 October 2013 01:54, Jonathan Wakely wrote:
> On 28 October 2013 01:45, Jonathan Wakely wrote:
>>
>> My guess would be that boost::thread's move constructor is not
>> noexcept, so the libc++ std::vector refuses to use a move when
>> inserting, and falls back to a copy instead, which fails. That is the
>> correct, standard behaviour for std::vector.

Actually, the correct behaviour would be to fall back to the
possibly-throwing move if the type is non-copyable, so maybe there's a
problem with the libc++ std::vector, although I'd find that almost as
surprising as a bug in clang's deleted function support.

The boost::thread move ctor should still be noexcept anyway, and that
should solve the problem.

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

Re: [config] clang deleted functions bugged

Vicente Botet
In reply to this post by Jonathan Wakely-2
Le 28/10/13 02:45, Jonathan Wakely a écrit :

> On 28 October 2013 00:12, Vicente J. Botet Escriba wrote:
>> Boost.Thread defines a private constructor when
>> BOOST_NO_CXX11_DELETED_FUNCTIONS is defined and in this case there is no
>> compiler error.
> That might be true, but that is not sufficient to conclude that clang
> doesn't support deleted functions!
>
> The stackoverflow question you linked to is years old and Howard's
> answer is wrong (but Johannes's comments on that answer are correct,
> and agree with what Peter already said.)
>
> The stack trace shows that std::vector is trying to copy the element,
> not move it.
>
> My guess would be that boost::thread's move constructor is not
> noexcept, so the libc++ std::vector refuses to use a move when
> inserting, and falls back to a copy instead, which fails. That is the
> correct, standard behaviour for std::vector. If my guess is correct
> you should make the move constructor noexcept.
>
How is it that when I define the boost::thread constructor private the
compiler has no issues?
I will check for noexcept.

Best,
Vicente

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

Re: [config] clang deleted functions bugged

Vicente Botet
In reply to this post by Jonathan Wakely-2
Le 28/10/13 02:54, Jonathan Wakely a écrit :

> On 28 October 2013 01:45, Jonathan Wakely wrote:
>> On 28 October 2013 00:12, Vicente J. Botet Escriba wrote:
>>> Boost.Thread defines a private constructor when
>>> BOOST_NO_CXX11_DELETED_FUNCTIONS is defined and in this case there is no
>>> compiler error.
>> That might be true, but that is not sufficient to conclude that clang
>> doesn't support deleted functions!
>>
>> The stackoverflow question you linked to is years old and Howard's
>> answer is wrong (but Johannes's comments on that answer are correct,
>> and agree with what Peter already said.)
>>
>> The stack trace shows that std::vector is trying to copy the element,
>> not move it.
>>
>> My guess would be that boost::thread's move constructor is not
>> noexcept, so the libc++ std::vector refuses to use a move when
>> inserting, and falls back to a copy instead, which fails. That is the
>> correct, standard behaviour for std::vector. If my guess is correct
>> you should make the move constructor noexcept.
> Yep, here's your bug:
>
>          thread(BOOST_THREAD_RV_REF(thread) x)
>          {
>              thread_info=BOOST_THREAD_RV(x).thread_info;
>              BOOST_THREAD_RV(x).thread_info.reset();
>          }
>
> That is missing a BOOST_NOEXCEPT exception-specification.
You are right, I missed it. And adding it fixes the issue I had. Thanks
so much.
My apologies for the noise. Hopping others have learn something from you
insight.
> Why not simply do x.thread_info.swap(thread_info) to avoid ref-count
> updates in that constructor?
I will change it.
>
> Also, there's a bogus copyright date in boost/thread/detail/thread.hpp:
>
> // (C) Copyright 20011-2012 Vicente J. Botet Escriba
>
>
Thanks for catching this ;-)
Vicente

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

Re: [config] clang deleted functions bugged

Vicente Botet
Le 28/10/13 03:23, Vicente J. Botet Escriba a écrit :

> Le 28/10/13 02:54, Jonathan Wakely a écrit :
>> On 28 October 2013 01:45, Jonathan Wakely wrote:
>>> On 28 October 2013 00:12, Vicente J. Botet Escriba wrote:
>>>> Boost.Thread defines a private constructor when
>>>> BOOST_NO_CXX11_DELETED_FUNCTIONS is defined and in this case there
>>>> is no
>>>> compiler error.
>>> That might be true, but that is not sufficient to conclude that clang
>>> doesn't support deleted functions!
>>>
>>> The stackoverflow question you linked to is years old and Howard's
>>> answer is wrong (but Johannes's comments on that answer are correct,
>>> and agree with what Peter already said.)
>>>
>>> The stack trace shows that std::vector is trying to copy the element,
>>> not move it.
>>>
>>> My guess would be that boost::thread's move constructor is not
>>> noexcept, so the libc++ std::vector refuses to use a move when
>>> inserting, and falls back to a copy instead, which fails. That is the
>>> correct, standard behaviour for std::vector. If my guess is correct
>>> you should make the move constructor noexcept.
>> Yep, here's your bug:
>>
>>          thread(BOOST_THREAD_RV_REF(thread) x)
>>          {
>>              thread_info=BOOST_THREAD_RV(x).thread_info;
>>              BOOST_THREAD_RV(x).thread_info.reset();
>>          }
>>
>> That is missing a BOOST_NOEXCEPT exception-specification.
> You are right, I missed it. And adding it fixes the issue I had.
> Thanks so much.
> My apologies for the noise. Hopping others have learn something from
> you insight.
>> Why not simply do x.thread_info.swap(thread_info) to avoid ref-count
>> updates in that constructor?
> I will change it.
>>
>> Also, there's a bogus copyright date in boost/thread/detail/thread.hpp:
>>
>> // (C) Copyright 20011-2012 Vicente J. Botet Escriba
>>
>>
> Thanks for catching this ;-)
>
>
Done, https://svn.boost.org/trac/boost/changeset/86493
Vicente

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