JDevlieghere added a comment.
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.
> 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.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150639/new/
https://reviews.llvm.org/D150639
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
