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

remove lvalue-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`.
* Note: `get(...)` refers to `get()` and `get(Duration)`.

Reviewed By: yfeldblum

Differential Revision: D8710296

fbshipit-source-id: ae201af1928eb2f6a2897c9b7db884393b36b910
parent 41ed7dd9
......@@ -2190,11 +2190,6 @@ T Future<T>::get() && {
return copy(std::move(*this)).value();
}
template <class T>
T Future<T>::get() & {
return std::move(wait().value());
}
template <class T>
T Future<T>::get(Duration dur) && {
wait(dur);
......@@ -2205,15 +2200,6 @@ T Future<T>::get(Duration dur) && {
return std::move(future).value();
}
template <class T>
T Future<T>::get(Duration dur) & {
wait(dur);
if (!this->isReady()) {
throw_exception<FutureTimeout>();
}
return std::move(this->value());
}
template <class T>
Try<T>& Future<T>::getTry() {
return result();
......
......@@ -1550,13 +1550,6 @@ class Future : private futures::detail::FutureBase<T> {
/// - `valid() == false`
T get() &&;
/// Blocks until the future is fulfilled. Returns the value (moved out), or
/// throws the exception. The future must not already have a callback.
///
/// Deprecated in favor of `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
/// exception).
......@@ -1570,14 +1563,6 @@ class Future : private futures::detail::FutureBase<T> {
/// - `valid() == false`
T get(Duration dur) &&;
/// Blocks until the future is fulfilled, or until `dur` elapses. Returns the
/// value (moved out), or throws the exception (which might be a FutureTimeout
/// exception).
///
/// Deprecated in favor of `get(Duration) &&`.
[[deprecated("being removed soon; use rvalue-qualified get() instead")]] T
get(Duration dur) &;
/// A reference to the Try of the value
///
/// Preconditions:
......
......@@ -233,11 +233,6 @@ 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());
......@@ -272,11 +267,6 @@ 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