On Mon, May 20, 2013 at 5:55 PM, Howard Hinnant <span dir="ltr"><<a href="mailto:hhinnant@apple.com" target="_blank">hhinnant@apple.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Please review:<br>
<br>
I've enclosed a proposed patch for fixing <atomic>.  This patch changes us from using _Atomic(T) for the stored type, to just T, but aligned to _Atomic(T).  It also switches from the __c11_atomic intrinsics to the gcc __atomic intrinsics.  This change exposed several silly bugs in the test suite.  But most importantly it is meant to allow std::atomic to be instantiated with trivially copyable class types.<br>

<br>
The enclosed patch passes all (modified) tests on Apple's Mountain Lion except one:<br>
<br>
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics<br>
passed 2 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.fences<br>
passed 11 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.flag<br>
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.general<br>
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.lockfree<br>
passed 2 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.order<br>
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.syn<br>
passed 5 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.generic<br>
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations<br>
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.arith<br>
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.general<br>
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.pointer<br>
Undefined symbols for architecture x86_64:<br>
  "___atomic_is_lock_free", referenced from:<br>
      void test<A>() in atomic_is_lock_free-cA4Y5d.o<br>
ld: symbol(s) not found for architecture x86_64<br>
clang: error: linker command failed with exit code 1 (use -v to see invocation)<br>
/Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.req/atomic_is_lock_free.pass.cpp failed to compile<br>
Compile line was: xcrun clang++ -stdlib=libc++ -std=c++11 -I/Users/hhinnant/Development/temp_libcxx/test/support -I/Users/hhinnant/Development/temp_libcxx/include -L/Users/hhinnant/Development/temp_libcxx/lib -D_LIBCPP_STD_VER=13 -I/Users/hhinnant/Development/temp_libcxx/test/support -I/Users/hhinnant/Development/temp_libcxx/include -L/Users/hhinnant/Development/temp_libcxx/lib atomic_is_lock_free.pass.cpp<br>

failed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.req<br>
passed 22 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.req<br>
passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.templ<br>
****************************************************<br>
Results for /Users/hhinnant/Development/temp_libcxx/test/atomics:<br>
using Apple LLVM version 4.2 (clang-425.1.16) (based on LLVM 3.2svn)<br>
Target: x86_64-apple-darwin12.3.0<br>
Thread model: posix<br>
with -stdlib=libc++ -std=c++11 -I/Users/hhinnant/Development/temp_libcxx/test/support -I/Users/hhinnant/Development/temp_libcxx/include -L/Users/hhinnant/Development/temp_libcxx/lib -D_LIBCPP_STD_VER=13 -I/Users/hhinnant/Development/temp_libcxx/test/support -I/Users/hhinnant/Development/temp_libcxx/include -L/Users/hhinnant/Development/temp_libcxx/lib<br>

----------------------------------------------------<br>
sections without tests   : 0<br>
sections with failures   : 1<br>
sections without failures: 13<br>
                       +   ----<br>
total number of sections : 14<br>
----------------------------------------------------<br>
number of tests failed   : 1<br>
number of tests passed   : 51<br>
                       +   ----<br>
total number of tests    : 52<br>
****************************************************<br>
<br>
Comments welcomed.  We need more clang expertise here than libc++ expertise.</blockquote><div><br></div><div>The patch looks basically right. However, since you get the alignment of the inner object correct, you should pass 0 as the second parameter to __atomic_is_lock_free (that should remove your one remaining link error). Also, your placement-new expressions aren't robust against an overloaded operator& on _Tp.</div>
</div>