(Wangle) chain -> thenMulti + thenMultiWithExecutor

Summary:
If we make `chain` a member function we can avoid the type issues and infer everything. I also added thenMulti for symmetry. Sadly the compiler doesn't like having a thenMulti with an optional `Executor*` as the first argument, it fails after some deductions. Hence `thenMultiWithExecutor`.

itssobeautiful

Test Plan: Run all the tests.

Reviewed By: hans@fb.com

Subscribers: trunkagent, folly-diffs@, jsedgwick, yfeldblum, chalfant

FB internal diff: D2021000

Signature: t1:2021000:1431557618:169447dc9d747b23a8a1ba830e78c43713d09a96

diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h
index f17eabfafc4be5cb14f4cafbde26121f103cf3d4..aecfa5784d694c2e0d8353ede95776e6f4a40001 100644 (file)
--- a/folly/futures/Future-inl.h
+++ b/folly/futures/Future-inl.h
@@ -1018,30 +1018,47 @@ Future<T> Future<T>::filter(F predicate) {
   });
 }
 
-namespace futures {
-  namespace {
-    template <class Z>
-    Future<Z> chainHelper(Future<Z> f) {
-      return f;
-    }
+template <class T>
+template <class Callback>
+auto Future<T>::thenMulti(Callback&& fn)
+    -> decltype(this->then(std::forward<Callback>(fn))) {
+  // thenMulti with one callback is just a then
+  return then(std::forward<Callback>(fn));
+}
 
-    template <class Z, class F, class Fn, class... Callbacks>
-    Future<Z> chainHelper(F f, Fn fn, Callbacks... fns) {
-      return chainHelper<Z>(f.then(fn), fns...);
-    }
-  }
+template <class T>
+template <class Callback, class... Callbacks>
+auto Future<T>::thenMulti(Callback&& fn, Callbacks&&... fns)
+    -> decltype(this->then(std::forward<Callback>(fn)).
+                      thenMulti(std::forward<Callbacks>(fns)...)) {
+  // thenMulti with two callbacks is just then(a).thenMulti(b, ...)
+  return then(std::forward<Callback>(fn)).
+         thenMulti(std::forward<Callbacks>(fns)...);
+}
 
-  template <class A, class Z, class... Callbacks>
-  std::function<Future<Z>(Try<A>)>
-  chain(Callbacks... fns) {
-    MoveWrapper<Promise<A>> pw;
-    MoveWrapper<Future<Z>> fw(chainHelper<Z>(pw->getFuture(), fns...));
-    return [=](Try<A> t) mutable {
-      pw->setTry(std::move(t));
-      return std::move(*fw);
-    };
-  }
+template <class T>
+template <class Callback, class... Callbacks>
+auto Future<T>::thenMultiWithExecutor(Executor* x, Callback&& fn,
+                                      Callbacks&&... fns)
+    -> decltype(this->then(std::forward<Callback>(fn)).
+                      thenMulti(std::forward<Callbacks>(fns)...)) {
+  // thenMultiExecutor with two callbacks is
+  // via(x).then(a).thenMulti(b, ...).via(oldX)
+  auto oldX = getExecutor();
+  setExecutor(x);
+  return then(std::forward<Callback>(fn)).
+         thenMulti(std::forward<Callbacks>(fns)...).via(oldX);
+}
 
+template <class T>
+template <class Callback>
+auto Future<T>::thenMultiWithExecutor(Executor* x, Callback&& fn)
+    -> decltype(this->then(std::forward<Callback>(fn))) {
+  // thenMulti with one callback is just a then with an executor
+  return then(x, std::forward<Callback>(fn));
+}
+
+namespace futures {
   template <class It, class F, class ItT, class Result>
   std::vector<Future<Result>> map(It first, It last, F func) {
     std::vector<Future<Result>> results;

diff --git a/folly/futures/Future.h b/folly/futures/Future.h
index ccea88b3389a94808204607d3adf27ca83567cb4..4f5db68e011b80db1fd7e54af15859e97ec532af 100644 (file)
--- a/folly/futures/Future.h
+++ b/folly/futures/Future.h
@@ -380,6 +380,42 @@ class Future {
   template <class I, class F>
   Future<I> reduce(I&& initial, F&& func);
 
+  /// Create a Future chain from a sequence of callbacks. i.e.
+  ///
+  ///   f.then(a).then(b).then(c)
+  ///
+  /// where f is a Future<A> and the result of the chain is a Future<D>
+  /// becomes
+  ///
+  ///   f.thenMulti(a, b, c);
+  template <class Callback, class... Callbacks>
+  auto thenMulti(Callback&& fn, Callbacks&&... fns)
+    -> decltype(this->then(std::forward<Callback>(fn)).
+                      thenMulti(std::forward<Callbacks>(fns)...));
+
+  // Nothing to see here, just thenMulti's base case
+  template <class Callback>
+  auto thenMulti(Callback&& fn)
+    -> decltype(this->then(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)
+  ///
+  /// where f is a Future<A> and the result of the chain is a Future<D>
+  /// becomes
+  ///
+  ///   f.thenMultiWithExecutor(executor, a, b, c);
+  template <class Callback, class... Callbacks>
+  auto thenMultiWithExecutor(Executor* x, Callback&& fn, Callbacks&&... fns)
+    -> decltype(this->then(std::forward<Callback>(fn)).
+                      thenMulti(std::forward<Callbacks>(fns)...));
+
+  // Nothing to see here, just thenMultiWithExecutor's base case
+  template <class Callback>
+  auto thenMultiWithExecutor(Executor* x, Callback&& fn)
+    -> decltype(this->then(std::forward<Callback>(fn)));
+
  protected:
   typedef detail::Core<T>* corePtr;
 

diff --git a/folly/futures/helpers.h b/folly/futures/helpers.h
index ded42702cce416a2a74cfacd56236b4e5249540d..775989e8a5b9295e9f0edd072eeaef0cd0650a31 100644 (file)
--- a/folly/futures/helpers.h
+++ b/folly/futures/helpers.h
@@ -39,19 +39,6 @@ namespace futures {
   /// Futures you will pay no Timekeeper thread overhead.
   Future<void> sleep(Duration, Timekeeper* = nullptr);
 
-  /// Create a Future chain from a sequence of callbacks. i.e.
-  ///
-  ///   f.then(a).then(b).then(c);
-  ///
-  /// where f is a Future<A> and the result of the chain is a Future<Z>
-  /// becomes
-  ///
-  ///   f.then(chain<A,Z>(a, b, c));
-  // If anyone figures how to get chain to deduce A and Z, I'll buy you a drink.
-  template <class A, class Z, class... Callbacks>
-  std::function<Future<Z>(Try<A>)>
-  chain(Callbacks... fns);
-
   /**
    * Set func as the callback for each input Future and return a vector of
    * Futures containing the results in the input order.

diff --git a/folly/futures/test/ViaTest.cpp b/folly/futures/test/ViaTest.cpp
index 061dc4c4b34de965cdf6f3de0852f61a8e2cf77d..edaed77409720cb3ff119581243258f6c633f7b0 100644 (file)
--- a/folly/futures/test/ViaTest.cpp
+++ b/folly/futures/test/ViaTest.cpp
@@ -171,16 +171,16 @@ TEST_F(ViaFixture, viaAssignment) {
 TEST(Via, chain1) {
   EXPECT_EQ(42,
             makeFuture()
-            .then(futures::chain<void, int>([] { return 42; }))
+            .thenMulti([] { return 42; })
             .get());
 }
 
 TEST(Via, chain3) {
   int count = 0;
-  auto f = makeFuture().then(futures::chain<void, int>(
+  auto f = makeFuture().thenMulti(
       [&]{ count++; return 3.14159; },
       [&](double) { count++; return std::string("hello"); },
-      [&]{ count++; return makeFuture(42); }));
+      [&]{ count++; return makeFuture(42); });
   EXPECT_EQ(42, f.get());
   EXPECT_EQ(3, count);
 }
@@ -228,6 +228,32 @@ TEST(Via, priority) {
   EXPECT_EQ(3, exe.count2);
 }
 
+TEST_F(ViaFixture, chainX1) {
+  EXPECT_EQ(42,
+            makeFuture()
+            .thenMultiWithExecutor(eastExecutor.get(),[] { return 42; })
+            .get());
+}
+
+TEST_F(ViaFixture, chainX3) {
+  auto westThreadId = std::this_thread::get_id();
+  int count = 0;
+  auto f = via(westExecutor.get()).thenMultiWithExecutor(
+      eastExecutor.get(),
+      [&]{
+        EXPECT_NE(std::this_thread::get_id(), westThreadId);
+        count++; return 3.14159;
+      },
+      [&](double) { count++; return std::string("hello"); },
+      [&]{ count++; })
+    .then([&](){
+        EXPECT_EQ(std::this_thread::get_id(), westThreadId);
+        return makeFuture(42);
+    });
+  EXPECT_EQ(42, f.getVia(waiter.get()));
+  EXPECT_EQ(3, count);
+}
+
 TEST(Via, then2) {
   ManualExecutor x1, x2;
   bool a = false, b = false, c = false;