From: James Sedgwick <jsedgwick@fb.com>
Date: Wed, 22 Apr 2015 21:27:36 +0000 (-0700)
Subject: take collections by reference
X-Git-Tag: v0.36.0~11
X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=03b14b9cb09c07e582c9671bbb0ba85a545522a6;p=folly.git

take collections by reference

Summary: This covers the ubiquitous case. If the approach is approved (the task mentioned changing impl to use 'auto for' so this is a bit different) i'll codemod callsites

Test Plan: unit

Reviewed By: hans@fb.com

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

FB internal diff: D2013573

Tasks: 6779710

Signature: t1:2013573:1429735631:cfa0d3f6672a8966afc0ea18308307e2153793ce
---

diff --git a/folly/futures/Future.h b/folly/futures/Future.h
index 2df34b79..a14d62cf 100644
--- a/folly/futures/Future.h
+++ b/folly/futures/Future.h
@@ -191,6 +191,13 @@ namespace futures {
             class Result = decltype(std::declval<ItT>().then(std::declval<F>()))>
   std::vector<Future<Result>> map(It first, It last, F func);
 
+  // Sugar for the most common case
+  template <class Collection, class F>
+  auto map(Collection&& c, F&& func)
+      -> decltype(map(c.begin(), c.end(), func)) {
+    return map(c.begin(), c.end(), std::forward<F>(func));
+  }
+
 }
 
 template <class T>
@@ -643,6 +650,12 @@ Future<std::vector<Try<
   typename std::iterator_traits<InputIterator>::value_type::value_type>>>
 collectAll(InputIterator first, InputIterator last);
 
+// Sugar for the most common case
+template <class Collection>
+auto collectAll(Collection&& c) -> decltype(collectAll(c.begin(), c.end())) {
+  return collectAll(c.begin(), c.end());
+}
+
 /// This version takes a varying number of Futures instead of an iterator.
 /// The return type for (Future<T1>, Future<T2>, ...) input
 /// is a Future<std::tuple<Try<T1>, Try<T2>, ...>>.
@@ -661,6 +674,12 @@ Future<typename detail::CollectContext<
 >::result_type>
 collect(InputIterator first, InputIterator last);
 
+// Sugar for the most common case
+template <class Collection>
+auto collect(Collection&& c) -> decltype(collect(c.begin(), c.end())) {
+  return collect(c.begin(), c.end());
+}
+
 /** 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
   complete when passed in), the "winner" is chosen non-deterministically.
@@ -673,6 +692,12 @@ Future<std::pair<
   Try<typename std::iterator_traits<InputIterator>::value_type::value_type>>>
 collectAny(InputIterator first, InputIterator last);
 
+// Sugar for the most common case
+template <class Collection>
+auto collectAny(Collection&& c) -> decltype(collectAny(c.begin(), c.end())) {
+  return collectAny(c.begin(), c.end());
+}
+
 /** when n Futures have completed, the Future completes with a vector of
   the index and Try of those n Futures (the indices refer to the original
   order, but the result vector will be in an arbitrary order)
@@ -685,6 +710,13 @@ Future<std::vector<std::pair<
   Try<typename std::iterator_traits<InputIterator>::value_type::value_type>>>>
 collectN(InputIterator first, InputIterator last, size_t n);
 
+// Sugar for the most common case
+template <class Collection>
+auto collectN(Collection&& c, size_t n)
+    -> decltype(collectN(c.begin(), c.end(), n)) {
+  return collectN(c.begin(), c.end(), n);
+}
+
 template <typename F, typename T, typename ItT>
 using MaybeTryArg = typename std::conditional<
   detail::callableWith<F, T&&, Try<ItT>&&>::value, Try<ItT>, ItT>::type;
@@ -711,6 +743,17 @@ template <class It, class T, class F,
 typename std::enable_if<isFutureResult<F, T, Arg>::value, Future<T>>::type
 reduce(It first, It last, T initial, F func);
 
+// Sugar for the most common case
+template <class Collection, class T, class F>
+auto reduce(Collection&& c, T&& initial, F&& func)
+    -> decltype(reduce(c.begin(), c.end(), initial, func)) {
+  return reduce(
+      c.begin(),
+      c.end(),
+      std::forward<T>(initial),
+      std::forward<F>(func));
+}
+
 } // folly
 
 #include <folly/futures/Future-inl.h>
diff --git a/folly/futures/test/FutureTest.cpp b/folly/futures/test/FutureTest.cpp
index 00a56be6..ad99f3fd 100644
--- a/folly/futures/test/FutureTest.cpp
+++ b/folly/futures/test/FutureTest.cpp
@@ -699,7 +699,7 @@ TEST(Future, collectAll) {
     for (auto& p : promises)
       futures.push_back(p.getFuture());
 
-    auto allf = collectAll(futures.begin(), futures.end());
+    auto allf = collectAll(futures);
 
     random_shuffle(promises.begin(), promises.end());
     for (auto& p : promises) {
@@ -722,7 +722,7 @@ TEST(Future, collectAll) {
     for (auto& p : promises)
       futures.push_back(p.getFuture());
 
-    auto allf = collectAll(futures.begin(), futures.end());
+    auto allf = collectAll(futures);
 
 
     promises[0].setValue(42);
@@ -754,7 +754,7 @@ TEST(Future, collectAll) {
     for (auto& p : promises)
       futures.push_back(p.getFuture());
 
-    auto allf = collectAll(futures.begin(), futures.end())
+    auto allf = collectAll(futures)
       .then([](Try<vector<Try<void>>>&& ts) {
         for (auto& f : ts.value())
           f.value();
@@ -776,7 +776,7 @@ TEST(Future, collect) {
     for (auto& p : promises)
       futures.push_back(p.getFuture());
 
-    auto allf = collect(futures.begin(), futures.end());
+    auto allf = collect(futures);
 
     random_shuffle(promises.begin(), promises.end());
     for (auto& p : promises) {
@@ -798,7 +798,7 @@ TEST(Future, collect) {
     for (auto& p : promises)
       futures.push_back(p.getFuture());
 
-    auto allf = collect(futures.begin(), futures.end());
+    auto allf = collect(futures);
 
     random_shuffle(promises.begin(), promises.end());
     for (int i = 0; i < 10; i++) {
@@ -833,7 +833,7 @@ TEST(Future, collect) {
     for (auto& p : promises)
       futures.push_back(p.getFuture());
 
-    auto allf = collect(futures.begin(), futures.end());
+    auto allf = collect(futures);
 
     random_shuffle(promises.begin(), promises.end());
     for (auto& p : promises) {
@@ -852,7 +852,7 @@ TEST(Future, collect) {
     for (auto& p : promises)
       futures.push_back(p.getFuture());
 
-    auto allf = collect(futures.begin(), futures.end());
+    auto allf = collect(futures);
 
     random_shuffle(promises.begin(), promises.end());
     for (int i = 0; i < 10; i++) {
@@ -887,7 +887,7 @@ TEST(Future, collect) {
     for (auto& p : promises)
       futures.push_back(p.getFuture());
 
-    collect(futures.begin(), futures.end());
+    collect(futures);
   }
 
 }
@@ -910,7 +910,7 @@ TEST(Future, collectNotDefaultConstructible) {
   for (auto& p : promises)
     futures.push_back(p.getFuture());
 
-  auto allf = collect(futures.begin(), futures.end());
+  auto allf = collect(futures);
 
   for (auto i : indices) {
     EXPECT_FALSE(allf.isReady());
@@ -937,7 +937,7 @@ TEST(Future, collectAny) {
       EXPECT_FALSE(f.isReady());
     }
 
-    auto anyf = collectAny(futures.begin(), futures.end());
+    auto anyf = collectAny(futures);
 
     /* futures were moved in, so these are invalid now */
     EXPECT_FALSE(anyf.isReady());
