[PATCH] Add ErrorOr<>::getOrDie() which uses report_fatal_error instead of assert.

[Alexander Kornienko]

On Fri, Oct 10, 2014 at 12:26 AM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> On 9 October 2014 15:44, Justin Bogner <mail at justinbogner.com> wrote:
> > Alexander Kornienko <alexfh at google.com> writes:
> >> This method allows writing code that consistently fails in case of
> >> dereferencing an ErrorOr<> containing no value rather than crashing in
> >> debug and using uninitialized data in release build.
> >
> > I don't see why this would be better than doing this check in the
> > caller, for two reasons:
> >
> > 1. Situations where report_fatal_error is the right thing are fairly
> >    rare, so I wouldn't expect it to be a lot of boilerplate.
> >
> > 2. The caller generally can and should report a more specific error
>
> I don't thing anyone disagrees that a nice error message is a feature,
> but it might not be the priority of every code to use a SMDiagnostic.
> Using report_fatal_error is a reasonable way to start (or an
> improvement over a misplaced assert/unreachable, like we have in parts
> of LLVM (pr20485 for example)).
>
> Having said that, calling report_fatal_error in a class that is pretty
> independent from llvm looks a bit odd.


Well, this class is a part of LLVM, so it could use other LLVM facilities.


> Could we maybe have something
> like
>
> template<typename T>
> const T& extractOrDie(const ErrorOr<T> &V) {
>   if (V)
>     return *V;
>   report_fatal_error("foo");
> }
>
> defined in some other header?
>

It can be a standalone function, if you think that it makes sense keeping
ErrorOr<> dependencies on LLVM minimal. But the function would be closely
tied to ErrorOr<>, and it looks reasonable to put it to the same header.

-- Alex


> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141010/f9a285a8/attachment.html>