Skip to content

Fixing omissions - #308

Open
dietmarkuehl wants to merge 4 commits into
mainfrom
fixing-omissions
Open

Fixing omissions#308
dietmarkuehl wants to merge 4 commits into
mainfrom
fixing-omissions

Conversation

@dietmarkuehl

Copy link
Copy Markdown
Member

No description provided.

- get_domain should return a default constructed domain
- stop_when needs to unregister the stop_callbacks
Copilot AI lite review requested due to automatic review settings August 14, 2026 23:12
@dietmarkuehl
dietmarkuehl requested a review from camio as a code owner August 14, 2026 23:12
struct state {
using operation_state_concept = ::beman::execution::operation_state_tag;
struct base_state {
using rcvr_t = ::std::remove_cvref_t<Rcvr>;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[pre-commit] reported by reviewdog 🐶

Suggested change
using rcvr_t = ::std::remove_cvref_t<Rcvr>;
using rcvr_t = ::std::remove_cvref_t<Rcvr>;

struct env {
base_state* st;
base_state<Rcvr>* st;
auto query(const ::beman::execution::get_stop_token_t&) const noexcept {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[pre-commit] reported by reviewdog 🐶

Suggested change
auto query(const ::beman::execution::get_stop_token_t&) const noexcept {
auto query(const ::beman::execution::get_stop_token_t&) const noexcept {

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR appears to address “omissions” around query/CPO noexcept handling and domain/environment behavior in the beman execution library, while updating tests and documentation to match the new constraints/semantics.

Changes:

  • Refactors several CPOs (e.g., start, set_*, get_*) to enforce nothrow requirements via static_assert.
  • Adjusts multiple tests to work around (or defer) compile-time negative cases, and modifies get_domain-related expectations.
  • Updates stop_when state management and moves one test (exec-spawn-future.test) into the supported test list; minor doc updates.

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 20 comments.

Show a summary per file
File Description
tests/beman/execution/exec-snd-expos.test.cpp Adds a custom query tag and uses it in query_with_default test.
tests/beman/execution/exec-set-stopped.test.cpp Comments out a compile-time nothrow/ill-formedness test.
tests/beman/execution/exec-set-error.test.cpp Comments out compile-time negative requires checks for nothrow behavior.
tests/beman/execution/exec-recv.test.cpp Comments out a negative completion validation check.
tests/beman/execution/exec-read-env.test.cpp Adds a custom domain query tag; changes get_domain expectations and completion signature assertions.
tests/beman/execution/exec-opstate.test.cpp Comments out an operation_state negative test for throwing start().
tests/beman/execution/exec-get-stop-token.test.cpp Comments out a fallback behavior test for non-noexcept queries.
tests/beman/execution/exec-get-domain.test.cpp Comments out a case around non-noexcept get_domain query; changes expected returned value.
tests/beman/execution/exec-get-delegation-scheduler.test.cpp Comments out a negative availability test.
tests/beman/execution/exec-fwd-env.test.cpp Comments out a fallback behavior test for non-noexcept forwarding query customization.
tests/beman/execution/CMakeLists.txt Moves exec-spawn-future.test from unsupported to supported tests.
include/beman/execution/detail/stop_when.hpp Adds polymorphic base state, callback reset, and introduces stdout logging in stop callback.
include/beman/execution/detail/start.hpp Replaces deleted overloads for noexcept with static_assert checks in the body.
include/beman/execution/detail/set_value.hpp Adds an additional static_assert nothrow check in the body.
include/beman/execution/detail/set_stopped.hpp Replaces deleted overload-based noexcept enforcement with a body static_assert.
include/beman/execution/detail/set_error.hpp Replaces deleted overload-based noexcept enforcement with a body static_assert.
include/beman/execution/detail/get_stop_token.hpp Drops noexcept from query detection and adds body static_assert.
include/beman/execution/detail/get_start_scheduler.hpp Drops noexcept from query detection and adds body static_assert.
include/beman/execution/detail/get_scheduler.hpp Adds a static_assert(noexcept(...)) guard before returning scheduler.
include/beman/execution/detail/get_forward_progress_guarantee.hpp Adds static_assert checks for noexcept and return type.
include/beman/execution/detail/get_env.hpp Switches to calling get_env() on std::as_const(obj) and updates associated static_asserts.
include/beman/execution/detail/get_domain.hpp Reworks logic to default-construct the return type and duplicates an else-if branch.
include/beman/execution/detail/get_delegation_scheduler.hpp Drops noexcept from query detection and adds body static_assert.
include/beman/execution/detail/get_completion_scheduler.hpp Adds a static_assert(noexcept(...)) guard before recursion.
include/beman/execution/detail/get_await_completion_adaptor.hpp Drops noexcept from query detection and adds body static_assert.
include/beman/execution/detail/forwarding_query.hpp Drops noexcept from query detection and adds body static_assert (despite documented semantics).
docs/overview.md Formatting/spacing tweaks around section boundaries.
docs/implementation-status.md Adds SPDX header comment and adjusts status indicators.
Suppressed comments (2)

tests/beman/execution/exec-get-domain.test.cpp:111

  • This assertion now expects get_domain(has_get_domain<true, domain>{42}) to return a default-initialized domain (value 0), which contradicts the env.query(get_domain) implementation in the test type and masks incorrect get_domain behavior.

    static_assert(0 == test_std::get_domain(has_get_domain<true, domain>{42}).value);

tests/beman/execution/exec-read-env.test.cpp:85

  • The expected completion signatures for read_env(get_domain) have the set_error(std::exception_ptr) case commented out. This removes coverage for error propagation in read_env's completion signatures.
    static_assert(
        std::same_as<test_std::completion_signatures<test_std::set_value_t(domain)
                                                     //-dk:TODO verify , test_std::set_error_t(std::exception_ptr)
                                                     >,
                     decltype(test_std::get_completion_signatures<decltype(sender), env>())>);


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 37 to 41
if constexpr (requires { ::std::as_const(env).query(*this); }) {
return ::std::as_const(env).query(*this);
using type = decltype(auto(::std::as_const(env).query(*this)));
static_assert(noexcept(type{}));
return type{};
} else if constexpr (requires {
Comment on lines 25 to 32
template <typename Object>
requires requires(Object&& object, const forwarding_query_t& query) {
{ ::std::forward<Object>(object).query(query) } noexcept -> ::std::same_as<bool>;
{ ::std::forward<Object>(object).query(query) } -> ::std::same_as<bool>;
}
constexpr auto operator()(Object&& object) const noexcept -> bool {
static_assert(noexcept(::std::forward<Object>(object).query(*this)));
return ::std::forward<Object>(object).query(*this);
}
Comment on lines 31 to 39
struct get_stop_token_t {
template <typename Object>
requires requires(Object&& object, const get_stop_token_t& tag) {
{ ::std::as_const(object).query(tag) } noexcept -> ::beman::execution::detail::decayed_stoppable_token;
{ ::std::as_const(object).query(tag) } -> ::beman::execution::detail::decayed_stoppable_token;
}
auto operator()(Object&& object) const noexcept {
static_assert(noexcept(::std::as_const(object).query(*this)));
return ::std::as_const(object).query(*this);
}
Comment on lines 25 to 32
template <typename Env>
requires requires(const get_start_scheduler_t& self, const Env& env) {
{ auto(::std::as_const(env).query(self)) } noexcept -> beman::execution::scheduler;
{ auto(::std::as_const(env).query(self)) } -> beman::execution::scheduler;
}
auto operator()(const Env& env) const noexcept {
return env.query(*this);
static_assert(noexcept(::std::as_const(env).query(*this)));
return ::std::as_const(env).query(*this);
}
Comment on lines 24 to 32
struct get_delegation_scheduler_t {
template <typename Env>
requires requires(Env&& env, const get_delegation_scheduler_t& g) {
{ auto(::std::as_const(env).query(g)) } noexcept -> ::beman::execution::scheduler;
{ auto(::std::as_const(env).query(g)) } -> ::beman::execution::scheduler;
}
auto operator()(Env&& env) const noexcept {
static_assert(noexcept(::std::as_const(env).query(*this)));
return ::std::as_const(env).query(*this);
}
Comment on lines +59 to +60
//-dk:TODO test this fails to compile:
// test_get_stop_token<test_std::never_stop_token>(has_get_stop_token<false>());

test_get_delegation_scheduler<false>(test_std::env<>{});
test_get_delegation_scheduler<false>(env<false, scheduler>{});
//-dk:TODO verify that this fails to compile test_get_delegation_scheduler<false>(env<false, scheduler>{});
static_assert(test_std::forwarding_query(static_query<>()));
static_assert(noexcept(test_std::forwarding_query(static_query<>())));
static_assert(not test_std::forwarding_query(static_query<false>()));
//-dk:TODO verify this fails to compile: static_assert(not test_std::forwarding_query(static_query<false>()));
test_get_domain<test_std::default_domain>(non_const_get_domain<false>{}); // falling back to `default_domain`
test_get_domain<domain>(has_get_domain<true, domain>{42});
test_get_domain<domain>(has_get_domain<false, domain>{42});
//-dk:TODO verify that this fails to compile test_get_domain<domain>(has_get_domain<false, domain>{42});
Comment on lines +72 to +73
ASSERT(domain{} == test_std::get_domain(env{17}));
ASSERT(domain{} == test_std::get_domain(test_std::get_env(receiver{17, 0})));
@coveralls

coveralls commented Aug 14, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 93.454% (-0.2%) from 93.627% — fixing-omissions into main

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.

3 participants