[Lldb-commits] [PATCH] D150639: [lldb] Define lldbassert based on NDEBUG instead of LLDB_CONFIGURATION_DEBUG

[Jonas Devlieghere via Phabricator via lldb-commits]

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