rmaprath added inline comments.

================
Comment at: src/fallback_malloc.cpp:37
 class mutexor {
 public:
----------------
EricWF wrote:
> rmaprath wrote:
> > EricWF wrote:
> > > Can't we replace this class with `std::mutex` directly?
> > Again, I should've included more context to the patch :(
> > 
> > The `mutexor` here is supposed to degrade to a nop when threading is 
> > disabled. I think we cannot use `std::mutex` without threading support.
> > 
> > Will update the patch with more context.
> We cannot use `std::mutex` without threading support but I would still rather 
> use it.
> 
> I would `typedef std::lock_guard<std::mutex> mutexor` when threading is 
> enabled and otherwise I would just `#ifdef` all usages away.
> 
> Also please add `_LIBCPP_SAFE_STATIC` to the heap_mutex declaration.
Hmmm, using `std::mutex` here breaks the shared library builds of `libcxxabi`. 
The problem is that methods like `std::mutex::lock()` and `~std::mutex()` are 
only declared in the `<mutex>` header, with the implementation going into the 
dylib.

I was just about to update the patch because it worked for all my other 
configurations, but those configurations are either static builds or ones that 
suppress the `-Wl, -z defs` linker option passed to the shared library builds 
by default (this is an inherited option).

Perhaps it's time to get rid of `-Wl, -z defs` for all the `libcxxabi` 
configurations? That makes sense if `libcxxabi` is inherently dependent on 
`libcxx`.


https://reviews.llvm.org/D27575



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to