[PATCH] D34405: [Testing/Support] Remove the const_cast in TakeExpected
Pavel Labath via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 20 09:38:55 PDT 2017
labath added inline comments.
================
Comment at: include/llvm/Testing/Support/Error.h:34
+template <typename T> ExpectedHolder<T> TakeExpected(Expected<T> &&Exp) {
+ return TakeExpected(Exp);
}
----------------
zturner wrote:
> zturner wrote:
> > Should this be `std::move(Exp)`?
> Actually wait. If we have an rvalue reference overload just delete the lvalue ref overload and put all the code in here.
That won't work because the rvalue reference will not bind to lvalues (`Expected<T> foo = bar(); ASSERT_THAT_EXPECTED(foo, Succeeded());`).
This does look peculiarly recursive, but it's fine because within the body of this function "Exp" is an lvalue, so it will call the other function. std::move would actually make this recursive.
A possibly cleaner solution would be to have a TakeExpectedImpl function, taking a (lvalue) reference argument, and having both of these delegate to that. (But the only difference there would be that the non-recursivity would be more obvious, and that can also be done with a comment).
https://reviews.llvm.org/D34405
More information about the llvm-commits
mailing list