[Lldb-commits] [PATCH] D33241: Add Status -- llvm::Error glue
Zachary Turner via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Wed May 17 09:07:48 PDT 2017
zturner added inline comments.
================
Comment at: source/Utility/Status.cpp:81-88
+Status::operator llvm::Error() {
+ if (Success())
+ return llvm::Error::success();
+ if (m_type == ErrorType::eErrorTypePOSIX)
+ return llvm::errorCodeToError(std::error_code(m_code, std::generic_category()));
+ return llvm::make_error<llvm::StringError>(AsCString(),
+ llvm::inconvertibleErrorCode());
----------------
labath wrote:
> zturner wrote:
> > zturner wrote:
> > > Delete in favor of an `LLDBError` class as mentioned before.
> > Actually this doesn't really work, because you don't want to call `make_error<LLDBError>(S)` if `S` is a success condition.
> >
> > So perhaps instead, I would at least call it something more explicit. `llvm::Error toError() const` perhaps.
> Besides the success case issue, I believe having a member function (be it a conversion operator or not) allows us to have better interoperability between the two. I think of `Status` as being at the same level as llvm::Error, as it also tries to multiplex multiple error kinds into a single type, whereas having LLDBError introduces a subordinate relationship. Having a member function allows us to represent errno errors the same way as llvm does it.
>
> Instead of an LLDBError I'd propose we create more specialized error classes (UnwindError? NetworkError?) once we actually identify use cases for them. Although we can have LLDBError as a common superclass of these errors if you think it will be useful.
Yes, eventually we will definitely want to have different error implementations. `CommandArgumentError`, `RemoteCommunicationError`, `DebugInfoError`, etc all come to mind as potential candidates.
https://reviews.llvm.org/D33241
More information about the lldb-commits
mailing list