[patch] Simplify ErrorOr

Rafael EspĂ­ndola rafael.espindola at gmail.com
Mon Nov 4 07:11:45 PST 2013


> ErrorOr<void> isn't that weird. I just means an error or nothing. We
> could also just typedef ErrorOr<void> as Error or something.
>
> As for llvm::error_code, it is explicitly based on the C++11
> std::error_code, and I plan to replace it with that when we move over
> to C++11.

I agree with not changing error_code. It looks just right for
representing errors which are just an int, like anything coming from
errno.

>>
>> * The vast majority of use cases don't need it, so it is probably
>> better to have a different type that makes it clear user data is being
>> hold (say user_error_code<T>). This could then also avoid the separate
>> allocation and indirection.
>
> Almost all code would end up using this type then, as it would need to
> properly propagate the extra info.

No, all the OS basic errors can be just error_code. Just that seems
sufficient to justify having two types. In its current implementation
at least I would strongly object to propagating ErrorOr everywhere:

* It has virtual methods.
* It has reference counted data.
* It has strange move semantics. Why the InValid flag? ErrorOr is just
a wrapper. If it wrapping something with special move logic
(std::vector for example), it is ok to call the destructor. It is
std::vector's destructor that handles it having been moved.

To avoid this and still make it possible to implement the features you
want for lld, what about:

* ErrorOr becomes a simple wrapper over two templated types: An error
type and a data type.
* The error type default to error_code.
* The data type has no default.
* They are stored in a union. We use an extra bit for the tag.
* They are all values. No pointer indirection, reference counting or
special handling of moves. If any of the error (or data) types need
that, it can be implemented in the type itself.
* When we want an error that caries more information than error_code,
we can just use ErrorOr<ResultType, ErrorType>.

> Also, would T be the user data type? That would make it not possible
> to propagate different types of user error data.

No, T would be the type of the extra error info. In any case, the
above proposal replaces this one.

>>
>> * The need for anything more than a string is also not clear, so a
>> str_error_code would probably be sufficient.
>
> A string is not enough. For example in lld we want to use debug info
> to provide better diagnostics. We can't always do this at the time the
> error is encountered, so the state needs to be persisted in a easily
> usable format.

If this is something that is only used in LLD, it should probably be
in lld. With the above proposal you could just implement a
DebugInfoError in lld. What do you think?

Cheers,
Rafael



More information about the llvm-commits mailing list