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

[David Truby via Phabricator via llvm-commits]

DavidTruby added a comment.

I agree with @dblaikie, @jdoerfert and @aaron.ballman here.

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.

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. If there's a reason that this design choice should only apply to flang and not e.g. clang then I'm happy to drop the consistency argument.

It boils down to the fact that our code should follow the coding guidelines unless there's a specific, exceptional reason that applies to our subproject that we can't do so. I strongly believe the burden of argument in these cases should be on those arguing that we should diverge from the coding guidelines, not on those who are simply following the written rules of the project that we are part of. Any other way of working will only cause us issues going forward when (hopefully) we get people working with us cross-project.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79507