[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