Make collect work for types with no default constructors

Summary: This doesn't make the code less efficient, because RVO can't be used for CollectVariadicContext. Thus moving existing tuple vs constructing new tuple by moving all values from other tuple (where each value is wrapped in folly::Optional) should be pretty much the same.

Reviewed By: hannesr

Differential Revision: D2650293

fb-gh-sync-id: 648a358bf093a0bb9d058a997af9bf59014ad77c

diff --git a/folly/futures/detail/Core.h b/folly/futures/detail/Core.h
index bca540decf292449613af333cf24f10ea8b4a775..b0bb2ff778ddea99ee09139ae9a36426688971dc 100644 (file)
--- a/folly/futures/detail/Core.h
+++ b/folly/futures/detail/Core.h
@@ -410,18 +410,37 @@ struct CollectVariadicContext {
          p.setException(std::move(t.exception()));
        }
      } else if (!threw) {
-       std::get<I>(results) = std::move(t.value());
+       std::get<I>(results) = std::move(t);
      }
   }
   ~CollectVariadicContext() {
     if (!threw.exchange(true)) {
-      p.setValue(std::move(results));
+      p.setValue(unwrap(std::move(results)));
     }
   }
   Promise<std::tuple<Ts...>> p;
-  std::tuple<Ts...> results;
+  std::tuple<folly::Try<Ts>...> results;
   std::atomic<bool> threw {false};
   typedef Future<std::tuple<Ts...>> type;
+
+ private:
+  template <typename... Ts2>
+  static std::tuple<Ts...> unwrap(std::tuple<folly::Try<Ts>...>&& o,
+                                  Ts2&&... ts2) {
+    static_assert(sizeof...(ts2) <
+                  std::tuple_size<std::tuple<folly::Try<Ts>...>>::value,
+                  "Non-templated unwrap should be used instead");
+    assert(std::get<sizeof...(ts2)>(o).hasValue());
+
+    return unwrap(std::move(o),
+                  std::forward<Ts2>(ts2)...,
+                  std::move(*std::get<sizeof...(ts2)>(o)));
+  }
+
+  static std::tuple<Ts...> unwrap(std::tuple<folly::Try<Ts>...>&& o,
+                                  Ts&&... ts) {
+    return std::tuple<Ts...>(std::forward<Ts>(ts)...);
+  }
 };
 
 template <template <typename ...> class T, typename... Ts>

diff --git a/folly/futures/test/CollectTest.cpp b/folly/futures/test/CollectTest.cpp
index 4c75fa5d87cf7dcf019fdab7da33e6008380bec9..e74fe9db597961a7e675ef436bc023b87b97ab5c 100644 (file)
--- a/folly/futures/test/CollectTest.cpp
+++ b/folly/futures/test/CollectTest.cpp
@@ -630,3 +630,14 @@ TEST(Collect, collectAllNone) {
   auto f = collectAll(fs);
   EXPECT_TRUE(f.isReady());
 }
+
+TEST(Collect, noDefaultConstructor) {
+  struct A {
+    explicit A(size_t x) {}
+  };
+
+  auto f1 = makeFuture(A(1));
+  auto f2 = makeFuture(A(2));
+
+  auto f = collect(std::move(f1), std::move(f2));
+}