Refactor variadic collect and collectAll

Summary:
[Folly] Refactor variadic `collect` and `collectAll`.

No change in behavior - just moving code around. Principally, moving related code closer together, such as moving helper context types into the functions which use them.

Reviewed By: andriigrynenko

Differential Revision: D8416843

fbshipit-source-id: 3637c46c073191ae2dabc78cbaa145fe34cca220

folly/futures/Future-inl.h

@@ -1295,8 +1295,27 @@ Future<Unit> via(Executor* executor, int8_t priority) {
   return makeFuture().via(executor, priority);
 }
 
-// mapSetCallback calls func(i, Try<T>) when every future completes
+namespace futures {
+namespace detail {
+
+template <typename V, typename... Fs, std::size_t... Is>
+FOLLY_ALWAYS_INLINE FOLLY_ATTR_VISIBILITY_HIDDEN void
+foreach_(index_sequence<Is...>, V&& v, Fs&&... fs) {
+  using _ = int[];
+  void(_{0, (void(v(index_constant<Is>{}, static_cast<Fs&&>(fs))), 0)...});
+}
+template <typename V, typename... Fs>
+FOLLY_ALWAYS_INLINE FOLLY_ATTR_VISIBILITY_HIDDEN void foreach(
+    V&& v,
+    Fs&&... fs) {
+  using _ = index_sequence_for<Fs...>;
+  foreach_(_{}, static_cast<V&&>(v), static_cast<Fs&&>(fs)...);
+}
+
+} // namespace detail
+} // namespace futures
 
