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

[Lang Hames via llvm-dev]

Hi Rafael,

> 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.

I'm not saying that this system would replace diagnostic handlers in
general, or that it should be shoehorned into places where it doesn't fit.
This system is a replacement for std::error_code, because std::error_code
is deficient as an error return. It happens that once you have a generic
error return value you can use it for a lot of simple diagnostic cases,
rather than threading a diagnostic handler everywhere. If you want to mix
and match my error scheme with a diagnostic handler you still can.

As to the performance concern - you're trying to optimizing the error case.
As noted, my scheme is slower than std::error_code on the error_path, but
not prohibitively so. I've thought about this, and cannot imagine a
reasonable use of this system that would lead to unacceptable overhead on
the error path.

> So I am worried we are coding for the hypothetical and adding complexity.

Don't worry - I'm too busy with real problems to tackle hypothetical ones.
Here's another example of a problem that my proposal solves:

I recently added support for remote JITing to ORC. There are in-tree
library facilities for JITing code into a process on the other end of an
abstract "RPCChannel". What happens if something goes wrong at one end? We
want to be able to communicate an error back across the RPCChannel
(assuming it's still intact) so the other side can recover or fail
gracefully. That means we need to be able to serialize an error with enough
information to describe what went wrong. There's no practical way to
maintain a serialization routine for all possible std::error_codes that
might come up, even if they were powerful enough to describe everything
that could go wrong (which again, being static kinds, they're not). With my
proposal however, a JITError base class can be defined as:

class JITError : public TypedErrorInfo<JITError> {
public:
  virtual void serialize(RPCChannel &C) const = 0;
};

Now you just describe serialization/deserialization for each error when you
define it. :)

(Yes - this hand waves over deserialization. It's solvable. The gory
details will add nothing to this discussion).

> 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. We were still not even
> done converting away from bool+std::string :-(

I think I've covered this now, but once more for emphasis: I'm not going to
start tearing all the diagnostic handlers out. I absolutely see the value
of diagnostic handlers for producing intricate diagnostics. My proposal
tackles the error return problem, and it turns out that once you have a
customizable error return value you can use it to produce decent
diagnostics, as long as the client doesn't need to be able to control the
formatting of those diagnostics. This property will allow us to produce
better diagnostics that we currently do in several tools, without the need
to introduce diagnostic handlers there. We can still introduce them later
if we want to.

Cheers,
Lang.

On Wed, Feb 10, 2016 at 6:47 AM, Rafael Espíndola <
rafael.espindola at gmail.com> 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. We were still not even
> done converting away from bool+std::string :-(
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160210/383d585c/attachment.html>