Commit 860b718e authored by Marshall Cline's avatar Marshall Cline Committed by Facebook Github Bot

use rvalue-qual Future::get(): pass 3

Summary:
This is part of "the great r-valuification of folly::Future":

* This is something we should do for safety in general.
* Using lvalue-qualified `Future::get()` has caused some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
* Problems with `Future::get() &`: it moves-out the result but doesn't invalidate the Future - the Future remains (technically) valid even though it actually is partially moved-out. Callers can subsequently access that moved-out result via things like `future.get()`, `future.result()`, `future.value()`, etc. - these access an already-moved-out result which is/can be surprising.
* Reasons `Future::get() &&` is better: its semantics are more obvious and user-testable. It moves-out the Future, leaving it with `future.valid() == false`.

Codemod steps (where `get(...)` means both `Future::get()` and `Future::get(Duration)`):

* expr.get(...) ==> std::move(expr).get(...)  // if expr is not already an xvalue
* expr->get(...) ==> std::move(*expr).get(...)

Note: operator precedence of that last step is safe - no need to parenthesize `expr`. Reason: `->` binds more tightly than unary `*`.

Reviewed By: yfeldblum

Differential Revision: D8683752

fbshipit-source-id: 593bcd92ca4e11244e0f912cf956538889a2c777
parent 3dd3467e
......@@ -95,7 +95,7 @@ TEST(Future, getRequiresOnlyMoveCtor) {
auto f = makeFuture<MoveCtorOnly>(MoveCtorOnly(42));
EXPECT_TRUE(f.valid());
EXPECT_TRUE(f.isReady());
auto v = f.get();
auto v = std::move(f).get();
EXPECT_EQ(v.id_, 42);
}
{
......@@ -109,7 +109,7 @@ TEST(Future, getRequiresOnlyMoveCtor) {
auto f = makeFuture<MoveCtorOnly>(MoveCtorOnly(42));
EXPECT_TRUE(f.valid());
EXPECT_TRUE(f.isReady());
auto v = f.get(std::chrono::milliseconds(10));
auto v = std::move(f).get(std::chrono::milliseconds(10));
EXPECT_EQ(v.id_, 42);
}
{
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment