[PATCH] D79507: [flang] Change DIE("unreachable") cases to use llvm_unreachable

[Paul Robinson via Phabricator via llvm-commits]

probinson added a comment.

In D79507#2093088 <https://reviews.llvm.org/D79507#2093088>, @DavidTruby wrote:

> I fully support having a requirement that LLVM should support being shipped in Release with `LLVM_ENABLE_ASSERTIONS` configurations as a first class citizen, and would support an RFC on this on the LLVM mailing list.


As others pointed out, this build configuration is fully supported, many bots use it, many developers use it every day.  However, there's a difference between "you can build it this way if you want" and "this is what we package and deliver in our releases."  If the flang project is packaging its own releases, it can arrange to build LLVM as Release+Asserts as well as the flang front-end project.  I believe that is *not* how the standard Clang release packaging works, however.

> As a part of this I think we should consider cases where particularly expensive checks or ABI-incompatible changes are hidden behind `NDEBUG` as bugs that need fixing.

ABI-breaking changes are supposed to be under a separate configuration flag, LLVM_ABI_BREAKING_CHECKS.  "Expensive" checks similarly have their own flag, LLVM_ENABLE_EXPENSIVE_CHECKS (and there is at least one bot enabling that flag).
So, it is already the case that if such things are merely under NDEBUG then those are bugs that need fixing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79507