[PATCH] D17951: Implement is_always_lock_free

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 16 10:32:42 PDT 2016


EricWF added inline comments.

================
Comment at: include/atomic:859
@@ +858,3 @@
+template <> _LIBCPP_CONSTEXPR bool __libcpp_always_lock_free<char32_t> = 2 == ATOMIC_CHAR32_T_LOCK_FREE;
+template <> _LIBCPP_CONSTEXPR bool __libcpp_always_lock_free<wchar_t> = 2 == ATOMIC_WCHAR_T_LOCK_FREE;
+template <> _LIBCPP_CONSTEXPR bool __libcpp_always_lock_free<short> = 2 == ATOMIC_SHORT_LOCK_FREE;
----------------
jfb wrote:
> EricWF wrote:
> > jfb wrote:
> > > bcraig wrote:
> > > > Do we need to support configurations were wchar_t is a typedef to an integer type, or is that an abomination too painful to deal with?
> > > > 
> > > > I have no idea if the rest of libcxx attempts to deal with wchar_t typedefs.
> > > Do you have examples of such platforms? The standard is pretty clear that they're wrong:
> > > 
> > > >  [basic.fundamental]
> > > >
> > > > Type `wchar_t` is a distinct type whose values can represent distinct codes for all members of the largest extended character set specified among the supported locales. Type `wchar_t` shall have the same size, signedness, and alignment requirements as one of the other integral types, called its *underlying type*.
> > > 
> > > I'll defer to @mclow.lists on whether we can about such implementations or not.
> > I don't think we have to deal with such typedefs for `wchar_t` but we do have to deal with non-sense typedefs (that we unfortunately supply) for `char16_t` and `char32_t` in C++03 mode with GCC. 
> > 
> > And our <atomic> implementation needs to compile in C++03 (Sorry, I did that).
> `__cpp_lib_atomic_is_always_lock_free` takes care of it: it's only defined in C++17 and later so `<atomic>` should compile just fine.
> 
> Is there a test that would catch C++03 breakage?
Ah I didn't realize that. We have a C++03 bot that will catch any breakage but if your only exposing this in C++17 then we are fine.

================
Comment at: include/atomic:898
@@ -834,1 +897,3 @@
 
+#if defined(__cpp_lib_atomic_is_always_lock_free)
+# if defined(_LIBCPP_LOCK_FREE_IS_SIZE_BASED)
----------------
jfb wrote:
> EricWF wrote:
> > @mclow.lists I'm fine exposing this typedef retroactively in C++11 and earlier. Do you have any objections to that?
> Which typedef do you mean? Looks like phabricator may have tagged the wrong line.
Sorry I haven't had my coffee yet :-S. I meant the "is_always_lock_free" variable. IDK why  I said typedef. However I missed that this is already only exposed in C++17 and beyond. That's even better.

================
Comment at: test/std/atomics/atomics.lockfree/lockfree.pass.cpp:66
@@ +65,3 @@
+
+// structs and unions can't be defined in the template invocation.
+// Work around this with a typedef.
----------------
This needs to be guarded by `#if TEST_STD_VER > 14`. The macro lives in `test_macros.h`.


http://reviews.llvm.org/D17951





More information about the cfe-commits mailing list