[llvm-dev] [RFC] Error handling in LLVM libraries.

[David Blaikie via llvm-dev]

On Wed, Feb 10, 2016 at 8:40 AM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> On 10 February 2016 at 11:10, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> > On Wed, Feb 10, 2016 at 6:47 AM, Rafael Espíndola <
> llvm-dev at lists.llvm.org>
> > wrote:
> >>
> >> >> This highlights why I think it is important to keep diagnostics and
> >> >> errors separate. In the solution you have there is a need to allocate
> >> >> a std::string, even if that is never used.
> >> >
> >> > Errors are only constructed on the error path. There is no
> construction
> >> > on
> >> > the success path.
> >>
> >> But they are always created, even if it as error the caller wants to
> >> ignore. For example, you will always create a "file foo.o in bar.a is
> >> not a bitcode" message (or copy sufficient information for that to be
> >> created). With a dignostic handler no copying is needed, since the
> >> call happens in the context where the error is found. It is easy to
> >> see us in a position where a lot of context is copied because some
> >> client somewhere might want it.
> >>
> >> So I am worried we are coding for the hypothetical and adding
> >> complexity. In particular, if we are going this way I think it is
> >> critical that your patch *removes* the existing diagnostic handlers
> >> (while making sure test/Bitcode/invalid.ll still passes) so that we
> >> don't end up with two overlapping solutions.
> >
> >
> > Removes diagnostic handlers in other parts of LLVM (& Clang) - the IR
> > verifier's diagnostic handling is what you're referring to here ^?
> >
> > I don't think that would be an improvement - it seems like different
> > situations call for different tools (as I was saying yesterday on this
> > thread). In some places a diagnostic handler is the right tool, and in
> some
> > places error codes/results/etc are the right tool. We already live in
> that
> > world & it seems like a reasonable one (there doesn't seem to be a
> > fundamental conflict between our bool+std::string or error_code returns
> and
> > existing diagnostic handlers - I think they can reasonably coexist in
> > different parts of the codebase for different use cases)
>
> In which case we do need a plain checked error_code. Right now we use
> a diagnostic handler in the BitcodeReader, but it is really easy to
> miss propagating an error out. We shouldn't be in a position where we
> have to decide to use an diagnostic handler or have checked errors.


I don't think we are or would be in that position based on this proposal -
it sounds to me like the TypedError has an implementation that wraps
error_code & that one could be used in places where we want a diagnostic
handler + checked errors (& as a side benefit we get hierarchical errors,
which can be handy) without having to bundle all the necessary state for
diagnostic emission through that path if it happens not to be the right
tool for the job in some contexts.


> It
> should be possible to have both if it is not desired that the new
> error handling replaces the use of diagnostic handling.
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160210/55fa941e/attachment.html>