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