rvalueification of Future::thenMultiWithExecutor(...): 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: D9437727

fbshipit-source-id: 7656aff65db69a5cb51b621b7d7b1b01ef0d3e12

folly/futures/Future.h

@@ -1813,7 +1813,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.thenMultiWithExecutor(executor, a, b, c);
+  ///   std::move(f).thenMultiWithExecutor(executor, a, b, c);
   ///
   /// Preconditions:
   ///
@@ -1825,7 +1825,8 @@ 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 thenMultiWithExecutor(Executor* x, Callback&& fn, Callbacks&&... fns) {
+  auto
+  thenMultiWithExecutor(Executor* x, Callback&& fn, Callbacks&&... fns) && {
     // thenMultiExecutor with two callbacks is
     // via(x).then(a).thenMulti(b, ...).via(oldX)
     auto oldX = this->getExecutor();
@@ -1837,11 +1838,23 @@ class Future : private futures::detail::FutureBase<T> {
   }
 
   template <class Callback>
-  auto thenMultiWithExecutor(Executor* x, Callback&& fn) {
+  auto thenMultiWithExecutor(Executor* x, Callback&& fn) && {
     // thenMulti with one callback is just a then with an executor
     return std::move(*this).then(x, std::forward<Callback>(fn));
   }
 
+  template <class Callback, class... Callbacks>
+  auto thenMultiWithExecutor(Executor* x, Callback&& fn, Callbacks&&... fns) & {
+    return std::move(*this).themMultiWithExecutor(
+        x, std::forward<Callback>(fn), std::forward<Callbacks>(fns)...);
+  }
+
+  template <class Callback>
+  auto thenMultiWithExecutor(Executor* x, Callback&& fn) & {
+    return std::move(*this).themMultiWithExecutor(
+        x, std::forward<Callback>(fn));
+  }
+
   /// Moves-out `*this`, creating/returning a corresponding SemiFuture.
   /// Result will behave like `*this` except result won't have an Executor.
   ///