Commit 8c529eea authored by Lee Howes's avatar Lee Howes Committed by Facebook GitHub Bot

Remove collectXSemiFuture

Summary:
Migration from Future-returning executor-erasing collectX forms to
SemiFuture-returning forms, that are less risky in particular with coroutines.

Earlier changes added collectXSemiFuture and collectXUnsafe as a migration
path. We then migrated collectX callsites to collectXSemiFuture or
collectXUnsafe and switched the implementation of collectX to the SemiFuture
form.

This diff removes collectXSemiFuture completely.

Reviewed By: yfeldblum, mpark

Differential Revision: D21631886

fbshipit-source-id: c7b296d51f30c8b51ab0c7d06b1ce62c8cdd28d5
parent 8abb78f4
...@@ -1409,12 +1409,6 @@ collectAll(Fs&&... fs) { ...@@ -1409,12 +1409,6 @@ collectAll(Fs&&... fs) {
return future; return future;
} }
template <typename... Fs>
SemiFuture<std::tuple<Try<typename remove_cvref_t<Fs>::value_type>...>>
collectAllSemiFuture(Fs&&... fs) {
return collectAll(std::forward<Fs>(fs)...);
}
template <typename... Fs> template <typename... Fs>
Future<std::tuple<Try<typename remove_cvref_t<Fs>::value_type>...>> Future<std::tuple<Try<typename remove_cvref_t<Fs>::value_type>...>>
collectAllUnsafe(Fs&&... fs) { collectAllUnsafe(Fs&&... fs) {
...@@ -1477,13 +1471,6 @@ collectAllUnsafe(InputIterator first, InputIterator last) { ...@@ -1477,13 +1471,6 @@ collectAllUnsafe(InputIterator first, InputIterator last) {
return collectAll(first, last).toUnsafeFuture(); return collectAll(first, last).toUnsafeFuture();
} }
template <class InputIterator>
SemiFuture<std::vector<
Try<typename std::iterator_traits<InputIterator>::value_type::value_type>>>
collectAllSemiFuture(InputIterator first, InputIterator last) {
return collectAll(first, last);
}
// collect (iterator) // collect (iterator)
template <class InputIterator> template <class InputIterator>
...@@ -1549,13 +1536,6 @@ collectUnsafe(InputIterator first, InputIterator last) { ...@@ -1549,13 +1536,6 @@ collectUnsafe(InputIterator first, InputIterator last) {
return collect(first, last).toUnsafeFuture(); return collect(first, last).toUnsafeFuture();
} }
template <class InputIterator>
SemiFuture<std::vector<
typename std::iterator_traits<InputIterator>::value_type::value_type>>
collectSemiFuture(InputIterator first, InputIterator last) {
return collect(first, last);
}
// collect (variadic) // collect (variadic)
template <typename... Fs> template <typename... Fs>
...@@ -1603,38 +1583,14 @@ SemiFuture<std::tuple<typename remove_cvref_t<Fs>::value_type...>> collect( ...@@ -1603,38 +1583,14 @@ SemiFuture<std::tuple<typename remove_cvref_t<Fs>::value_type...>> collect(
return future; return future;
} }
template <typename... Fs>
[[deprecated("collectSemiFuture is deprecated and identical to plain collect. Please use collect instead.")]] SemiFuture<
std::tuple<typename remove_cvref_t<Fs>::value_type...>>
collectSemiFuture(Fs&&... fs) {
return collect(std::forward<Fs>(fs)...);
}
template <typename... Fs> template <typename... Fs>
Future<std::tuple<typename remove_cvref_t<Fs>::value_type...>> collectUnsafe( Future<std::tuple<typename remove_cvref_t<Fs>::value_type...>> collectUnsafe(
Fs&&... fs) { Fs&&... fs) {
return collect(std::forward<Fs>(fs)...).toUnsafeFuture(); return collect(std::forward<Fs>(fs)...).toUnsafeFuture();
} }
template <class Collection>
[[deprecated(
"collectSemiFuture is deprecated and identical to plain collect. Please use collect instead.")]] auto
collectSemiFuture(Collection&& c)
-> decltype(collectSemiFuture(c.begin(), c.end())) {
return collectSemiFuture(c.begin(), c.end());
}
// collectAny (iterator) // collectAny (iterator)
// TODO(T26439406): Make return SemiFuture
template <class InputIterator>
SemiFuture<std::pair<
size_t,
Try<typename std::iterator_traits<InputIterator>::value_type::value_type>>>
collectAnySemiFuture(InputIterator first, InputIterator last) {
return collectAny(first, last);
}
template <class InputIterator> template <class InputIterator>
Future<std::pair< Future<std::pair<
size_t, size_t,
......
...@@ -2365,27 +2365,12 @@ auto via(Executor::KeepAlive<>, Func&& func) -> Future< ...@@ -2365,27 +2365,12 @@ auto via(Executor::KeepAlive<>, Func&& func) -> Future<
last Future completes in. last Future completes in.
The return type for Future<T> input is a SemiFuture<std::vector<Try<T>>> The return type for Future<T> input is a SemiFuture<std::vector<Try<T>>>
for collectX and collectXSemiFuture. for collectX.
collectXUnsafe returns an inline Future that erases the executor from the collectXUnsafe returns an inline Future that erases the executor from the
incoming Futures/SemiFutures. collectXUnsafe should be phased out and incoming Futures/SemiFutures. collectXUnsafe should be phased out and
replaced with collectX(...).via(e) where e is a valid non-inline executor. replaced with collectX(...).via(e) where e is a valid non-inline executor.
*/ */
template <class InputIterator>
[[deprecated("collectAllSemiFuture is deprecated and identical to plain collectAll. Please use collectAll instead.")]] SemiFuture<
std::vector<Try<
typename std::iterator_traits<InputIterator>::value_type::value_type>>>
collectAllSemiFuture(InputIterator first, InputIterator last);
/// Sugar for the most common case
template <class Collection>
[[deprecated(
"collectAllSemiFuture is deprecated and identical to plain collectAll. Please use collectAll instead.")]] auto
collectAllSemiFuture(Collection&& c)
-> decltype(collectAllSemiFuture(c.begin(), c.end())) {
return collectAllSemiFuture(c.begin(), c.end());
}
// Unsafe variant, see above comment for details // Unsafe variant, see above comment for details
template <class InputIterator> template <class InputIterator>
Future<std::vector< Future<std::vector<
...@@ -2409,21 +2394,16 @@ auto collectAll(Collection&& c) -> decltype(collectAll(c.begin(), c.end())) { ...@@ -2409,21 +2394,16 @@ auto collectAll(Collection&& c) -> decltype(collectAll(c.begin(), c.end())) {
return 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 SemiFuture<std::tuple<Try<T1>, Try<T2>, ...>>.
/// The Futures are moved in, so your copies are invalid.
template <typename... Fs>
[[deprecated("collectAllSemiFuture is deprecated and identical to plain collectAll. Please use collectAll instead.")]] SemiFuture<
std::tuple<Try<typename remove_cvref_t<Fs>::value_type>...>>
collectAllSemiFuture(Fs&&... fs);
// Unsafe variant of collectAll, see coment above for details. Returns // Unsafe variant of collectAll, see coment above for details. Returns
// a Future<std::tuple<Try<T1>, Try<T2>, ...>> on the Inline executor. // a Future<std::tuple<Try<T1>, Try<T2>, ...>> on the Inline executor.
template <typename... Fs> template <typename... Fs>
Future<std::tuple<Try<typename remove_cvref_t<Fs>::value_type>...>> Future<std::tuple<Try<typename remove_cvref_t<Fs>::value_type>...>>
collectAllUnsafe(Fs&&... fs); collectAllUnsafe(Fs&&... fs);
/// This version takes a varying number of Futures instead of an iterator.
/// The return type for (Future<T1>, Future<T2>, ...) input
/// is a SemiFuture<std::tuple<Try<T1>, Try<T2>, ...>>.
/// The Futures are moved in, so your copies are invalid.
template <typename... Fs> template <typename... Fs>
SemiFuture<std::tuple<Try<typename remove_cvref_t<Fs>::value_type>...>> SemiFuture<std::tuple<Try<typename remove_cvref_t<Fs>::value_type>...>>
collectAll(Fs&&... fs); collectAll(Fs&&... fs);
...@@ -2482,14 +2462,6 @@ Future<std::pair< ...@@ -2482,14 +2462,6 @@ Future<std::pair<
Try<typename std::iterator_traits<InputIterator>::value_type::value_type>>> Try<typename std::iterator_traits<InputIterator>::value_type::value_type>>>
collectAnyUnsafe(InputIterator first, InputIterator last); collectAnyUnsafe(InputIterator first, InputIterator last);
template <class InputIterator>
[[deprecated("collectAnySemiFuture is deprecated and identical to plain collectAny. Please use collectAny instead.")]] SemiFuture<
std::pair<
size_t,
Try<typename std::iterator_traits<
InputIterator>::value_type::value_type>>>
collectAnySemiFuture(InputIterator first, InputIterator last);
/// Sugar for the most common case /// Sugar for the most common case
template <class Collection> template <class Collection>
auto collectAny(Collection&& c) -> decltype(collectAny(c.begin(), c.end())) { auto collectAny(Collection&& c) -> decltype(collectAny(c.begin(), c.end())) {
...@@ -2502,13 +2474,6 @@ auto collectAnyUnsafe(Collection&& c) ...@@ -2502,13 +2474,6 @@ auto collectAnyUnsafe(Collection&& c)
-> decltype(collectAnyUnsafe(c.begin(), c.end())) { -> decltype(collectAnyUnsafe(c.begin(), c.end())) {
return collectAnyUnsafe(c.begin(), c.end()); return collectAnyUnsafe(c.begin(), c.end());
} }
template <class Collection>
[[deprecated(
"collectAnySemiFuture is deprecated and identical to plain collectAny. Please use collectAny instead.")]] auto
collectAnySemiFuture(Collection&& c)
-> decltype(collectAnySemiFuture(c.begin(), c.end())) {
return collectAnySemiFuture(c.begin(), c.end());
}
/** Similar to collectAny, collectAnyWithoutException return the first Future to /** Similar to collectAny, collectAnyWithoutException return the first Future to
* complete without exceptions. If none of the future complete without * complete without exceptions. If none of the future complete without
......
...@@ -193,9 +193,7 @@ TEST(Collect, collectAllUnsafe) { ...@@ -193,9 +193,7 @@ TEST(Collect, collectAllUnsafe) {
} }
} }
FOLLY_PUSH_WARNING TEST(Collect, collectAllInline) {
FOLLY_GNU_DISABLE_WARNING("-Wdeprecated-declarations")
TEST(Collect, collectAllSemiFutureInline) {
// inline future collection on same executor // inline future collection on same executor
{ {
ManualExecutor x; ManualExecutor x;
...@@ -204,8 +202,7 @@ TEST(Collect, collectAllSemiFutureInline) { ...@@ -204,8 +202,7 @@ TEST(Collect, collectAllSemiFutureInline) {
futures.emplace_back(makeFuture(42).via(&x)); futures.emplace_back(makeFuture(42).via(&x));
futures.emplace_back(makeFuture(42).via(&x)); futures.emplace_back(makeFuture(42).via(&x));
auto allf = auto allf = collectAll(futures).via(&x).thenTryInline([](auto&&) {});
collectAllSemiFuture(futures).via(&x).thenTryInline([](auto&&) {});
EXPECT_FALSE(allf.isReady()); EXPECT_FALSE(allf.isReady());
EXPECT_EQ(3, x.run()); EXPECT_EQ(3, x.run());
EXPECT_TRUE(allf.isReady()); EXPECT_TRUE(allf.isReady());
...@@ -218,7 +215,7 @@ TEST(Collect, collectAllSemiFutureInline) { ...@@ -218,7 +215,7 @@ TEST(Collect, collectAllSemiFutureInline) {
futures.emplace_back(makeSemiFuture(42).defer([](auto&&) { return 42; })); futures.emplace_back(makeSemiFuture(42).defer([](auto&&) { return 42; }));
futures.emplace_back(makeSemiFuture(42).defer([](auto&&) { return 42; })); futures.emplace_back(makeSemiFuture(42).defer([](auto&&) { return 42; }));
auto allf = collectAllSemiFuture(futures).defer([](auto&&) {}).via(&x); auto allf = collectAll(futures).defer([](auto&&) {}).via(&x);
EXPECT_FALSE(allf.isReady()); EXPECT_FALSE(allf.isReady());
EXPECT_EQ(3, x.run()); EXPECT_EQ(3, x.run());
EXPECT_TRUE(allf.isReady()); EXPECT_TRUE(allf.isReady());
...@@ -230,7 +227,7 @@ TEST(Collect, collectAllSemiFutureInline) { ...@@ -230,7 +227,7 @@ TEST(Collect, collectAllSemiFutureInline) {
futures.emplace_back(makeFuture(42).via(&x1)); futures.emplace_back(makeFuture(42).via(&x1));
futures.emplace_back(makeFuture(42).via(&x2)); futures.emplace_back(makeFuture(42).via(&x2));
auto allf = collectAllSemiFuture(futures).defer([](auto&&) {}).via(&x1); auto allf = collectAll(futures).defer([](auto&&) {}).via(&x1);
EXPECT_FALSE(allf.isReady()); EXPECT_FALSE(allf.isReady());
EXPECT_EQ(1, x2.run()); EXPECT_EQ(1, x2.run());
EXPECT_FALSE(allf.isReady()); EXPECT_FALSE(allf.isReady());
...@@ -255,7 +252,6 @@ TEST(Collect, collectAllSemiFutureInline) { ...@@ -255,7 +252,6 @@ TEST(Collect, collectAllSemiFutureInline) {
EXPECT_TRUE(allf.isReady()); EXPECT_TRUE(allf.isReady());
} }
} }
FOLLY_POP_WARNING
TEST(Collect, collect) { TEST(Collect, collect) {
// success case // success case
...@@ -420,8 +416,6 @@ TEST(Collect, collectNotDefaultConstructible) { ...@@ -420,8 +416,6 @@ TEST(Collect, collectNotDefaultConstructible) {
} }
} }
FOLLY_PUSH_WARNING
FOLLY_GNU_DISABLE_WARNING("-Wdeprecated-declarations")
TEST(Collect, collectAny) { TEST(Collect, collectAny) {
{ {
std::vector<Promise<int>> promises(10); std::vector<Promise<int>> promises(10);
...@@ -489,7 +483,7 @@ TEST(Collect, collectAny) { ...@@ -489,7 +483,7 @@ TEST(Collect, collectAny) {
EXPECT_FALSE(f.isReady()); EXPECT_FALSE(f.isReady());
} }
auto anyf = collectAnySemiFuture(futures); auto anyf = collectAny(futures);
/* futures were moved in, so these are invalid now */ /* futures were moved in, so these are invalid now */
EXPECT_FALSE(anyf.isReady()); EXPECT_FALSE(anyf.isReady());
...@@ -543,7 +537,6 @@ TEST(Collect, collectAny) { ...@@ -543,7 +537,6 @@ TEST(Collect, collectAny) {
EXPECT_TRUE(anyf.isReady()); EXPECT_TRUE(anyf.isReady());
} }
} }
FOLLY_POP_WARNING
TEST(Collect, collectAnyWithoutException) { TEST(Collect, collectAnyWithoutException) {
auto& executor = folly::InlineExecutor::instance(); auto& executor = folly::InlineExecutor::instance();
......
...@@ -976,14 +976,12 @@ TEST(SemiFuture, semiFutureWithinNoValueReferenceWhenTimeOut) { ...@@ -976,14 +976,12 @@ TEST(SemiFuture, semiFutureWithinNoValueReferenceWhenTimeOut) {
std::move(f).get(); std::move(f).get();
} }
FOLLY_PUSH_WARNING TEST(SemiFuture, collectAllDeferredWork) {
FOLLY_GNU_DISABLE_WARNING("-Wdeprecated-declarations")
TEST(SemiFuture, collectAllSemiFutureDeferredWork) {
{ {
Promise<int> promise1; Promise<int> promise1;
Promise<int> promise2; Promise<int> promise2;
auto future = collectAllSemiFuture( auto future = collectAll(
promise1.getSemiFuture().deferValue([](int x) { return x * 2; }), promise1.getSemiFuture().deferValue([](int x) { return x * 2; }),
promise2.getSemiFuture().deferValue([](int x) { return x * 2; })); promise2.getSemiFuture().deferValue([](int x) { return x * 2; }));
...@@ -1002,7 +1000,7 @@ TEST(SemiFuture, collectAllSemiFutureDeferredWork) { ...@@ -1002,7 +1000,7 @@ TEST(SemiFuture, collectAllSemiFutureDeferredWork) {
Promise<int> promise1; Promise<int> promise1;
Promise<int> promise2; Promise<int> promise2;
auto future = collectAllSemiFuture( auto future = collectAll(
promise1.getSemiFuture().deferValue([](int x) { return x * 2; }), promise1.getSemiFuture().deferValue([](int x) { return x * 2; }),
promise2.getSemiFuture().deferValue([](int x) { return x * 2; })); promise2.getSemiFuture().deferValue([](int x) { return x * 2; }));
...@@ -1027,7 +1025,7 @@ TEST(SemiFuture, collectAllSemiFutureDeferredWork) { ...@@ -1027,7 +1025,7 @@ TEST(SemiFuture, collectAllSemiFutureDeferredWork) {
futures.push_back( futures.push_back(
promise2.getSemiFuture().deferValue([](int x) { return x * 2; })); promise2.getSemiFuture().deferValue([](int x) { return x * 2; }));
auto future = collectAllSemiFuture(futures); auto future = collectAll(futures);
promise1.setValue(1); promise1.setValue(1);
promise2.setValue(2); promise2.setValue(2);
...@@ -1045,23 +1043,20 @@ TEST(SemiFuture, collectAllSemiFutureDeferredWork) { ...@@ -1045,23 +1043,20 @@ TEST(SemiFuture, collectAllSemiFutureDeferredWork) {
{ {
Promise<int> promise; Promise<int> promise;
auto guard = makeGuard([&] { deferredDestroyed = true; }); auto guard = makeGuard([&] { deferredDestroyed = true; });
collectAllSemiFuture(promise.getSemiFuture().deferValue( collectAll(promise.getSemiFuture().deferValue(
[guard = std::move(guard)](int x) { return x; })); [guard = std::move(guard)](int x) { return x; }));
} }
EXPECT_TRUE(deferredDestroyed); EXPECT_TRUE(deferredDestroyed);
} }
} }
FOLLY_POP_WARNING
FOLLY_PUSH_WARNING TEST(SemiFuture, collectDeferredWork) {
FOLLY_GNU_DISABLE_WARNING("-Wdeprecated-declarations")
TEST(SemiFuture, collectSemiFutureDeferredWork) {
{ {
Promise<int> promise1; Promise<int> promise1;
Promise<int> promise2; Promise<int> promise2;
auto future = collectSemiFuture( auto future = collect(
promise1.getSemiFuture().deferValue([](int x) { return x * 2; }), promise1.getSemiFuture().deferValue([](int x) { return x * 2; }),
promise2.getSemiFuture().deferValue([](int x) { return x * 2; })); promise2.getSemiFuture().deferValue([](int x) { return x * 2; }));
...@@ -1080,7 +1075,7 @@ TEST(SemiFuture, collectSemiFutureDeferredWork) { ...@@ -1080,7 +1075,7 @@ TEST(SemiFuture, collectSemiFutureDeferredWork) {
Promise<int> promise1; Promise<int> promise1;
Promise<int> promise2; Promise<int> promise2;
auto future = collectSemiFuture( auto future = collect(
promise1.getSemiFuture().deferValue([](int x) { return x * 2; }), promise1.getSemiFuture().deferValue([](int x) { return x * 2; }),
promise2.getSemiFuture().deferValue([](int x) { return x * 2; })); promise2.getSemiFuture().deferValue([](int x) { return x * 2; }));
...@@ -1105,7 +1100,7 @@ TEST(SemiFuture, collectSemiFutureDeferredWork) { ...@@ -1105,7 +1100,7 @@ TEST(SemiFuture, collectSemiFutureDeferredWork) {
futures.push_back( futures.push_back(
promise2.getSemiFuture().deferValue([](int x) { return x * 2; })); promise2.getSemiFuture().deferValue([](int x) { return x * 2; }));
auto future = collectSemiFuture(futures); auto future = collect(futures);
promise1.setValue(1); promise1.setValue(1);
promise2.setValue(2); promise2.setValue(2);
...@@ -1123,14 +1118,13 @@ TEST(SemiFuture, collectSemiFutureDeferredWork) { ...@@ -1123,14 +1118,13 @@ TEST(SemiFuture, collectSemiFutureDeferredWork) {
{ {
Promise<int> promise; Promise<int> promise;
auto guard = makeGuard([&] { deferredDestroyed = true; }); auto guard = makeGuard([&] { deferredDestroyed = true; });
collectSemiFuture(promise.getSemiFuture().deferValue( collect(promise.getSemiFuture().deferValue(
[guard = std::move(guard)](int x) { return x; })); [guard = std::move(guard)](int x) { return x; }));
} }
EXPECT_TRUE(deferredDestroyed); EXPECT_TRUE(deferredDestroyed);
} }
} }
FOLLY_POP_WARNING
TEST(SemiFuture, collectNDeferredWork) { TEST(SemiFuture, collectNDeferredWork) {
Promise<int> promise1; Promise<int> promise1;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment