[cfe-dev] libc++: help diagnosing the following std::atomic compile error

[Howard Hinnant]

On May 20, 2013, at 4:44 PM, Richard Smith <richard at metafoo.co.uk> wrote:

> On Mon, May 20, 2013 at 1:02 PM, Howard Hinnant <hhinnant at apple.com> wrote:
> On May 20, 2013, at 3:29 PM, Rich E <reakinator at gmail.com> wrote:
> 
> > Hello all,
> >
> > I am trying to help along the recently added Boost.Lockfree (1) library utilize libc++'s std::atomic, as it is currently using Boost.Atomic, and an emulated, locking implementation at that.  <atomic> is currently used for this library for modern gcc and msvc compilers, but not yet clang / libc++.
> >
> >
> > (note: to enable <atomic> for boost::lockfree, first apply this patch: https://svn.boost.org/trac/boost/attachment/ticket/8593/lockfree_atomic_libcpp.patch )
> >
> > The problem can be seen in the compile errors for this simple program:
> >
> > #include "boost/lockfree/queue.hpp"
> >
> > int main(int argc, const char * argv[])
> > {
> >   boost::lockfree::queue<int> q( 1 );
> >   return 0;
> > }
> >
> > The diagnostic is:
> >
> > In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/memory:606:
> > /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/atomic:623:58: error: no viable conversion from 'boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void_, boost::parameter::void_, boost::parameter::void_>::node>' to '_Atomic(boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void_, boost::parameter::void_, boost::parameter::void_>::node>)'
> >     _LIBCPP_CONSTEXPR __atomic_base(_Tp __d) _NOEXCEPT : __a_(__d) {}
> >                                                          ^    ~~~
> > /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/atomic:727:51: note: in instantiation of member function 'std::__1::__atomic_base<boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void_, boost::parameter::void_, boost::parameter::void_>::node>, false>::__atomic_base' requested here
> >     _LIBCPP_CONSTEXPR atomic(_Tp __d) _NOEXCEPT : __base(__d) {}
> >                                                   ^
> > ../../../../../cinder-dev/boost/boost/lockfree/queue.hpp:192:9: note: in instantiation of member function 'std::__1::atomic<boost::lockfree::detail::tagged_ptr<boost::lockfree::queue<int, boost::parameter::void_, boost::parameter::void_, boost::parameter::void_>::node> >::atomic' requested here
> >         head_(tagged_node_handle(0, 0)),
> >         ^
> > /Users/richeakin/code/cinder/audio-rewrite/audio2/test/BasicTest/xcode/../src/BasicTestApp.cpp:75:30: note: in instantiation of member function 'boost::lockfree::queue<int, boost::parameter::void_, boost::parameter::void_, boost::parameter::void_>::queue' requested here
> >         boost::lockfree::queue<int> q( 1 );
> >                                     ^
> > ../../../../../cinder-dev/boost/boost/lockfree/detail/tagged_ptr_dcas.hpp:112:5: note: candidate function
> >     operator bool(void) const
> >     ^
> >
> >
> > My understanding is (please correct me if wrong) that _Atomic(_Tp) is a directive to the compiler, which is replaced with atomic the true atomic instructions for each given architecture. As such, it doesn't know how to replace boost::lockfree::detail::tagged_ptr<...> with size_t, or whatever other atomic value lockfree expects. But, this is where my understanding of this sort of template metaprogramming reaches its end, I just can't tell why this trips up with libc++ but not with vc11.
> >
> > Can anyone see where the translation falls short, or have suggestions on how to proceeed?
> 
> > This looks like a clang bug to me.  _Atomic seems to not be set up to deal with non-integral types.
> 
> I don't think this is clearly a clang bug. We support _Atomic(T) in C++ mode as an extension, but that extension currently does not extend to allowing an _Atomic(T) to be initialized as if it were a T, if T is of class type. We can only reasonably support this when T's corresponding constructor is trivial, but I believe that's all you actually need here, right? (We don't want to allow calling member functions on an object of type T which is wrapped within an _Atomic(...)).

C++11 requires only that T be trivially copyable.  So it seems like this should work:

#include <atomic>
#include <type_traits>

struct A
{
    int i_;

    A(int i) : i_(i) {}
};

static_assert(std::is_trivially_copyable<A>::value, "");

int
main()
{
    std::atomic<A> q(A(1));
}

The _Atomic type specifier was added to libc++ by this commit:

---------------------------
r146865 | theraven | 2011-12-19 06:44:20 -0500 (Mon, 19 Dec 2011) | 7 lines

Some fixes to <atomic> operations to explicitly use atomic types and operations.

The integral types now work with clang trunk (if you remove the guard), although we're still missing an intrinsic for initialising atomics (needed for C1x too).

Howard: Please review.
---------------------------

David Chisnall and I had a private conversation about this addition:


On Dec 19, 2011, at 1:53 PM, David Chisnall <csdavec at swan.ac.uk> wrote:

> On 19 Dec 2011, at 18:33, Howard Hinnant wrote:

>> I'm not fully understanding why we need _Atomic(T).  If a mutex is needed for the type, I'm thinking that would go into compiler_rt. And I'm thinking that the only characteristic that would drive the need to do this is sizeof(T).
>> 
>> Does clang have any documentation on _Atomic?
> 
> It's part of the C1x spec and is supported by clang in C++ mode as an extension.
> 
> Specifically, _Atomic(T) is not required by the spec to have the same underlying representation as T (so operations on it can be guarded by an associated mutex or something lighter -  e.g. some architectures only permit atomic operations on small values - e.g. only on 1 bit - and so larger values can be guarded by a flag in this bit rather than a full mutex).  That's also the rationale behind the atomic_flag stuff in both C and C++ - any architecture that supports the atomics stuff should support 1-bit atomic operations and these can be used to implement everything else by implementing a lightweight mutex.

I still don't fully understand why we need _Atomic in the C++11 <atomic>.  However whether we do or not, I don't really care.  All I care about is that <atomic> works.  <atomic> can't be implemented in straight C++.  It requires compiler support.  I need a clang expert to advise me on what the libc++ <atomic> should look like to just work.

In Oct. 2010 I wrote this as a proposal for how it should work and had an <atomic> written to this spec:

http://libcxx.llvm.org/atomic_design_a.html

In late 2011 David Chisnall did the clang work to get <atomic> to work and updated <atomic> accordingly.  In Apr 2012 you switched <atomic> from __atomic_* builtins to __c11_atomic_* builtins.

<shrug>  The libc++ <atomic> appears to work only for scalar types, and the C++11 spec says it should work for all trivially copyable types.  I know of no changes I can make to libc++ at this time to fix that.  Clang experts please advise.

Howard