[Support] Add the 'Error' class for structured error handling.
Concern RaisedrL263609


[Support] Add the 'Error' class for structured error handling.

This patch introduces the Error classs for lightweight, structured,
recoverable error handling. It includes utilities for creating, manipulating
and handling errors. The scheme is similar to exceptions, in that errors are
described with user-defined types. Unlike exceptions however, errors are
represented as ordinary return types in the API (similar to the way
std::error_code is used).

For usage notes see the LLVM programmer's manual, and the Error.h header.
Usage examples can be found in unittests/Support/ErrorTest.cpp.

Many thanks to David Blaikie, Mehdi Amini, Kevin Enderby and others on the
llvm-dev and llvm-commits lists for lots of discussion and review.


lhamesMar 15 2016, 6:02 PM
rL263608: [X86] Regenerated widen load tests

Event Timeline

Benabik raised a concern with this commit.Mar 22 2016, 6:31 PM
Benabik added a subscriber: Benabik.
Benabik added inline comments.

This line, and the example after, are opposite to all of the other examples and the code itself (Error.h, line 188):

/// Bool conversion. Returns true if this Error is in a failure state,
/// and false if it is in an accept state. If the error is in a Success state
lhames added inline comments.Mar 22 2016, 8:06 PM

This comment applies to Expected<T>, not Error, and the two classes have opposite bool-conversion rules. This may be something that we should flag more prominently, but it's consistent with std::error_code and ErrorOr.

For Error we have:

if (auto Err = mayFail())
  return Err;

For Expected<T>:

if (auto ResultOrErr = mayFail2()) {
  // Success, access *ResultOrErr freely.
} else {
  // Failure.
  return ResultOrErr.takeError();
lhames added inline comments.Mar 22 2016, 8:08 PM

Looking at the paragraph that you quoted, we should definitely flag this more prominently - we never mention Error's bool conversion, or the fact that they behave differently. I'll work up a patch.

Thank you for catching this Brian.

I've made a clarification to the docs in r264135. Please let me know what you think. If it's still unclear, or you think the convention is inherently problematic that seems worthy of a discussion on the dev list.

Thanks again for the review!