@@ -965,7 +965,7 @@ TEST(Future, collectAny) {
       EXPECT_FALSE(f.isReady());
     }
 
-    auto anyf = collectAny(futures.begin(), futures.end());
+    auto anyf = collectAny(futures);
 
     EXPECT_FALSE(anyf.isReady());
 
@@ -982,7 +982,7 @@ TEST(Future, collectAny) {
     for (auto& p : promises)
       futures.push_back(p.getFuture());
 
-    auto anyf = collectAny(futures.begin(), futures.end())
+    auto anyf = collectAny(futures)
       .then([](pair<size_t, Try<int>> p) {
         EXPECT_EQ(42, p.second.value());
       });
@@ -999,7 +999,7 @@ TEST(when, already_completed) {
     for (int i = 0; i < 10; i++)
       fs.push_back(makeFuture());
 
-    collectAll(fs.begin(), fs.end())
+    collectAll(fs)
       .then([&](vector<Try<void>> ts) {
         EXPECT_EQ(fs.size(), ts.size());
       });
@@ -1009,7 +1009,7 @@ TEST(when, already_completed) {
     for (int i = 0; i < 10; i++)
       fs.push_back(makeFuture(i));
 
-    collectAny(fs.begin(), fs.end())
+    collectAny(fs)
       .then([&](pair<size_t, Try<int>> p) {
         EXPECT_EQ(p.first, p.second.value());
       });
@@ -1025,7 +1025,7 @@ TEST(when, collectN) {
 
   bool flag = false;
   size_t n = 3;
-  collectN(futures.begin(), futures.end(), n)
+  collectN(futures, n)
     .then([&](vector<pair<size_t, Try<void>>> v) {
       flag = true;
       EXPECT_EQ(n, v.size());
@@ -1056,7 +1056,7 @@ TEST(when, small_vector) {
     for (int i = 0; i < 10; i++)
       futures.push_back(makeFuture());
 
-    auto anyf = collectAny(futures.begin(), futures.end());
+    auto anyf = collectAny(futures);
   }
 
   {
@@ -1065,7 +1065,7 @@ TEST(when, small_vector) {
     for (int i = 0; i < 10; i++)
       futures.push_back(makeFuture());
 
-    auto allf = collectAll(futures.begin(), futures.end());
+    auto allf = collectAll(futures);
   }
 }
 
@@ -1111,7 +1111,7 @@ TEST(Future, collectAllVariadicReferences) {
 
 TEST(Future, collectAll_none) {
   vector<Future<int>> fs;
-  auto f = collectAll(fs.begin(), fs.end());
+  auto f = collectAll(fs);
   EXPECT_TRUE(f.isReady());
 }
 
@@ -1156,13 +1156,13 @@ TEST(Future, waitImmediate) {
   vector<Future<void>> v_f;
   v_f.push_back(makeFuture());
   v_f.push_back(makeFuture());
-  auto done_v_f = collectAll(v_f.begin(), v_f.end()).wait().value();
+  auto done_v_f = collectAll(v_f).wait().value();
   EXPECT_EQ(2, done_v_f.size());
 
   vector<Future<bool>> v_fb;
   v_fb.push_back(makeFuture(true));
   v_fb.push_back(makeFuture(false));
-  auto fut = collectAll(v_fb.begin(), v_fb.end());
+  auto fut = collectAll(v_fb);
   auto done_v_fb = std::move(fut.wait().value());
   EXPECT_EQ(2, done_v_fb.size());
 }
@@ -1262,7 +1262,7 @@ TEST(Future, waitWithDuration) {
   vector<Future<bool>> v_fb;
   v_fb.push_back(makeFuture(true));
   v_fb.push_back(makeFuture(false));
-  auto f = collectAll(v_fb.begin(), v_fb.end());
+  auto f = collectAll(v_fb);
   f.wait(milliseconds(1));
   EXPECT_TRUE(f.isReady());
   EXPECT_EQ(2, f.value().size());
@@ -1273,7 +1273,7 @@ TEST(Future, waitWithDuration) {
   Promise<bool> p2;
   v_fb.push_back(p1.getFuture());
   v_fb.push_back(p2.getFuture());
-  auto f = collectAll(v_fb.begin(), v_fb.end());
+  auto f = collectAll(v_fb);
   f.wait(milliseconds(1));
   EXPECT_FALSE(f.isReady());
   p1.setValue(true);
@@ -1485,7 +1485,7 @@ TEST(Future, t5506504) {
       for (auto& p : *promises) p.setValue();
     });
 
-    return collectAll(futures.begin(), futures.end());
+    return collectAll(futures);
   };
 
   fn().wait();
@@ -1691,7 +1691,7 @@ TEST(Reduce, Basic) {
   {
     auto fs = makeFutures(0);
 
-    Future<double> f1 = reduce(fs.begin(), fs.end(), 1.2,
+    Future<double> f1 = reduce(fs, 1.2,
       [](double a, Try<int>&& b){
         return a + *b + 0.1;
       });
@@ -1702,7 +1702,7 @@ TEST(Reduce, Basic) {
   {
     auto fs = makeFutures(1);
 
-    Future<double> f1 = reduce(fs.begin(), fs.end(), 0.0,
+    Future<double> f1 = reduce(fs, 0.0,
       [](double a, Try<int>&& b){
         return a + *b + 0.1;
       });
@@ -1713,7 +1713,7 @@ TEST(Reduce, Basic) {
   {
     auto fs = makeFutures(3);
 
-    Future<double> f1 = reduce(fs.begin(), fs.end(), 0.0,
+    Future<double> f1 = reduce(fs, 0.0,
       [](double a, Try<int>&& b){
         return a + *b + 0.1;
       });
@@ -1724,7 +1724,7 @@ TEST(Reduce, Basic) {
   {
     auto fs = makeFutures(3);
 
-    Future<double> f1 = reduce(fs.begin(), fs.end(), 0.0,
+    Future<double> f1 = reduce(fs, 0.0,
       [](double a, int&& b){
         return a + b + 0.1;
       });
@@ -1735,7 +1735,7 @@ TEST(Reduce, Basic) {
   {
     auto fs = makeFutures(3);
 
-    Future<double> f2 = reduce(fs.begin(), fs.end(), 0.0,
+    Future<double> f2 = reduce(fs, 0.0,
       [](double a, Try<int>&& b){
         return makeFuture<double>(a + *b + 0.1);
       });
@@ -1746,7 +1746,7 @@ TEST(Reduce, Basic) {
   {
     auto fs = makeFutures(3);
 
-    Future<double> f2 = reduce(fs.begin(), fs.end(), 0.0,
+    Future<double> f2 = reduce(fs, 0.0,
       [](double a, int&& b){
         return makeFuture<double>(a + b + 0.1);
       });
@@ -1765,7 +1765,7 @@ TEST(Map, Basic) {
   fs.push_back(p3.getFuture());
 
   int c = 0;
-  auto fs2 = futures::map(fs.begin(), fs.end(), [&](int i){
+  auto fs2 = futures::map(fs, [&](int i){
     c += i;
   });
 
@@ -1777,5 +1777,5 @@ TEST(Map, Basic) {
   p1.setValue(1);
   EXPECT_EQ(3, c);
 
-  EXPECT_TRUE(collect(fs2.begin(), fs2.end()).isReady());
+  EXPECT_TRUE(collect(fs2).isReady());
 }