[PATCH] D121750: Add a cmake flag to turn `llvm_unreachable()` into builtin_trap() when assertions are disabled

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

I like this idea.

Not sure if `LLVM_UNREACHABLE_SHOULD_TRAP` is quite right... would `LLVM_UNREACHABLE_ALWAYS_TRAP` be better?
(I'm honestly not sure. "should" seems a bit off to me, since it doesn't sound like a guarantee.)

Or: `LLVM_UNREACHABLE_OPTIMIZE=OFF`?
(Reasoning: it's disabling the optimization opportunity in no-asserts builds but leaving behind the debugging feature in asserts builds.)



================
Comment at: llvm/include/llvm/Support/ErrorHandling.h:127-128
 /// In !NDEBUG builds, prints the message and location info to stderr.
 /// In NDEBUG builds, becomes an optimizer hint that the current location
 /// is not supposed to be reachable.  On compilers that don't support
 /// such hints, prints a reduced message instead and aborts the program.
----------------
I think this part of the comment should be updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121750