From: Sven Over <over@fb.com>
Date: Wed, 31 May 2017 16:43:09 +0000 (-0700)
Subject: Future: some fixes re: handling of universal references
X-Git-Tag: v2017.06.05.00~15
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=8401bbaffc06e05b14ea7057842b7ed2804375c0;p=folly.git

Future: some fixes re: handling of universal references

Summary:
Callables that are passed as rvalues to makeFutureWith should be
executed as rvalues.
Generally callables that get captured by value should be executed
as rvalues, to allow passing e.g. folly::Partial objects that contain
move-only objects like unique_ptr or folly::Promise.
`collect` would move out of parameters passed as lvalues.

Reviewed By: fugalh

Differential Revision: D5120420

fbshipit-source-id: 1633c6b7e3fbb562f4d31e21d28c98b053de9912
---

diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h
index c4f9ca2f..4089a676 100644
--- a/folly/futures/Future-inl.h
+++ b/folly/futures/Future-inl.h
@@ -324,7 +324,7 @@ template <class T>
 template <class F>
 Future<T> Future<T>::ensure(F&& func) {
   return this->then([funcw = std::forward<F>(func)](Try<T> && t) mutable {
-    funcw();
+    std::move(funcw)();
     return makeFuture(std::move(t));
   });
 }
@@ -333,7 +333,7 @@ template <class T>
 template <class F>
 Future<T> Future<T>::onTimeout(Duration dur, F&& func, Timekeeper* tk) {
   return within(dur, tk).onError([funcw = std::forward<F>(func)](
-      TimedOut const&) { return funcw(); });
+      TimedOut const&) { return std::move(funcw)(); });
 }
 
 template <class T>
@@ -457,8 +457,7 @@ inline Future<T> Future<T>::via(Executor* executor, int8_t priority) & {
 
 template <class Func>
 auto via(Executor* x, Func&& func)
-  -> Future<typename isFuture<decltype(func())>::Inner>
-{
+    -> Future<typename isFuture<decltype(std::declval<Func>()())>::Inner> {
   // TODO make this actually more performant. :-P #7260175
   return via(x).then(std::forward<Func>(func));
 }
@@ -504,7 +503,7 @@ makeFutureWith(F&& func) {
   using InnerType =
       typename isFuture<typename std::result_of<F()>::type>::Inner;
   try {
-    return func();
+    return std::forward<F>(func)();
   } catch (std::exception& e) {
     return makeFuture<InnerType>(
         exception_wrapper(std::current_exception(), e));
@@ -522,9 +521,8 @@ typename std::enable_if<
 makeFutureWith(F&& func) {
   using LiftedResult =
       typename Unit::Lift<typename std::result_of<F()>::type>::type;
-  return makeFuture<LiftedResult>(makeTryWith([&func]() mutable {
-    return func();
-  }));
+  return makeFuture<LiftedResult>(
+      makeTryWith([&func]() mutable { return std::forward<F>(func)(); }));
 }
 
 template <class T>
@@ -574,7 +572,7 @@ collectAll(Fs&&... fs) {
   auto ctx = std::make_shared<detail::CollectAllVariadicContext<
     typename std::decay<Fs>::type::value_type...>>();
   detail::collectVariadicHelper<detail::CollectAllVariadicContext>(
-    ctx, std::forward<typename std::decay<Fs>::type>(fs)...);
+      ctx, std::forward<Fs>(fs)...);
   return ctx->p.getFuture();
 }
 
@@ -677,7 +675,7 @@ collect(Fs&&... fs) {
   auto ctx = std::make_shared<detail::CollectVariadicContext<
     typename std::decay<Fs>::type::value_type...>>();
   detail::collectVariadicHelper<detail::CollectVariadicContext>(
-    ctx, std::forward<typename std::decay<Fs>::type>(fs)...);
+      ctx, std::forward<Fs>(fs)...);
   return ctx->p.getFuture();
 }
 
diff --git a/folly/futures/detail/Core.h b/folly/futures/detail/Core.h
index 3124cda3..b196b664 100644
--- a/folly/futures/detail/Core.h
+++ b/folly/futures/detail/Core.h
@@ -470,9 +470,11 @@ template <template <typename ...> class T, typename... Ts,
           typename THead, typename... TTail>
 void collectVariadicHelper(const std::shared_ptr<T<Ts...>>& ctx,
                            THead&& head, TTail&&... tail) {
-  head.setCallback_([ctx](Try<typename THead::value_type>&& t) {
-    ctx->template setPartialResult<typename THead::value_type,
-                                   sizeof...(Ts) - sizeof...(TTail) - 1>(t);
+  using ValueType = typename std::decay<THead>::type::value_type;
+  std::forward<THead>(head).setCallback_([ctx](Try<ValueType>&& t) {
+    ctx->template setPartialResult<
+        ValueType,
+        sizeof...(Ts) - sizeof...(TTail)-1>(t);
   });
   // template tail-recursion
   collectVariadicHelper(ctx, std::forward<TTail>(tail)...);
diff --git a/folly/futures/helpers.h b/folly/futures/helpers.h
index bc0b8b95..26187b8f 100644
--- a/folly/futures/helpers.h
+++ b/folly/futures/helpers.h
@@ -148,7 +148,7 @@ inline Future<Unit> via(
 /// easier to read and slightly more efficient.
 template <class Func>
 auto via(Executor*, Func&& func)
-  -> Future<typename isFuture<decltype(func())>::Inner>;
+    -> Future<typename isFuture<decltype(std::declval<Func>()())>::Inner>;
 
 /** When all the input Futures complete, the returned Future will complete.
   Errors do not cause early termination; this Future will always succeed