theraven added inline comments.
================
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
{
----------------
espositofulvio wrote:
> 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.
I *think* that it's correct, but it's worth compiling a program that has one
compilation unit using the old header and another using the new one and check
that it's able to interoperate correctly.
Also check whether this depends on the implementation of the condition
variable. On FreeBSD, the pthread types are all pointers (currently - we're
going to have some painful ABI breakage at some point when we move them to
being something that can live in shared memory). In glibc, they're structures.
I don't think that you'll end up with different padding in the ABI from
wrapping them in a class, but it's worth checking.
================
Comment at: include/mutex:182
@@ -181,2 +181,3 @@
#endif
-#include <sched.h>
+#ifndef _WIN32
+#include <support/pthread/mutex.hpp>
----------------
espositofulvio wrote:
> 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?
>
We'll probably end up with a set of #ifdef FreeBSD (or whatever) things, but
making sure that they're all in a single file will help. If you're doing
bring-up of a new platform, just having to set #define __LIBCXX_THREAD_API
__LIBCXX_PTHREAD, or __LIBCXX_C11_THREADS, (or Haiku threads, or whatever) in
one place makes life a bit easier. One of the annoyances with trying to port
asan was that the original developers used #ifdef __APPLE__ to mean 'is not
Linux' and 'is Darwin' in various places, so we needed to look at every single
change and determine whether they were shared between multiple non-GNU
platforms or whether they were specific to OS X / iOS.
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
