[libcxx-commits] [PATCH] D68480: Implementation of C++20's P1135R6 for libcxx

[Olivier Giroux via Phabricator via libcxx-commits]

__simt__ marked 10 inline comments as done.
__simt__ added a comment.

Closed some of the comments that have been addressed.



================
Comment at: include/__threading_support:566
+
+using __libcpp_platform_wait_t = int;
+
----------------
zoecarver wrote:
> If this file is used before C++11, this should be a typedef.
I think this should be C++11 or later, will need an ifdef here.


================
Comment at: include/__threading_support:587
+
+struct alignas(64) __libcpp_contention_t {
+#if defined(_LIBCPP_HAS_PLATFORM_WAIT)
----------------
zoecarver wrote:
> zoecarver wrote:
> > Are you sure that `64` is always the correct alignment here? What if `__libcpp_platform_wait_t` is only 8 bits, or what if its 128 bits? The same could probably go for `ptrdiff_t` (but, I'm not sure about the exact requirements for it). 
> Additionally, `alignas` won't work in C++03. You can use `aligned_storage` if you need. 
I don't think we'll support these C++20 features in C++03, but probably will do in C++11. That needs some more testing.


================
Comment at: include/atomic:633
 
+extern "C" int memcmp( const void * ptr1, const void * ptr2, size_t num );
+
----------------
zoecarver wrote:
> Why can't you just use `std::memcmp`?
I'd like to but I didn't want to pull the headers that define it into this one. 


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68480/new/

https://reviews.llvm.org/D68480