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

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

> I haven't seen a reason articulated as to why this change shouldn't be made that doesn't apply to the project as a whole;

I think that this is exactly right. Moreover, if your goal is to make release builds more robust in reporting internal errors, you don't just want to enable asserts to Flang, but also in MLIR and LLVM itself. FWIW, all of my production builds for my organization shipped with asserts enabled (LLVM_ENABLE_ASSERTIONS) for many years, precisely because that risk tradeoff seemed more appropriate for that environment. If you're in a similar place in the risk-tradeoff space, I recommend building with Release/LLVM_ENABLE_ASSERTIONS=ON.

That all having been said, I'm open to having more granular flags for essentially selecting NDEBUG on a subproject level when configuring. I can certainly see why that might be convenient while working on development of a particular subproject for running test suites and the like.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79507