[PATCH] D11781: Refactored pthread usage in libcxx

[Fulvio Esposito via cfe-commits]

espositofulvio added a comment.

In http://reviews.llvm.org/D11781#227446, @EricWF wrote:

> This patch has a long way to go but it has also come a long way. Here are a couple of problems I see with it.
>
> 2. This patch adds a lot of headers. libc++ has historically tried to keep the number of headers to a minimum for the reason that filesystem operations are expensive and its cheaper to include a few big headers as opposed to many small ones.


I know but it was a request from reviewers so that platform specific decision were localized. I can obviously merge all of the support/mutex.h, support/thread.h and support/condition_variable.h in a single support/thread.h if that's the case.


================
Comment at: include/type_traits:222
@@ -221,3 +221,3 @@
 
-template <class _Tp>
-struct __identity { typedef _Tp type; };
+template <class T>
+struct __identity { typedef T type; };
----------------
EricWF wrote:
> This change seems to have snuck in.
Yes, and actually I'm not sure how it did as it's something I haven't touched. Unfortunately I don't have time before later next week to address all the issues.

================
Comment at: src/algorithm.cpp:51
@@ -50,3 +50,3 @@
 #ifndef _LIBCPP_HAS_NO_THREADS
-static pthread_mutex_t __rs_mut = PTHREAD_MUTEX_INITIALIZER;
+static mutex __rs_mut;
 #endif
----------------
EricWF wrote:
> I think this prevents __rs_mut from being initialized during constant initialization. (http://en.cppreference.com/w/cpp/language/constant_initialization)
I think this falls in the second case:

Static or thread-local object of class type that is initialized by a constructor call, if the constructor is constexpr and all constructor arguments (including implicit conversions) are constant expressions, and if the initializers in the constructor's initializer list and the brace-or-equal initializers of the class memebers only contain constant expressions.

but I was actually relying on the fact that we have constexpr, which now I understand could not be the case.

================
Comment at: src/memory.cpp:130
@@ -129,3 +129,1 @@
 static const std::size_t __sp_mut_count = 16;
-static pthread_mutex_t mut_back_imp[__sp_mut_count] =
-{
----------------
EricWF wrote:
> I have no idea what is going on here. Do you understand what this code was trying to do?
My understanding is that this is trying to init the array during constant initialization and later relying on the internal layout of std::mutex to be equal to a pthread_mutex_t to cast it and use it as a std::mutex.

from cppreference:
Static or thread-local object (not necessarily of class type), that is not initialized by a constructor call, if the object is value-initialized or if every expression in its initializer is a constant expression.

Again, I think my change should be ok as long as constexpr is available, but I could be wrong.


Repository:
  rL LLVM

http://reviews.llvm.org/D11781