As mentioned in issue 68828, the names of some of our error-handling classes are unintuitive and even misleading. I propose to change them as follows.
std::error_code and maybe T) becomes
llvm::Error and maybe T) becomes
Obviously all of the old
ErrorOr instances would have to be changed first, before starting on the
Renaming something into a name we used to have but changed in the meantime is generally not a good idea. I am 100% sure I will be very confused by this at some point, at the latest, when I look at new and old code at the same time, e.g., compare code and an old PR, or do git blame.
Can we come up with a fresh name?
I had the same thought as @jdoerfert reading the proposal, what about
(note that “expected” is also consistent with the C++23
std::expected: std::expected - cppreference.com )
I’m very sure that the existing
ErrorOr is a bad name choice and should be changed.
I’m willing to say
Expected is what it is and leave it alone.
Why do we need both of them? Structure-wise they are essentially the same, i.e. “T or error information”.
Edit: I’m suggesting we convert uses of
template< class T, class E >
It is up to you what you put in
E . The LLVM
expected seems to be missing the
ErrorOr<T> is a simple enum or T, and doesn’t insist that you check whether it worked.
llvm::Error which is more heavyweight, and aborts if you don’t check it.
If someone doesn’t care what the error was, they can use
std::optional. If they want to have error information, then they can use a type where the error check is mandatory. I understand that they are not exactly equivalent, but I think that we should have a canonical way of handling errors, and that
Expected fits the bill a lot better than
llvm::Expected<T> is roughly
std::expected<T, llvm::Error> probably with some extra methods (I haven’t looked). I’m not proposing to redefine it that way, although someone could.
Expected is used far more often than
ErrorOr. But as I skim through the uses of
ErrorOr, I see it used quite often in file operations, where
std::error_code (basically, errno values) is way more appropriate than
llvm::Error. I’m reluctant to say those are all wrong.
My point is more about having a single mechanism for error handling, than the details of individual classes. If the current implementation of
Expected is deficient, we should consider improving it. Maybe
Expected should take that extra
E, which can be defaulted to
llvm:Error for compatibility?
On that note, I’m wondering if there are any uses of
ErrorOr, where the “error” is really a “failure”…
expected with the standard is the right direction. I prefer partial specialisation over hidden defaults.
template <class T> using ExpectedError = expected<T, llvm::Error>;
The standard does not have a default and leaves it fully open, it seems appropriate to me that the default Error in llvm would be
llvm::Error, so I would think that this would be a good setup for LLVM:
template <class T, class E = llvm::Error>
ECError (it is a
llvm::Error that wraps a
std::error_code) be enough to obsolete the
I think this would be an ideal outcome. I think the only trouble is figuring out how to handle known error codes in a reasonably ergonomic way, in code like this and this looking for
llvm::errc::no_such_file_or_directory. Can we make some utility that handles specific error codes better than we do currently?
As for the renaming dance - if we waited a release or so before reusing the name it might not be the worst thing. But if the standard (albeit more generic) version is called expected, maybe it’s OK to just leave
llvm::Expected as-is even if the
ErrorOr name is freed up.
I don’t think we need to generalize
llvm::Expected to allow for other error entities if we don’t have a compelling use case (if we really want to keep error_code handling around, then maybe that’s sufficient use case, but then we still need a name for it - and yeah, maybe that name is
Expected<T, error_code> or something)
But would be great to just standardize on
llvm::Error (& consequently just
llvm::Expected without being able to configure the error object)
When I get a chance, I will look at replacing
ECError and see how that goes, with particular attention to the point @rnk raised. This would be more than a simple rename, I think, so might require a bit more effort than I was expecting.
std::expected we can look ahead to that, but note that LLVM’s current dialect is C++17 so we can’t assume
std::expected is available.
There was llvm::Optional for the same reasons. The std version is only available in later dialects. llvm::Optional is gone now. Adding llvm::E/eexpected with no default until we move to C++23 ought to be an improvement.
llvm::Expected has the additional behavior that it will abort if the error isn’t checked; so its behavior does not align as cleanly with std::expected as in the Optional/optional case. I think the intention to eliminate llvm::Expected would have to be a separate discussion related to the value of that additional behavior.
ATM I am not intending to do anything wrt llvm::Expected.
Aligning the llvm Expected with the standard makes the transition smoother and leaves you the option what to put in
E : llvm::Error, std::error_code, …
As @pogo59 mentioned,
Expected has extra checking (it checks that you checked the error, and assert-fails if you don’t)
std::expected doesn’t have and won’t have - so it’s not as obvious that we’ll want to transition to the standard one (if I had to bet right now - we won’t ever trasnsition
std::expected because the extra error checking checking is of significant value) - so I wouldn’t be inclined to over-generalize it now/with that direction in mind.