Skip to content

Commit 92c473c

Browse files
snarkmasterfacebook-github-bot
authored andcommitted
Add some missing customization points to NowTaskWithExecutor
Summary: - Explicitly test that you can't await a moved `NowTaskWithExecutor`. The manual test showed the correct "deleted copy ctor" error for this. - Plumb through `co_withCancellation` so that things like `timeout(nowTaskFn().scheduleOn(ex))` would work. - Plumb through `unsafeMoveMustAwaitImmediately()` so that generic wrappers (like `Sequence.h` and `Noexcept.h`) can wrap `NowTask`s into other `MustAwaitImmediately` containers. Reviewed By: ispeters Differential Revision: D70355306 fbshipit-source-id: 7adae485aa377181947b7d38de1c4da67947cf29
1 parent 7aa9b2c commit 92c473c

File tree

2 files changed

+51
-6
lines changed

2 files changed

+51
-6
lines changed

folly/coro/safe/NowTask.h

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,37 @@ template <typename T>
4848
class FOLLY_NODISCARD NowTaskWithExecutor final : private MustAwaitImmediately {
4949
protected:
5050
template <safe_alias, typename>
51-
friend class BackgroundTask; // see `CanUnwrapNowTask` below
51+
friend class BackgroundTask; // for `unwrapTaskWithExecutor`, remove later
5252
TaskWithExecutor<T> unwrapTaskWithExecutor() && { return std::move(inner_); }
5353

5454
template <typename>
5555
friend class NowTask; // can construct
5656

5757
explicit NowTaskWithExecutor(TaskWithExecutor<T> t) : inner_(std::move(t)) {}
5858

59+
public:
60+
// Required for `await_result_t` to work.
61+
// NB: Even though this is rvalue-qualified, this does not wrongly allow
62+
// `co_await std::move(myNowTask().scheduleOn(ex));`, see `withExecutor` test
63+
auto operator co_await() && noexcept {
64+
return std::move(inner_).operator co_await();
65+
}
66+
67+
NowTaskWithExecutor unsafeMoveMustAwaitImmediately() && {
68+
return NowTaskWithExecutor{std::move(inner_)};
69+
}
70+
71+
friend NowTaskWithExecutor co_withCancellation(
72+
folly::CancellationToken cancelToken, NowTaskWithExecutor te) noexcept {
73+
return NowTaskWithExecutor{
74+
co_withCancellation(std::move(cancelToken), std::move(te.inner_))};
75+
}
76+
77+
friend auto co_viaIfAsync(
78+
Executor::KeepAlive<> executor, NowTaskWithExecutor te) noexcept {
79+
return co_viaIfAsync(std::move(executor), std::move(te.inner_));
80+
}
81+
5982
private:
6083
TaskWithExecutor<T> inner_;
6184
};
@@ -91,7 +114,6 @@ class FOLLY_CORO_TASK_ATTRS NowTask final
91114
using promise_type = detail::NowTaskPromise<T>;
92115

93116
// If `makeNowTask().scheduleOn()` is movable, it defeats our purpose.
94-
FOLLY_NODISCARD
95117
NowTaskWithExecutor<T> scheduleOn(Executor::KeepAlive<> exec) && noexcept {
96118
return NowTaskWithExecutor<T>{
97119
std::move(*this).unwrap().scheduleOn(std::move(exec))};
@@ -121,13 +143,11 @@ class FOLLY_CORO_TASK_ATTRS NowTask final
121143
}
122144

123145
protected:
124-
// These 3 `friend`s (+ 1 above) are for `unwrap()`. If this list grows,
125-
// introduce a `CanUnwrapNowTask` passkey type.
126146
template <typename U>
127-
friend auto toNowTask(NowTask<U> t);
147+
friend auto toNowTask(NowTask<U> t); // for `unwrap`
128148
// `async_now_closure` wraps `NowTask`s into `NowTask`s
129149
template <auto>
130-
friend auto detail::bind_captures_to_closure(auto&&, auto);
150+
friend auto detail::bind_captures_to_closure(auto&&, auto); // for `unwrap`
131151
};
132152

133153
// NB: `toNowTask(SafeTask)` is in `SafeTask.h` to avoid circular deps.

folly/coro/safe/test/NowTaskTest.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222

2323
namespace folly::coro {
2424

25+
static_assert(std::is_void_v<await_result_t<NowTaskWithExecutor<void>>>);
26+
static_assert(std::is_same_v<int, await_result_t<NowTaskWithExecutor<int>>>);
27+
2528
Task<int> demoTask(int x) {
2629
co_return 1300 + x;
2730
}
@@ -83,6 +86,28 @@ CO_TEST(NowTaskTest, simple) {
8386
static_assert(!test_transform_moved_v<NowTask<int>&>);
8487
}
8588

89+
template <typename T>
90+
inline constexpr bool test_transform_moved_with_executor_v = std::is_same_v<
91+
detected_t<await_transform_result_t, T>,
92+
StackAwareViaIfAsyncAwaitable<TaskWithExecutor<int>>>;
93+
94+
CO_TEST(NowTaskTest, withExecutor) {
95+
auto exec = co_await co_current_executor;
96+
EXPECT_EQ(1337, co_await demoNowTask(37).scheduleOn(exec));
97+
98+
static_assert(test_transform_moved_with_executor_v<TaskWithExecutor<int>>);
99+
static_assert(test_transform_moved_with_executor_v<TaskWithExecutor<int>&&>);
100+
static_assert(test_transform_moved_with_executor_v<NowTaskWithExecutor<int>>);
101+
static_assert(
102+
!test_transform_moved_with_executor_v<NowTaskWithExecutor<int>&&>);
103+
#if 0 // The above asserts are a proxy for this manual test
104+
auto te = demoNowTask(37).scheduleOn(exec);
105+
co_await std::move(te);
106+
#endif
107+
static_assert(
108+
!test_transform_moved_with_executor_v<NowTaskWithExecutor<int>&>);
109+
}
110+
86111
// `co_nothrow` isn't a function object, so we can't wrap it & pass prvalues
87112
template <typename T>
88113
using co_nothrow_result_t = decltype(co_nothrow(FOLLY_DECLVAL(T)));

0 commit comments

Comments
 (0)