Skip to content

Commit ba47708

Browse files
snarkmasterfacebook-github-bot
authored andcommitted
Some no-op fixes (comments + static_assert)
Summary: - Some of the `SafeAlias` comments were stale / wrong. - Per ispeters's comment on D74771290, it would be a footgun to use `OnCancel<T>` with a type that can throw when copied, so let's explicitly prohibit that. Reviewed By: ispeters Differential Revision: D75415739 fbshipit-source-id: c799508c2aaa0f170bb696f3144faa4cf1d59402
1 parent f5acb81 commit ba47708

File tree

2 files changed

+14
-10
lines changed

2 files changed

+14
-10
lines changed

third-party/folly/src/folly/coro/Noexcept.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,10 @@ inline constexpr TerminateOnCancel terminateOnCancel{};
9494
template <typename T>
9595
struct OnCancel {
9696
T privateVal_; // only `public` to make this a structural type
97-
T onCancelDefaultValue() const noexcept { return privateVal_; }
97+
T onCancelDefaultValue() const noexcept {
98+
static_assert(std::is_nothrow_copy_constructible_v<T>);
99+
return privateVal_;
100+
}
98101
consteval explicit OnCancel(T t) : privateVal_{std::move(t)} {}
99102
};
100103

third-party/folly/src/folly/coro/safe/SafeAlias.h

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -142,17 +142,18 @@ enum class safe_alias {
142142
// the `Captures.h` mechanism that discourages moving `async_closure`
143143
// capture wrappers out of the closure that owns it (we can't prevent it).
144144
closure_min_arg_safety = shared_cleanup,
145-
// Used only in `async_closure*()`:
146-
// - Valid ONLY until the current closure's cleanup starts.
147-
// - ONLY safe to pass to sub-closures
148-
// - NOT safe to return or pass to callbacks from ancestor closures
149-
// - NOT safe to pass to callbacks of the same closure's cleanup args
150-
// (e.g. `SafeAsyncScope`).
145+
// Used only in `async_closure*()` when it takes a `co_cleanup_capture` ref
146+
// from a parent:
147+
// - NOT safe to reference from tasks spawned on `co_cleanup_capture` args.
148+
// - Otherwise, just like `co_cleanup_safe_ref`.
151149
after_cleanup_ref,
152150
// Used only in `async_closure*()`:
153-
// - Valid until the end of the current closure's cleanup.
154-
// - Safe to pass to sub-closures, or to callbacks from this
155-
// closure's `SafeAsyncScope` (& other `co_cleanup()` args).
151+
// - Unlike `after_cleanup_ref`, is safe to reference from tasks spawned on
152+
// `co_cleanup_capture` args -- because we know these belong to the
153+
// current closure.
154+
// - Outlives the end of the current closure's cleanup, and is thus safe to
155+
// use in `after_cleanup{}` or sub-closures.
156+
// - Safe to pass to sub-closures.
156157
// - NOT safe to return or pass to callbacks from ancestor closures.
157158
co_cleanup_safe_ref,
158159
// Looks like a "value", i.e. alive as long as you hold it. Remember

0 commit comments

Comments
 (0)