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

[Rafael Espíndola via llvm-dev]

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