Skip to content

move subscribe's customization point into stdexec as __subscribe - #2443

Open
alwaysprince05 wants to merge 2 commits into
NVIDIA:mainfrom
alwaysprince05:refactor/subscribe-cpo
Open

alwaysprince05 wants to merge 2 commits into
NVIDIA:mainfrom
alwaysprince05:refactor/subscribe-cpo

Conversation

@alwaysprince05

Copy link
Copy Markdown
Contributor

Step 1 from the discussion in #2439.

The lookup for __static_subscribe / .subscribe() now lives in STDEXEC::__subscribe (same idea as __submit), and exec::subscribe calls it. exec::subscribe stays the public name and the lookup order is the same, so no behavior change.

This lets senders in stdexec hook subscribe without depending on exec. Next PR has __seq::__sndr do that so starts_on works with sequence senders.

Added a test for the hooks in test_sequence_senders.cpp. test.exec and test.stdexec pass locally, also checked with g++-16 and with STDEXEC_ENABLE_EXTRA_TYPE_CHECKING, clang-format 21.1.8 clean.

exec::subscribe used to look for __static_subscribe / .subscribe() itself.
That lookup now lives in STDEXEC::__subscribe (like __submit), and
exec::subscribe calls it, so senders defined in stdexec can hook subscribe
without depending on exec. exec::subscribe stays the public name and the
lookup order is unchanged.
@copy-pr-bot

copy-pr-bot Bot commented Nov 10, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

concept __subscribable_with_static_member =
__minvocable_q<__subscribe_static_member_result_t, _Sequence, _Receiver>;

struct subscribe_t

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would expect this to become a simple alias for STDEXEC::__subscribe_::__subscribe_t. why not?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the rest of subscribe_t uses set_next / next_sender_of_t, __stopped_means_break, item_types / sequence_sender_in and the adaptor bits from #2294, and those all live in exec right now. so making it an alias means moving most of the sequence sender basics into stdexec/__detail too. i kept this PR to just the hook lookup to keep it small, but happy to move all of that here if that's what you'd like. or would you rather have that as its own PR first?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's fine, i'm just making sure i'm understanding the PR correctly.

constexpr auto operator()(_Sender&& __sndr, _Receiver __rcvr) const
noexcept(noexcept(STDEXEC_REMOVE_REFERENCE(
_Sender)::__static_subscribe(__declval<_Sender>(), __declval<_Receiver>())))
-> decltype(STDEXEC_REMOVE_REFERENCE(_Sender)::__static_subscribe(__declval<_Sender>(),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

STDEXEC_REMOVE_REFERENCE sometimes expands into a builtin, and in my experience gcc does not like mangling builtins so they shouldn't show up in function signatures. but i'll run CI and see what it says.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you were right, gcc 13-15 and clang 16 rejected the builtin in the signature. switched to calling __static_subscribe through the object like __submit does, in 295304f. checked locally with gcc 14 (test.exec and test.stdexec pass).

@ericniebler

Copy link
Copy Markdown
Collaborator

/ok to test bce52d1

gcc rejects builtin traits in function signatures since it cannot mangle
them. call __static_subscribe through the object instead, like __submit
does.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants