[PATCH] D11781: Refactored pthread usage in libcxx

[David Chisnall via cfe-commits]

theraven added inline comments.

================
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+    defined(__NetBSD__) || \
----------------
espositofulvio wrote:
> jroelofs wrote:
> > espositofulvio wrote:
> > > jroelofs wrote:
> > > > jroelofs wrote:
> > > > > @espositofulvio: @ed meant this:
> > > > > 
> > > > > ```
> > > > > #ifndef _WIN32
> > > > > #  include <unistd.h>
> > > > > #  if _POSIX_THREADS > 0
> > > > > ...
> > > > > #  endif
> > > > > #endif
> > > > > ```
> > > > > 
> > > > > Which //is// the correct way to test for this.
> > > > That being said, there have been discussions before about whether or not we should #include <unistd.h> in <__config>, with the conclusion being that we shouldn't.
> > > > 
> > > > It would be better if this were a CMake configure-time check that sets _LIBCPP_THREAD_API, rather than these build-time guards.
> > > Tried adding that as configure time checks, but then libcxxabi fails to compile because of the guard in __config to check that _LIBCPP_THREAD_API has beed defined when _LIBCPP_HAS_NO_THREADS is not. 
> > > 
> > > As a side note: Is Windows the only OS which hasn't got unistd.h?
> > > Tried adding that as configure time checks...
> > 
> > Can you put the patch for that up on gist.github.com, or a pastebin?... I'll take a look.
> > 
> > > As a side note: Is Windows the only OS which hasn't got unistd.h?
> > 
> > For the platforms libcxx currently builds on, yes.
> > Can you put the patch for that up on gist.github.com, or a pastebin?... I'll take a look.
> 
> It's here https://gist.github.com/espositofulvio/eac2fb08acf2e430c516
I'm sorry to have triggered this bikeshed.  Given that, of the currently supported platforms, Windows is the only one where we want to use threads but don't want to use PTHREADS, I think it's fine to have this check be !WIN32.  The important thing is having the check *in one place* so that the person who wants to add the *second* non-pthread threading implementation doesn't have a load of different places to look: they can just change it in `__config` and then chase all of the compile failures.


Repository:
  rL LLVM

http://reviews.llvm.org/D11781