This revision was automatically updated to reflect the committed changes.
Closed by commit rL303348: Add Status -- llvm::Error glue (authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D33241?vs=99283&id=99429#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33241
Files:
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
labath updated this revision to Diff 99283.
labath added a comment.
use a separate function instead of a conversion operator
https://reviews.llvm.org/D33241
Files:
include/lldb/Utility/Status.h
source/Utility/Status.cpp
unittests/Utility/StatusTest.cpp
Index: unittests/Utility/StatusTest
Having the construction accept an Error is probably fine, because you have
to std::move it anyway, which means you know you're giving up ownership
On Wed, May 17, 2017 at 3:08 AM Pavel Labath via Phabricator <
revi...@reviews.llvm.org> wrote:
> labath added a comment.
>
> In https://reviews.llvm.o
labath added a comment.
In https://reviews.llvm.org/D33241#756921, @zturner wrote:
> Mostly just that implicit conversion operators are a very subtle source of
> bugs, and you can't even find where they're being used because it's
> impossible to grep for it.
I agree, and that's why I made the
zturner added a comment.
Mostly just that implicit conversion operators are a very subtle source of
bugs, and you can't even find where they're being used because it's impossible
to grep for it.
If you're signing up to get an object that has strict usage semantics like
`llvm::Error`, you had b
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.
This seems fine to me.
Zachary didn't give reasons why he didn't like the conversion form so I can't
really assess that point. The use in the ErrorConversion test case seems
pretty natural
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
zturner added a reviewer: lhames.
zturner added inline comments.
Comment at: include/lldb/Utility/Status.h:108
+ explicit Status(llvm::Error error);
+ explicit operator llvm::Error();
+
I think we should remove the conversion operator. Instead, why don't we ma
labath created this revision.
I tried convert a function to llvm::Error/Expected but I quickly ran into the
issue of interfacing with code that still expects the Status objects.
This is my proposal for conversion functions between the two. I use
constructors and conversion operators on the Status
10 matches
Mail list logo
