espositofulvio added inline comments.
================
Comment at: include/__mutex_base:19
@@ +18,3 @@
+#ifndef _WIN32
+#include <support/pthread/mutex.hpp>
+#endif
----------------
jroelofs wrote:
> I think it might make sense to create a file: `<support/mutex.h>` where the
> contents are just:
>
> ```
> #ifndef _WIN32
> #include <support/pthread/mutex.hpp>
> #endif
> ```
>
> And include that file here so that the platform decision of "which mutex
> implementation should we use" doesn't have these #ifndef guards spread
> everywhere.
>
> Maybe you could also add some defines & guards to enforce the idea that
> `<support/pthread/mutex.hpp>` is never included directly, but rather only via
> `<support/mutex.h>`.
>
> Same should go for thread.hpp, too.
A very good idea indeed. I'll make the change and update the patch.
================
Comment at: include/__mutex_base:246
@@ -266,3 +245,3 @@
-class _LIBCPP_TYPE_VIS condition_variable
+class _LIBCPP_TYPE_VIS condition_variable : private
__libcxx_support::condition_variable
{
----------------
theraven wrote:
> Does this change the ABI for a mutex on *NIX platforms? We can't change the
> class layout for existing platforms (though we can indirect things via
> typedefs).
My hunch is that it doesn't, std::condition_variable has no data member now and
the first base class (__libcxx_support::condition_variable) contains only
pthread_cond_t which will be effectively laid out at the starting address of
the object, as it was previously for std::condition_variable,. I will check
this out later in the evening though.
================
Comment at: include/mutex:182
@@ -181,2 +181,3 @@
#endif
-#include <sched.h>
+#ifndef _WIN32
+#include <support/pthread/mutex.hpp>
----------------
theraven wrote:
> As above, there should probably be in a cross-platform support file that
> includes these. In particular, not-win32 is not the correct condition for
> uses-pthreads. We should probably have something in __config that determines
> the thread library to use. It would be quite nice to have a C11 thread back
> end, for example (though most implementations currently wrap pthreads).
In this case having a series of #ifdef __FreeBSD__ (or _WIN32, etc.) doesn't
quite cut it as we want to be able to select C11 or pthread on most of them and
on Windows one day it might even be C11, pthread or the win32 api. I guess the
alternative is to provide a cmake variable that default to something different
on each platform?
Repository:
rL LLVM
http://reviews.llvm.org/D11781
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
