folly: specialise makeFutureWith for functions returning futures

Summary: makeFutureWith executes a function and returns a future containing
set to the function's return value. This diff adds a specialisation
for the case when the function returns some type Future<T>. Instead
of returning Future<Future<T>>, makeFutureWith now just passes
on the future that the function returned, which may or may not have
a value set at this time. In case the function throws,
makeFutureWith returns a Future<T> containing the exception.

With this diff, the following two lines produce equivalent results:
  auto f1 = makeFutureWith(func);
  auto f2 = makeFuture().then(func);
except for the fact that f2 requires an additional temporary
Future<Unit> to be created and destroyed.

Reviewed By: @fugalh

Differential Revision: D2388335

diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h
index fecf1781e0d35c147ef183d8840ab730e5419011..eeb0e2e56e759308d0862afab33ce0a42ee11b41 100644 (file)
--- a/folly/futures/Future-inl.h
+++ b/folly/futures/Future-inl.h
@@ -475,10 +475,32 @@ Future<Unit> makeFuture() {
   return makeFuture(Unit{});
 }
 
+// makeFutureWith(Future<T>()) -> Future<T>
 template <class F>
-auto makeFutureWith(F&& func)
-    -> Future<typename Unit::Lift<decltype(func())>::type> {
-  using LiftedResult = typename Unit::Lift<decltype(func())>::type;
+typename std::enable_if<isFuture<typename std::result_of<F()>::type>::value,
+                        typename std::result_of<F()>::type>::type
+makeFutureWith(F&& func) {
+  using InnerType =
+      typename isFuture<typename std::result_of<F()>::type>::Inner;
+  try {
+    return func();
+  } catch (std::exception& e) {
+    return makeFuture<InnerType>(
+        exception_wrapper(std::current_exception(), e));
+  } catch (...) {
+    return makeFuture<InnerType>(exception_wrapper(std::current_exception()));
+  }
+}
+
+// makeFutureWith(T()) -> Future<T>
+// makeFutureWith(void()) -> Future<Unit>
+template <class F>
+typename std::enable_if<
+    !(isFuture<typename std::result_of<F()>::type>::value),
+    Future<typename Unit::Lift<typename std::result_of<F()>::type>::type>>::type
+makeFutureWith(F&& func) {
+  using LiftedResult =
+      typename Unit::Lift<typename std::result_of<F()>::type>::type;
   return makeFuture<LiftedResult>(makeTryWith([&func]() mutable {
     return func();
   }));

diff --git a/folly/futures/helpers.h b/folly/futures/helpers.h
index 30e657c7a9cd675c0c461f15f82ec8c130a04bfb..cf3e150b88eae2653c5bf6397804ad1170cc0eb9 100644 (file)
--- a/folly/futures/helpers.h
+++ b/folly/futures/helpers.h
@@ -75,11 +75,36 @@ Future<typename std::decay<T>::type> makeFuture(T&& t);
 /** Make a completed void Future. */
 Future<Unit> makeFuture();
 
-/** Make a completed Future by executing a function. If the function throws
-  we capture the exception, otherwise we capture the result. */
+/**
+  Make a Future by executing a function.
+
+  If the function returns a value of type T, makeFutureWith
+  returns a completed Future<T>, capturing the value returned
+  by the function.
+
+  If the function returns a Future<T> already, makeFutureWith
+  returns just that.
+
+  Either way, if the function throws, a failed Future is
+  returned that captures the exception.
+
+  Calling makeFutureWith(func) is equivalent to calling
+  makeFuture().then(func).
+*/
+
+// makeFutureWith(Future<T>()) -> Future<T>
+template <class F>
+typename std::enable_if<isFuture<typename std::result_of<F()>::type>::value,
+                        typename std::result_of<F()>::type>::type
+makeFutureWith(F&& func);
+
+// makeFutureWith(T()) -> Future<T>
+// makeFutureWith(void()) -> Future<Unit>
 template <class F>
-auto makeFutureWith(F&& func)
-    -> Future<typename Unit::Lift<decltype(func())>::type>;
+typename std::enable_if<
+    !(isFuture<typename std::result_of<F()>::type>::value),
+    Future<typename Unit::Lift<typename std::result_of<F()>::type>::type>>::type
+makeFutureWith(F&& func);
 
 /// Make a failed Future from an exception_ptr.
 /// Because the Future's type cannot be inferred you have to specify it, e.g.

diff --git a/folly/futures/test/FutureTest.cpp b/folly/futures/test/FutureTest.cpp
index 16c9f1f9f72c6fc37635c4e8e3b096b3d988f95b..cdec62e9e1d7fd29b9772401c0b92017540101e2 100644 (file)
--- a/folly/futures/test/FutureTest.cpp
+++ b/folly/futures/test/FutureTest.cpp
@@ -501,10 +501,20 @@ TEST(Future, makeFuture) {
   EXPECT_TYPE(makeFutureWith(fun), Future<int>);
   EXPECT_EQ(42, makeFutureWith(fun).value());
 
+  auto funf = [] { return makeFuture<int>(43); };
+  EXPECT_TYPE(makeFutureWith(funf), Future<int>);
+  EXPECT_EQ(43, makeFutureWith(funf).value());
+
   auto failfun = []() -> int { throw eggs; };
   EXPECT_TYPE(makeFutureWith(failfun), Future<int>);
+  EXPECT_NO_THROW(makeFutureWith(failfun));
   EXPECT_THROW(makeFutureWith(failfun).value(), eggs_t);
 
+  auto failfunf = []() -> Future<int> { throw eggs; };
+  EXPECT_TYPE(makeFutureWith(failfunf), Future<int>);
+  EXPECT_NO_THROW(makeFutureWith(failfunf));
+  EXPECT_THROW(makeFutureWith(failfunf).value(), eggs_t);
+
   EXPECT_TYPE(makeFuture(), Future<Unit>);
 }