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

Mon Jun 15 09:46:54 PDT 2020

DavidTruby added a comment.

In D79507#2093054 <https://reviews.llvm.org/D79507#2093054>, @pmccormick wrote:

> I said this on today's call but wanted to also add a note here.
>
> We should split this into two separate discussions.   Both points here are valid concerns but I would propose we separate implementation practices from the requirements.
>
> I think we should follow the established LLVM practices here.   From this perspective this patch should proceed.
>
> However, we need to make certain it is possible to build a release of the compiler that enables the execution of internal checks (even at the cost of some performance) as a build option.  I will go so far as to suggest that this should be a requirement as it matches many uses cases that I'm aware of (as also pointed out by @hfinkel).   I'm not certain how far away we are from supporting this across clang, flang, mlir, llvm, etc. but the LLVM_ENABLE_ASSERTIONS path seems like at least a place to start.

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 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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79507