espositofulvio 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
 {
----------------
theraven wrote:
> 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.
> 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

Unfortunately this isn't going to work, some simbols are now defined in 
__libcxx_support and not in std (they are just made available through using), 
so an object file that includes the old header won't link against the new 
library (unreferenced symbols). The alternative is to create inline forwarding 
methods.





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

Reply via email to