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

[Tim Keith via Phabricator via llvm-commits]

tskeith added a comment.

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

> I believe consistency is key, which is why I was pushing this change in the first place. Many of us work on multiple LLVM sub-projects and having to remember different coding guidelines for different files in the same repository is fairly irritating, and I think there's a barrier to entry for getting people from other subprojects to help us fix things if we're going to be rejecting their patches to flang because they don't follow the written guidelines the whole project should follow and the style that the subproject they usually work on follows.


flang has no guidelines that say don't use llvm_unreachable and no changes have been rejected because of its use.

> 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'd like to know what makes flang exceptional here that we should do things differently.

flang is not different. I think a developer in any part of the project should have the option of using a version of llvm_unreachable (called DIE or something else) that is defined to produce an internal error even in Release builds. I object to this particular change because whoever wrote the code chose to use DIE, which has well-defined behavior in Release builds and you haven't given a good reason (IMO) to override that decision and make it undefined. The reason for the change seems to be that someone new to flang might encounter DIE and not know at first what it does (!?); the same would seem to be true of thousands of other names they would encounter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79507