[PATCH] D11781: Refactored pthread usage in libcxx

[Fulvio Esposito via cfe-commits]

espositofulvio added inline comments.

================
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+    defined(__NetBSD__) || \
----------------
theraven wrote:
> 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.
Given that it's something I suggested in a comment on the first revision, I was happy to try it out. It was also a quick change. Unfortunately it brakes libcxxabi build (at least the way I've done it), so I'm happy to make the change if @jroelofs finds another way 


Repository:
  rL LLVM

http://reviews.llvm.org/D11781