[PATCH] D33960: ErrorHandling: report_fatal_error: call abort() instead of exit()

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D33960#862838, @MatzeB wrote:

> In https://reviews.llvm.org/D33960#862837, @dblaikie wrote:
>
> > Hrm.
> >
> > Seeing that closing stderr test - I wouldn't mind some more eyes on this change. There's a fair amount of report_fatal_error usage in LLVM tools that probably aren't abort-worthy? But I'm not sure. But maybe that's OK (I mean they're mostly internal tools anyway)
>


I agree that some of the LLVM tools probably need to move to other error reporting mechanisms.

> Many of the `report_fatal_error`s in LLVM are abort worthy in my opinion.

I suspect essentially all of the ones in libraries are. =]

> There are indeed some like the bitcode parsing failures that really should rather use one of the other mechanisms available such as passing an `Error()` value to the caller or at least using LLVMContext::diagnose(). I hope this won't stop this patch, in fact it may be nice that this patch makes these things more obvious and hopefully brings people to use more apropriate APIs there.

I think this change is probably a good starting point. I see David's point in that I think we may want to revisit some uses of report_fatal_error in tools. But as he says, they're internal tools, and so I feel like we can incrementally go after the ones that are indeed inappropriate to use 'abort()'. We can even to quick fixes by setting up a custom handler much like frontends do.


Repository:
  rL LLVM

https://reviews.llvm.org/D33960