Skip to content

Comments

rewrite sender-adaptor-closure infra#220

Merged
dietmarkuehl merged 6 commits intobemanproject:mainfrom
ecoezen:sender-adaptor-closure
Feb 20, 2026
Merged

rewrite sender-adaptor-closure infra#220
dietmarkuehl merged 6 commits intobemanproject:mainfrom
ecoezen:sender-adaptor-closure

Conversation

@ecoezen
Copy link
Collaborator

@ecoezen ecoezen commented Feb 18, 2026

  • Refactor sender-adaptor closure infrastructure around sender_adaptor_closure, adaptor_closure, and composed closure piping.
  • Migrate adaptor implementations (then, bulk, let, continues_on, associate, on, affine_on) to the closure-based path.
  • Enforce [exec.adapt.obj] constraints and behavior: pipe equivalence, composition semantics, construction well-formedness, and noexcept propagation.
  • Expand sender_adaptor_closure tests for uniqueness detection, composition associativity, composed call pattern, partial application well-formedness, and pipe parity.
  • Replace to and deprecate direct detail::sender_adaptor alias in favor of make_sender_adaptor(...) factories and document the direct-use layout break (Adaptor moved into adaptor_closure_binding with [[no_unique_address]] for EBO, pending product_type support).
  • Apply quality cleanup in sender_adaptor_closure module/header exports and comments.
  • Remove wrong assumptions about adaptor closures on [exec-on.test.cpp: struct both] per https://eel.is/c++draft/exec.adapt#obj-2 : closure cannot be a sender at the same time.
  • Add docs in headers.

Resolves #198

@ecoezen ecoezen force-pushed the sender-adaptor-closure branch 2 times, most recently from ed25811 to 8a21a3c Compare February 18, 2026 22:42
@coveralls
Copy link

coveralls commented Feb 19, 2026

Coverage Status

coverage: 95.14% (+0.02%) from 95.116%
when pulling a0c141a on ecoezen:sender-adaptor-closure
into 07d53da on bemanproject:main.

Copy link
Member

@dietmarkuehl dietmarkuehl left a comment

Choose a reason for hiding this comment

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

Nice! There are few things which may be changed.

@ecoezen
Copy link
Collaborator Author

ecoezen commented Feb 20, 2026

@dietmarkuehl please also let me know if i should fully qualify names. i'm preparing it anyway, but not sure what to fully qualify.

my feedback would be; fully qualifying everything makes things uglier and harder to read. beman/execution should be more readable and easier to follow than stdexec/execution imo. the community really needs a readable implementation to quickly understand the mechanics of this library. fully qualifying names increases cognitive effort.

@dietmarkuehl
Copy link
Member

@dietmarkuehl please also let me know if i should fully qualify names. i'm preparing it anyway, but not sure what to fully qualify.

my feedback would be; fully qualifying everything makes things uglier and harder to read. beman/execution should be more readable and easier to follow than stdexec/execution imo. the community really needs a readable implementation to quickly understand the mechanics of this library. fully qualifying names increases cognitive effort.

My normal style is to qualify everything. To me it makes it visible where a name is coming from. At times it avoids potential ambiguity when looking at the code. The compiler won’t be confused about where a name comes from but the human reader may be.

I can, however, see that the qualifaction can make things harder to read for others. Removing the qualification with a script would be trivial nearly (there may be some necessary qualifications, though), adding them back isn’t. The ideal compromise would be a view option to hide them but that assumes tooling.

I wouldn’t insist in qualification on PRs but I may add them when working on the code. If there is a broad consensus that there shall be no qualification, I’d follow that.

@ecoezen ecoezen requested a review from dietmarkuehl February 20, 2026 16:32
@ecoezen ecoezen force-pushed the sender-adaptor-closure branch from e8e5f91 to c047224 Compare February 20, 2026 16:51
…gration

- Refactor sender-adaptor closure infrastructure around sender_adaptor_closure, adaptor_closure, and composed closure piping.
- Migrate adaptor implementations (then, bulk, let, continues_on, associate, on, affine_on) to the closure-based path.
- Enforce [exec.adapt.obj] constraints and behavior: pipe equivalence, composition semantics, construction well-formedness, and noexcept propagation.
- Expand sender_adaptor_closure tests for uniqueness detection, composition associativity, composed call pattern, partial application well-formedness, and pipe parity.
- Deprecate direct detail::sender_adaptor alias in favor of make_sender_adaptor(...) factories and document the direct-use layout break (Adaptor moved into adaptor_closure_binding with [[no_unique_address]] for EBO, pending product_type support).
- Apply quality cleanup in sender_adaptor_closure module/header exports and comments.
- Add docs in headers.
Move the pipe operator overloads from beman::execution into
detail::pipeable so they are found only via ADL through the
closure_t base class. This prevents the operators from
participating in overload resolution for unrelated types in
beman::execution.

- Reintroduce beman::execution::detail::pipeable namespace.
- Add closure_t tag type as ADL anchor for sender_adaptor_closure
- Remove export using of operator| from execution.cppm
- Document detail::pipeable namespace in common.hpp
@ecoezen ecoezen force-pushed the sender-adaptor-closure branch from c047224 to a0c141a Compare February 20, 2026 16:58
Copy link
Member

@dietmarkuehl dietmarkuehl left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks!

@dietmarkuehl dietmarkuehl merged commit 66295d5 into bemanproject:main Feb 20, 2026
36 checks passed
@ecoezen ecoezen deleted the sender-adaptor-closure branch February 21, 2026 20:47
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.

composing sender adaptor closures doesn't work

3 participants