fix(exec): make write_env transparent to sequence sender semantics - #2294
alwaysprince05 wants to merge 5 commits into
Conversation
A write_env sender wrapping a sequence sender was not itself a sequence sender: get_item_types has no branch that looks through stdexec sender adaptors, so sequence-aware algorithms like exec::transform_each treated the wrapper as a sequence of one and collapsed the item types. Make write_env senders transparent to sequence semantics: - specialize enable_sequence_sender for __write_env_t sexprs - compute item types of a write_env sender from its child in the joined environment (recursing through stacked wrappers) - subscribe to the child through a receiver that joins the written environment with the wrapped receiver's environment First half of NVIDIA#2053. The starts_on half goes through the continues_on machinery that NVIDIA#2277 is reworking, so it is left for a follow-up.
|
/ok to test |
| // semantics from downstream sequence-aware algorithms. See issue #2053. | ||
| template <auto _DescriptorFn> | ||
| requires STDEXEC::__same_as< | ||
| typename STDEXEC::__desc_of_t<STDEXEC::__sexpr<_DescriptorFn>>::__tag, |
There was a problem hiding this comment.
could this be simply:
| typename STDEXEC::__desc_of_t<STDEXEC::__sexpr<_DescriptorFn>>::__tag, | |
| STDEXEC::tag_of_t<STDEXEC::__sexpr<_DescriptorFn>>, |
?
There was a problem hiding this comment.
Done — thanks. The specialization now reaches the tag through tag_of_t (it actually goes through the new __transparent_sequence_adaptor concept described below, which queries it the same way). The hunk has moved because of the rework from your other comments, so GitHub couldn't apply the suggestion in place; the change is in 3b2abb0.
| // signatures from its child's signatures in the joined environment. | ||
| using __child_env_t = __join_env_t<__decay_t<__data_of<_Sequence>> const &, _Env...>; | ||
| return __get_item_types_helper<__child_of<_Sequence>, __child_env_t>(); | ||
| } |
There was a problem hiding this comment.
this is not the right fix. the set of sender adaptors is open. we can't hard-code all of them into into get_item_types. i'm not familiar enough with the sequence sender stuff to have a different suggestion.
There was a problem hiding this comment.
Fair point — fixed in 3b2abb0. Nothing in get_item_types (or subscribe) names an adaptor anymore.
There's now an open customization point next to enable_sequence_sender:
template <class _Tag>
struct __sequence_adaptor_traits
{
// Adaptors default to opaque.
static constexpr bool __transparent = false;
};An adaptor whose tag is marked transparent is treated as a wrapper that forwards set_next and all completions through to a single child. get_item_types unwraps such senders one layer at a time and computes the child's item types in the child's environment; a traits member __child_env_fn<_Env, _Data> names the environment transformation (identity by default). write_env is now just a specialization of that trait, declaring the child's environment as the written env joined with the query environment — the same thing its operation state presents when connecting. Since sequence_sender goes through enable_sequence_sender, marking the trait is the only thing an adaptor author writes.
To check that the extension point is actually open, the new tests include a user-defined adaptor that opts in by specializing the trait alone, with no changes to the machinery.
Happy to reshape this if you'd prefer a different form — e.g. splitting the flag into its own variable template, or hanging it off __sexpr_defaults instead.
| __check_operation_state<__result_t>(); | ||
| constexpr bool __nothrow_subscribe = __nothrow_callable<subscribe_t, __child_t, __rcvr_t>; | ||
| return __declfn<__result_t, __nothrow_subscribe && __nothrow_tfx_seq>(); | ||
| } |
There was a problem hiding this comment.
Addressed in 3b2abb0. This branch is generic now: it keys off the same __transparent_sequence_adaptor concept as get_item_types, takes the tag and data from the sender expression, computes the child's environment through the adaptor's __child_env_fn, and recurses into subscribe with a receiver that presents that environment (the wrapper formerly known as __write_env_rcvr, renamed to __adaptor_rcvr). No adaptor names appear here anymore.
| __write_env_rcvr<_Receiver, __written_t>{ | ||
| static_cast<_Receiver&&>(__rcvr), | ||
| STDEXEC::__forward_like<decltype(__env)>(__env)}); | ||
| } |
There was a problem hiding this comment.
Same rework here: the runtime branch handles any transparent adaptor — it destructures the expression, builds the child's environment through the adaptor's __child_env_fn, and recurses into the CPO one layer at a time, so stacked wrappers peel off one per level. Nothing write_env-specific remains in subscribe.
…oint Review feedback on NVIDIA#2294: get_item_types and subscribe must not hard-code individual sender adaptors, since the set of adaptors is open. Replace the write_env-specific branches with a generic mechanism driven by a new __sequence_adaptor_traits customization point: an adaptor that forwards set_next and all completions to a single child declares itself transparent for its tag, optionally naming the environment its child observes. write_env now only specializes that trait, and the sequence machinery handles every transparent adaptor the same way. The enable_sequence_sender specialization also goes through tag_of_t rather than digging into the sender's descriptor. Also merge upstream/main, and extend the regression tests with a user-defined adaptor that opts in via the new trait, to show that the extension point is open.
|
/ok to test |
Empty struct bodies written as `{ };` fail the style CI job, which runs
clang-format-21 --dry-run --Werror over all tracked sources.
🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
|
/ok to test |
|
@ericniebler Circling back — all four comments are addressed in 3b2abb0, with fbf50b4 on top of that just for a clang-format fix, and I've merged main back in so the branch is current. The Everything passes locally: Still happy to reshape the traits design if you'd rather hang it off |
This branch has not been deployed
2 participants
Problem
write_envaround a sequence sender breaks sequence semantics (#2053, first repro case). When the child ofwrite_envis a sequence sender, the wrapper is not one:get_item_typeshas no branch that looks through stdexec sender adaptors, soexec::subscribefalls back to treating the wrapper as a sequence of one. Downstream,exec::transform_eachthen sees the collapsed item types and the pipeline stops compiling:Fix
Make the sequence machinery see through environment-modifying adaptor senders, via an open customization point in
include/exec/sequence_senders.hpp— nothing adaptor-specific is hard-coded there, since the set of sender adaptors is open:__sequence_adaptor_traits, a traits template keyed on the adaptor's tag. Adaptors default to opaque. An adaptor that forwardsset_nextand all completions through to a single child declares itself transparent, optionally naming the environment its child observes with a nested__child_env_fn. Senders whose traits mark them transparent count as sequence senders throughenable_sequence_sender.get_item_typesandsubscribeeach gained one generic branch that unwraps any transparent adaptor one layer at a time — computing the child's item types, resp. subscribing to the child, in the transformed environment — so stacked wrappers keep working.write_envopts in with a single trait specialization declaring the child's environment as the written env joined with the query env, which is the same environmentwrite_env's own operation state presents to its child when connecting.Scope
This fixes the
write_envhalf of #2053. Thestarts_onhalf rewrites to__sequence(continues_on(just(), sched), child), which goes through thecontinues_onmachinery that #2277 is currently reworking, so I left it alone for now to avoid colliding with that work. Happy to take it up once #2277 settles.Partially addresses #2053.
Verification
write_env+transform_each) now compiles clean with-Wall -Wextra -Werror, and the item actually flows: I checked at runtime that thethen([](int))lambda receives 42, both with one and two stackedwrite_envwrappers.test/exec/sequence/test_write_env_sequence.cppcover item types (single and stacked wrappers) and the runtime item flow, plus a user-defined adaptor that opts in through the trait alone, to check the extension point is actually open.test.stdexec+test.execrebuilt and pass: 100% of tests, both in the normal build and withSTDEXEC_ENABLE_EXTRA_TYPE_CHECKING=ON, and the C++20 module-purview build still compiles and passes its suite.starts_oncase from the issue still fails as expected; it needs thecontinues_onwork mentioned above.