[PATCH] D56245: Use llvm_unreachable instead of errs+abort in MCStreamer.cpp (in EmitRawTextImpl). NFC.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D56245#1345316 <https://reviews.llvm.org/D56245#1345316>, @rnk wrote:

> I don't think this should be llvm_unreachable, I think it should be report_fatal_error. LLVM has some some support for out of tree backends, and they could plausibly call this when using a release build of LLVM as a library. If all the callers were in-tree, then calling this would represent an LLVM-internal logic error (forgetting to check hasRawTextSupport()), and unreachable or assert would be appropriate.


Not a strong disagreement, but I'm not sure I follow this entirely - even for external users, there are preconditions for certain API calls that must be met & can still be considered assertion-level failures (ie: the calling code is buggy if it makes this call, not "the calling code can expect a specific failure behavior & rely on this if it wants to") & sounds like this could be one of those.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56245