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

rvalueification of Future::thenMulti(...): 1/n

Summary:
This is part of "the great r-valuification of folly::Future":
* This is something we should do for safety in general.
* Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical).
* This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.).
* This dichotomy and confusion has manifested itself by 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.
* The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).thenMulti(...)` instead of `f.thenMulti(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.

Reviewed By: LeeHowes

Differential Revision: D9415878

fbshipit-source-id: 3dd78941c60ee41f4284b5842e57c1c25005d15a
parent e432bbcd
......@@ -1759,7 +1759,7 @@ class Future : private futures::detail::FutureBase<T> {
/// where f is a Future<A> and the result of the chain is a Future<D>
/// becomes
///
/// f.thenMulti(a, b, c);
/// std::move(f).thenMulti(a, b, c);
///
/// Preconditions:
///
......@@ -1771,13 +1771,19 @@ class Future : private futures::detail::FutureBase<T> {
/// i.e., as if `*this` was moved into RESULT.
/// - `RESULT.valid() == true`
template <class Callback, class... Callbacks>
auto thenMulti(Callback&& fn, Callbacks&&... fns) {
auto thenMulti(Callback&& fn, Callbacks&&... fns) && {
// thenMulti with two callbacks is just then(a).thenMulti(b, ...)
return std::move(*this)
.then(std::forward<Callback>(fn))
.thenMulti(std::forward<Callbacks>(fns)...);
}
template <class Callback, class... Callbacks>
auto thenMulti(Callback&& fn, Callbacks&&... fns) & {
return std::move(*this).thenMulti(
std::forward<Callback>(fn), std::forward<Callbacks>(fns)...);
}
/// Create a Future chain from a sequence of callbacks.
///
/// Preconditions:
......@@ -1790,11 +1796,16 @@ class Future : private futures::detail::FutureBase<T> {
/// i.e., as if `*this` was moved into RESULT.
/// - `RESULT.valid() == true`
template <class Callback>
auto thenMulti(Callback&& fn) {
auto thenMulti(Callback&& fn) && {
// thenMulti with one callback is just a then
return std::move(*this).then(std::forward<Callback>(fn));
}
template <class Callback>
auto thenMulti(Callback&& fn) & {
return std::move(*this).thenMulti(std::forward<Callback>(fn));
}
/// Create a Future chain from a sequence of callbacks. i.e.
///
/// f.via(executor).then(a).then(b).then(c).via(oldExecutor)
......
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