+// mapSetCallback calls func(i, Try<T>) when every future completes
 template <class T, class InputIterator, class F>
 void mapSetCallback(InputIterator first, InputIterator last, F func) {
   for (size_t i = 0; first != last; ++first, ++i) {
@@ -1309,20 +1328,31 @@ void mapSetCallback(InputIterator first, InputIterator last, F func) {
 // collectAll (variadic)
 
 template <typename... Fs>
-typename futures::detail::CollectAllVariadicContext<
-    typename std::decay<Fs>::type::value_type...>::type
+SemiFuture<std::tuple<Try<typename remove_cvref_t<Fs>::value_type>...>>
 collectAllSemiFuture(Fs&&... fs) {
-  auto ctx = std::make_shared<futures::detail::CollectAllVariadicContext<
-      typename std::decay<Fs>::type::value_type...>>();
-  futures::detail::collectVariadicHelper<
-      futures::detail::CollectAllVariadicContext>(ctx, std::forward<Fs>(fs)...);
+  using Result = std::tuple<Try<typename remove_cvref_t<Fs>::value_type>...>;
+  struct Context {
+    ~Context() {
+      p.setValue(std::move(results));
+    }
+    Promise<Result> p;
+    Result results;
+  };
+
+  auto ctx = std::make_shared<Context>();
+  futures::detail::foreach(
+      [&](auto i, auto&& f) {
+        f.setCallback_([i, ctx](auto&& t) {
+          std::get<i.value>(ctx->results) = std::move(t);
+        });
+      },
+      static_cast<Fs&&>(fs)...);
   return ctx->p.getSemiFuture();
 }
 
 template <typename... Fs>
-Future<typename futures::detail::CollectAllVariadicContext<
-    typename std::decay<Fs>::type::value_type...>::type::value_type>
-collectAll(Fs&&... fs) {
+Future<std::tuple<Try<typename remove_cvref_t<Fs>::value_type>...>> collectAll(
+    Fs&&... fs) {
   return collectAllSemiFuture(std::forward<Fs>(fs)...).toUnsafeFuture();
 }
 
@@ -1430,13 +1460,34 @@ collect(InputIterator first, InputIterator last) {
 
 // TODO(T26439406): Make return SemiFuture
 template <typename... Fs>
-typename futures::detail::CollectVariadicContext<
-    typename std::decay<Fs>::type::value_type...>::type
-collect(Fs&&... fs) {
-  auto ctx = std::make_shared<futures::detail::CollectVariadicContext<
-      typename std::decay<Fs>::type::value_type...>>();
-  futures::detail::collectVariadicHelper<
-      futures::detail::CollectVariadicContext>(ctx, std::forward<Fs>(fs)...);
+Future<std::tuple<typename remove_cvref_t<Fs>::value_type...>> collect(
+    Fs&&... fs) {
+  using Result = std::tuple<typename remove_cvref_t<Fs>::value_type...>;
+  struct Context {
+    ~Context() {
+      if (!threw.exchange(true)) {
+        p.setValue(unwrapTryTuple(std::move(results)));
+      }
+    }
+    Promise<Result> p;
+    std::tuple<Try<typename remove_cvref_t<Fs>::value_type>...> results;
+    std::atomic<bool> threw{false};
+  };
+
+  auto ctx = std::make_shared<Context>();
+  futures::detail::foreach(
+      [&](auto i, auto&& f) {
+        f.setCallback_([i, ctx](auto&& t) {
+          if (t.hasException()) {
+            if (!ctx->threw.exchange(true)) {
+              ctx->p.setException(std::move(t.exception()));
+            }
+          } else if (!ctx->threw) {
+            std::get<i.value>(ctx->results) = std::move(t);
+          }
+        });
+      },
+      static_cast<Fs&&>(fs)...);
   return ctx->p.getSemiFuture().via(&InlineExecutor::instance());
 }
 

folly/futures/Future-pre.h

@@ -72,8 +72,6 @@ namespace futures {
 namespace detail {
 
 template <class> class Core;
-template <class...> struct CollectAllVariadicContext;
-template <class...> struct CollectVariadicContext;
 template <class> struct CollectContext;
 
 template <typename...>

folly/futures/detail/Core.h

@@ -588,28 +588,6 @@ class Core final {
   std::function<void(exception_wrapper const&)> interruptHandler_ {nullptr};
 };
 
-template <template <typename...> class T, typename... Ts>
-void collectVariadicHelper(const std::shared_ptr<T<Ts...>>& /* ctx */) {
-  // base case
-}
-
-template <
-    template <typename...> class T,
-    typename... Ts,
-    typename THead,
-    typename... TTail>
-void collectVariadicHelper(const std::shared_ptr<T<Ts...>>& ctx,
-                           THead&& head, TTail&&... tail) {
-  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)...);
-}
-
 } // namespace detail
 } // namespace futures
 } // namespace folly

folly/futures/helpers.h

@@ -27,49 +27,6 @@
 
 namespace folly {
 
-namespace futures {
-namespace detail {
-template <typename... Ts>
-struct CollectAllVariadicContext {
-  CollectAllVariadicContext() {}
-  template <typename T, size_t I>
-  inline void setPartialResult(Try<T>& t) {
-    std::get<I>(results) = std::move(t);
-  }
-  ~CollectAllVariadicContext() {
-    p.setValue(std::move(results));
-  }
-  Promise<std::tuple<Try<Ts>...>> p;
-  std::tuple<Try<Ts>...> results;
-  typedef SemiFuture<std::tuple<Try<Ts>...>> type;
-};
-
-template <typename... Ts>
-struct CollectVariadicContext {
-  CollectVariadicContext() {}
-  template <typename T, size_t I>
-  inline void setPartialResult(Try<T>& t) {
-    if (t.hasException()) {
-      if (!threw.exchange(true)) {
-        p.setException(std::move(t.exception()));
-      }
-    } else if (!threw) {
-      std::get<I>(results) = std::move(t);
-    }
-  }
-  ~CollectVariadicContext() noexcept {
-    if (!threw.exchange(true)) {
-      p.setValue(unwrapTryTuple(std::move(results)));
-    }
-  }
-  Promise<std::tuple<Ts...>> p;
-  std::tuple<folly::Try<Ts>...> results;
-  std::atomic<bool> threw{false};
-  typedef Future<std::tuple<Ts...>> type;
-};
-} // namespace detail
-} // namespace futures
-
 /// This namespace is for utility functions that would usually be static
 /// members of Future, except they don't make sense there because they don't
 /// depend on the template type (rather, on the type of their arguments in
@@ -340,15 +297,12 @@ auto collectAll(Collection&& c) -> decltype(collectAll(c.begin(), c.end())) {
 /// is a Future<std::tuple<Try<T1>, Try<T2>, ...>>.
 /// The Futures are moved in, so your copies are invalid.
 template <typename... Fs>
-typename futures::detail::CollectAllVariadicContext<
-    typename std::decay<Fs>::type::value_type...>::type
+SemiFuture<std::tuple<Try<typename remove_cvref_t<Fs>::value_type>...>>
 collectAllSemiFuture(Fs&&... fs);
 
 template <typename... Fs>
-Future<typename futures::detail::CollectAllVariadicContext<
-    typename std::decay<Fs>::type::value_type...>::type::value_type>
-collectAll(Fs&&... fs);
-
+Future<std::tuple<Try<typename remove_cvref_t<Fs>::value_type>...>> collectAll(
+    Fs&&... fs);
 /// Like collectAll, but will short circuit on the first exception. Thus, the
 /// type of the returned Future is std::vector<T> instead of
 /// std::vector<Try<T>>
@@ -367,9 +321,8 @@ auto collect(Collection&& c) -> decltype(collect(c.begin(), c.end())) {
 /// type of the returned Future is std::tuple<T1, T2, ...> instead of
 /// std::tuple<Try<T1>, Try<T2>, ...>
 template <typename... Fs>
-typename futures::detail::CollectVariadicContext<
-    typename std::decay<Fs>::type::value_type...>::type
-collect(Fs&&... fs);
+Future<std::tuple<typename remove_cvref_t<Fs>::value_type...>> collect(
+    Fs&&... fs);
 
 /** The result is a pair of the index of the first Future to complete and
   the Try. If multiple Futures complete at the same time (or are already