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

deprecate 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.
* Context: `Future::get(...)` means both `Future::get()` and `Future::get(Duration)`
* 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: LeeHowes

Differential Revision: D8799081

fbshipit-source-id: 890a221c84ba4847abfaf6e564b0eceb0176fd54
parent c8eeea45
......@@ -1553,7 +1553,8 @@ class Future : private futures::detail::FutureBase<T> {
/// throws the exception. The future must not already have a callback.
///
/// Deprecated in favor of `get() &&`.
T get() &;
[[deprecated("being removed soon; use rvalue-qualified get() instead")]] T
get() &;
/// Blocks until the future is fulfilled, or until `dur` elapses. Returns the
/// value (moved-out), or throws the exception (which might be a FutureTimeout
......@@ -1573,7 +1574,8 @@ class Future : private futures::detail::FutureBase<T> {
/// exception).
///
/// Deprecated in favor of `get(Duration) &&`.
T get(Duration dur) &;
[[deprecated("being removed soon; use rvalue-qualified get() instead")]] T
get(Duration dur) &;
/// A reference to the Try of the value
///
......
......@@ -233,8 +233,11 @@ TEST(Future, hasPreconditionValid) {
DOIT(f.isReady());
DOIT(f.result());
FOLLY_PUSH_WARNING
FOLLY_GNU_DISABLE_WARNING("-Wdeprecated-declarations")
DOIT(f.get());
DOIT(f.get(std::chrono::milliseconds(10)));
FOLLY_POP_WARNING
DOIT(std::move(f).get());
DOIT(std::move(f).get(std::chrono::milliseconds(10)));
DOIT(f.getTry());
......@@ -269,8 +272,11 @@ TEST(Future, hasPostconditionValid) {
DOIT(swallow(f.poll()));
DOIT(f.raise(std::logic_error("foo")));
DOIT(f.cancel());
FOLLY_PUSH_WARNING
FOLLY_GNU_DISABLE_WARNING("-Wdeprecated-declarations")
DOIT(swallow(f.get()));
DOIT(swallow(f.get(std::chrono::milliseconds(10))));
FOLLY_POP_WARNING
DOIT(swallow(f.getTry()));
DOIT(f.wait());
DOIT(std::move(f.wait()));
......
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