Commit 110882c8 authored by Marshall Cline's avatar Marshall Cline Committed by Facebook Github Bot

use rvalue-qual Future::get()

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`.

Reviewed By: yfeldblum

Differential Revision: D8704033

fbshipit-source-id: 246f35c6320425e4b76ad11882bf55169988b772
parent 2e7e90e4
...@@ -284,5 +284,5 @@ TEST(Executor, DoNothingExecutor) { ...@@ -284,5 +284,5 @@ TEST(Executor, DoNothingExecutor) {
x.add({}); x.add({});
EXPECT_TRUE(f.isReady()); EXPECT_TRUE(f.isReady());
EXPECT_THROW(f.get(), folly::BrokenPromise); EXPECT_THROW(std::move(f).get(), folly::BrokenPromise);
} }
...@@ -271,5 +271,5 @@ TEST_F(FutureDAGTest, DestroyBeforeComplete) { ...@@ -271,5 +271,5 @@ TEST_F(FutureDAGTest, DestroyBeforeComplete) {
f = localDag->go(); f = localDag->go();
} }
barrier->wait(); barrier->wait();
ASSERT_NO_THROW(f.get()); ASSERT_NO_THROW(std::move(f).get());
} }
...@@ -1620,7 +1620,7 @@ void singleBatchDispatch(ExecutorT& executor, int batchSize, int index) { ...@@ -1620,7 +1620,7 @@ void singleBatchDispatch(ExecutorT& executor, int batchSize, int index) {
auto indexCopy = index; auto indexCopy = index;
auto result = batchDispatcher.add(std::move(indexCopy)); auto result = batchDispatcher.add(std::move(indexCopy));
EXPECT_EQ(folly::to<std::string>(index), result.get()); EXPECT_EQ(folly::to<std::string>(index), std::move(result).get());
} }
TEST(FiberManager, batchDispatchTest) { TEST(FiberManager, batchDispatchTest) {
...@@ -1720,7 +1720,7 @@ void doubleBatchOuterDispatch( ...@@ -1720,7 +1720,7 @@ void doubleBatchOuterDispatch(
auto indexCopy = index; auto indexCopy = index;
auto result = batchDispatcher.add(std::move(indexCopy)); auto result = batchDispatcher.add(std::move(indexCopy));
EXPECT_EQ(folly::to<std::string>(index), result.get()); EXPECT_EQ(folly::to<std::string>(index), std::move(result).get());
} }
TEST(FiberManager, doubleBatchDispatchTest) { TEST(FiberManager, doubleBatchDispatchTest) {
......
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