rupprecht added a comment. In D150639#4346649 <https://reviews.llvm.org/D150639#4346649>, @JDevlieghere wrote:
> In D150639#4346009 <https://reviews.llvm.org/D150639#4346009>, @rupprecht > wrote: > >> +1 to being surprised this is not already the case >> >> Some other places should be updated after this, e.g. >> lldb/source/Symbol/SymbolFile.cpp also has a use that can be trivially >> updated: >> >> #if defined(LLDB_CONFIGURATION_DEBUG) >> // We assert that we have to module lock by trying to acquire the lock >> from a >> // different thread. Note that we must abort if the result is true to >> // guarantee correctness. >> assert(std::async( >> std::launch::async, >> [this] { >> return this->GetModuleMutex().try_lock(); >> }).get() == false && >> "Module is not locked"); >> #endif >> >> -> just wrap it in lldbassert > > This code actually has a comment right above it explaining that it's too > expensive to do in release builds, so this isn't a good candidate for > `lldbassert`. But this logic can be improved: it duplicates checking the > `LLDB_CONFIGURATION_DEBUG` in both `ASSERT_MODULE_LOCK` and > `AssertModuleLock` and should use `NDEBUG` too. I'll address that in a > separate patch. WDYT about just replacing `LLDB_CONFIGURATION_DEBUG` with LLVM's `EXPENSIVE_CHECKS` in this case? >> In D150639#4344695 <https://reviews.llvm.org/D150639#4344695>, @JDevlieghere >> wrote: >> >>> This patch also uses `__FILE_NAME__` (Clang-specific extension that >>> functions similar to __FILE__ but only renders the last path component (the >>> filename) instead of an invocation dependent full path to that file.) when >>> building with clang. >> >> Can you put this comment in `LLDBAssert.h` itself? > > Good idea. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150639/new/ https://reviews.llvm.org/D150639 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
