[Lldb-commits] [PATCH] D107659: [nfc] [lldb] Assertions for D106270 - [DWARF5] Fix offset check when using .debug_names

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D107659#2934245 <https://reviews.llvm.org/D107659#2934245>, @jankratochvil wrote:

> Thanks for checking it but ...
>
> In D107659#2931836 <https://reviews.llvm.org/D107659#2931836>, @clayborg wrote:
>
>> Can we not just grab the skeleton unit when/if needed instead of asserting in many places?
>
> ... I am not sure @clayborg likes the assertions.

I don't mind lldbassert() calls as they are not in release builds, so it is nice to catch things during debug build testing. I prefer that asserts are not used when the program would knowingly crash immediately after the assert. I don't like asserts in code like:

  assert(ptr)
  ptr->foo();

This code will crash both when asserts are enabled and also when they aren't. And the llvm/clang/lldb code is used in a shared library that other tools link against. So the code really shouldn't crash when it is easy to work around and add code to avoid the crash. I still prefer the above code to look like:

  assert(ptr)
  if (ptr)
    ptr->foo();

As long as the program continues to function correctly without the assert, feel free to add any asserts to help catch issues during development.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107659/new/

https://reviews.llvm.org/D107659