diff --git a/libs/core/execution/include/hpx/execution/algorithms/detail/inject_scheduler.hpp b/libs/core/execution/include/hpx/execution/algorithms/detail/inject_scheduler.hpp index 71be3690ff7b..d4c062f40c24 100644 --- a/libs/core/execution/include/hpx/execution/algorithms/detail/inject_scheduler.hpp +++ b/libs/core/execution/include/hpx/execution/algorithms/detail/inject_scheduler.hpp @@ -8,6 +8,7 @@ #pragma once #include +#include #include #include #include @@ -58,4 +59,45 @@ namespace hpx::execution::experimental::detail { return HPX_MOVE(p).invoke(HPX_MOVE(p.scheduler), HPX_FORWARD(U, u)); } }; + + // Sibling of `inject_scheduler` that holds a reference to a `run_loop` + // instead of a scheduler. Used so that `make_future(loop)` yields a + // pipe-able adapter which, when piped with a sender, dispatches + // `tag_invoke(Tag, run_loop&, sender, ...)`. + HPX_CXX_CORE_EXPORT template + struct inject_run_loop + : partial_algorithm_base, + Ts...> + { + private: + // Stored as a pointer because the partial may be moved and references + // can't be re-bound; the `run_loop&` lifetime is the caller's + // responsibility (the same as the `scheduler` case). + hpx::execution::experimental::run_loop* loop_; + + using base_type = partial_algorithm_base, Ts...>; + + public: + template + explicit constexpr inject_run_loop( + hpx::execution::experimental::run_loop& loop, Ts_&&... ts) + : base_type(HPX_FORWARD(Ts_, ts)...) + , loop_(&loop) + { + } + + // clang-format off + template + )> + // clang-format on + friend constexpr HPX_FORCEINLINE auto operator|( + U&& u, inject_run_loop p) + { + // NOLINTNEXTLINE(bugprone-use-after-move) + return HPX_MOVE(p).invoke(*p.loop_, HPX_FORWARD(U, u)); + } + }; } // namespace hpx::execution::experimental::detail diff --git a/libs/core/execution/include/hpx/execution/algorithms/make_future.hpp b/libs/core/execution/include/hpx/execution/algorithms/make_future.hpp index da41952550de..0b99939dae51 100644 --- a/libs/core/execution/include/hpx/execution/algorithms/make_future.hpp +++ b/libs/core/execution/include/hpx/execution/algorithms/make_future.hpp @@ -29,36 +29,6 @@ namespace hpx::execution::experimental { // enforce proper formatting namespace detail { - // Recover the parent `run_loop&` from a `run_loop::scheduler`. - // - // P2300 deliberately does not provide a public API to obtain the - // owning `run_loop&` from one of its schedulers. The only way to do - // this against the current stdexec implementation is to read the - // private `__loop_` member exposed by the scheduler's environment. - // - // This helper is the single point in HPX that touches that internal - // detail. If upstream stdexec ever renames/removes `__loop_` (as - // happened with `stdexec::tags`), only this function needs to change. - // Post-cleanup follow-up of #7123. - // - // The parameter is constrained to the concrete `run_loop::scheduler` - // type (rather than a generic template) because the implementation - // depends on `.__loop_` being the specific stdexec env layout. - // `run_loop::scheduler::schedule()` is `noexcept`, so the - // unconditional `noexcept` here is sound. The function is not marked - // `constexpr` because the `stdexec::schedule` CPO wrapper is not - // declared `constexpr` (GCC strict mode rejects calling it from a - // constexpr context, even though the underlying member is constexpr). - inline hpx::execution::experimental::run_loop& - get_run_loop_from_scheduler( - decltype(std::declval() - .get_scheduler()) const& sched) noexcept - { - return static_cast( - *hpx::execution::experimental::get_env(schedule(sched)) - .__loop_); - } - template void start_operation_state(OperationState& op_state) noexcept { @@ -200,11 +170,9 @@ namespace hpx::execution::experimental { template future_data_with_run_loop(init_no_addref no_addref, other_allocator const& alloc, - decltype(std::declval() - .get_scheduler()) const& sched, - Sender&& sender) + hpx::execution::experimental::run_loop& loop_, Sender&& sender) : base_type(no_addref, alloc, HPX_FORWARD(Sender, sender)) - , loop(get_run_loop_from_scheduler(sched)) + , loop(loop_) { this->set_on_completed([this]() { loop.finish(); }); } @@ -222,10 +190,53 @@ namespace hpx::execution::experimental { } }; + // Detects whether a sender's `set_value` completion scheduler is + // `run_loop::scheduler`. Used by `detail::make_future` to reject -- + // at compile time -- calls of the form `make_future(transfer_just( + // run_loop_sched, ...))` etc., which previously relied on HPX + // recovering the parent `run_loop&` from the scheduler via stdexec + // internals; that path was removed in this refactor and the new + // `make_future(loop, sender)` form must be used instead. + template + struct sender_completion_is_run_loop_scheduler : std::false_type + { + }; + + template + struct sender_completion_is_run_loop_scheduler(hpx::execution:: + experimental::get_env(std::declval())))>> + : std::is_same( + hpx::execution::experimental::get_env( + std::declval())))>, + hpx::execution::experimental::run_loop::scheduler> + { + }; + /////////////////////////////////////////////////////////////////////// HPX_CXX_CORE_EXPORT template auto make_future(Sender&& sender, Allocator const& allocator) { + // The previous implementation recovered the parent `run_loop&` + // from the sender's completion scheduler (a `run_loop::scheduler`) + // via stdexec internals so it could call `loop.run()` from the + // future's `get()`. That path is gone; without an explicit + // `run_loop&`, this `detail::make_future` would silently produce + // a future that hangs because nothing drives the loop. Reject + // such calls at compile time and direct the user to + // `make_future(loop, sender)`. + static_assert(!sender_completion_is_run_loop_scheduler< + std::decay_t>::value, + "make_future(sender) cannot drive the parent run_loop when " + "the sender's completion scheduler is a run_loop::scheduler. " + "Use make_future(loop, sender) -- pass the run_loop reference " + "explicitly so the resulting future can drive the loop in " + "its get()."); + using allocator_type = Allocator; using value_types = hpx::execution::experimental::value_types_of_t< @@ -264,9 +275,8 @@ namespace hpx::execution::experimental { /////////////////////////////////////////////////////////////////////// HPX_CXX_CORE_EXPORT template auto make_future_with_run_loop( - decltype(std::declval() - .get_scheduler()) const& sched, - Sender&& sender, Allocator const& allocator) + hpx::execution::experimental::run_loop& loop, Sender&& sender, + Allocator const& allocator) { using allocator_type = Allocator; @@ -294,7 +304,7 @@ namespace hpx::execution::experimental { hpx::util::allocator_deleter{alloc}); allocator_traits::construct(alloc, p.get(), init_no_addref{}, alloc, - sched, HPX_FORWARD(Sender, sender)); + loop, HPX_FORWARD(Sender, sender)); return hpx::traits::future_access>::create( p.release(), false); @@ -383,12 +393,11 @@ namespace hpx::execution::experimental { )> // clang-format on friend auto tag_invoke(make_future_t, - decltype(std::declval() - .get_scheduler()) const& sched, - Sender&& sender, Allocator const& allocator = Allocator{}) + hpx::execution::experimental::run_loop& loop, Sender&& sender, + Allocator const& allocator = Allocator{}) { return detail::make_future_with_run_loop( - sched, HPX_FORWARD(Sender, sender), allocator); + loop, HPX_FORWARD(Sender, sender), allocator); } // clang-format off @@ -421,6 +430,49 @@ namespace hpx::execution::experimental { HPX_FORWARD(Scheduler, scheduler), allocator}; } + // `make_future(loop.get_scheduler())` is intentionally rejected at + // the call site: the run-loop-backed binary + // `tag_invoke(make_future_t, run_loop::scheduler, sender, alloc)` + // overload is gone (replaced by the `run_loop&` overload above). + // Without these deletes, the generic scheduler partial above would + // accept the call, return an `inject_scheduler`, and fail with a + // confusing substitution error only when the partial is later piped + // with a sender. Use `make_future(loop, sender)` / + // `sender | make_future(loop)` instead. + // + // Two deleted overloads (one per arity), and neither uses + // `HPX_CONCEPT_REQUIRES_`: that macro expands to a defaulted + // template parameter, and GCC rejects defaulted template arguments + // on friend *declarations* (which a `= delete`d friend is). The + // 2-argument overload therefore takes any second arg (allocator or + // not); that's intentional -- `make_future(run_loop::scheduler, X)` + // shouldn't compile regardless of `X`. + friend constexpr auto tag_fallback_invoke(make_future_t, + hpx::execution::experimental::run_loop::scheduler const&) = delete; + + template + friend constexpr auto tag_fallback_invoke(make_future_t, + hpx::execution::experimental::run_loop::scheduler const&, + T const&) = delete; + + // Partial-algorithm overload for the `run_loop&` form so that + // `make_future(loop)` can be piped with a sender. Returns an + // `inject_run_loop` adapter which, when piped with a sender, calls + // `tag_invoke(make_future_t, loop, sender, allocator)` above. + // clang-format off + template , + HPX_CONCEPT_REQUIRES_( + hpx::traits::is_allocator_v + )> + // clang-format on + friend constexpr HPX_FORCEINLINE auto tag_fallback_invoke(make_future_t, + hpx::execution::experimental::run_loop& loop, + Allocator const& allocator = Allocator{}) + { + return hpx::execution::experimental::detail::inject_run_loop< + make_future_t, Allocator>{loop, allocator}; + } + // clang-format off template , HPX_CONCEPT_REQUIRES_( diff --git a/libs/core/execution/tests/unit/algorithm_run_loop.cpp b/libs/core/execution/tests/unit/algorithm_run_loop.cpp index bd76b873d496..8a178a18e294 100644 --- a/libs/core/execution/tests/unit/algorithm_run_loop.cpp +++ b/libs/core/execution/tests/unit/algorithm_run_loop.cpp @@ -611,7 +611,7 @@ void test_future_sender() [[maybe_unused]] auto sched = loop.get_scheduler(); auto s = ex::transfer_just(sched, 3); - auto f = ex::make_future(std::move(s)); + auto f = ex::make_future(loop, std::move(s)); HPX_TEST_EQ(f.get(), 3); } @@ -620,7 +620,7 @@ void test_future_sender() ex::run_loop loop; [[maybe_unused]] auto sched = loop.get_scheduler(); - auto f = ex::transfer_just(sched, 3) | ex::make_future(); + auto f = ex::transfer_just(sched, 3) | ex::make_future(loop); HPX_TEST_EQ(f.get(), 3); } @@ -629,7 +629,12 @@ void test_future_sender() ex::run_loop loop; [[maybe_unused]] auto sched = loop.get_scheduler(); - auto f = ex::just(3) | ex::make_future(sched); + // Use transfer_just(sched, ...) so the partial-pipe path actually + // schedules work onto `loop` and depends on `loop.run()` being + // driven by the resulting future's `get()` (a bug in the + // `inject_run_loop` partial would hang here, not silently pass + // as it would with `just(3)`). + auto f = ex::transfer_just(sched, 3) | ex::make_future(loop); HPX_TEST_EQ(f.get(), 3); } @@ -640,7 +645,7 @@ void test_future_sender() std::atomic called{false}; auto s = ex::schedule(sched) | ex::then([&] { called = true; }); - auto f = ex::make_future(std::move(s)); + auto f = ex::make_future(loop, std::move(s)); f.get(); HPX_TEST(called); } @@ -653,7 +658,7 @@ void test_future_sender() auto s1 = ex::transfer_just(sched, std::size_t(42)); auto s2 = ex::transfer_just(sched, 3.14); auto s3 = ex::transfer_just(sched, std::string("hello")); - auto f = ex::make_future(sched, + auto f = ex::make_future(loop, ex::then(ex::when_all(std::move(s1), std::move(s2), std::move(s3)), [](std::size_t x, double, std::string z) { return z.size() + x; @@ -667,7 +672,7 @@ void test_future_sender() ex::run_loop loop; [[maybe_unused]] auto sched = loop.get_scheduler(); auto result = tt::sync_wait( - ex::as_sender(ex::make_future(ex::transfer_just(sched, 42)))); + ex::as_sender(ex::make_future(loop, ex::transfer_just(sched, 42)))); HPX_TEST_EQ(hpx::get<0>(*result), 42); } @@ -681,9 +686,9 @@ void test_future_sender() return 42; }); - HPX_TEST_EQ( - ex::make_future(ex::transfer(ex::as_sender(std::move(f)), sched)) - .get(), + HPX_TEST_EQ(ex::make_future( + loop, ex::transfer(ex::as_sender(std::move(f)), sched)) + .get(), 42); } @@ -695,7 +700,7 @@ void test_future_sender() auto s1 = ex::transfer_just(sched, std::size_t(42)); auto s2 = ex::transfer_just(sched, 3.14); auto s3 = ex::transfer_just(sched, std::string("hello")); - auto f = ex::make_future(sched, + auto f = ex::make_future(loop, ex::then(ex::when_all(std::move(s1), std::move(s2), std::move(s3)), [](std::size_t x, double, std::string z) { return z.size() + x; @@ -705,13 +710,22 @@ void test_future_sender() auto t2 = sf.then([](auto&& sf) { return sf.get() + 2; }); auto t1s = ex::then( ex::as_sender(std::move(t1)), [](std::size_t x) { return x + 1; }); - auto t1f = ex::make_future(sched, std::move(t1s)); + auto t1f = ex::make_future(loop, std::move(t1s)); auto last = hpx::dataflow( hpx::unwrapping([](std::size_t x, std::size_t y) { return x + y; }), t1f, t2); HPX_TEST_EQ(last.get(), std::size_t(18)); } + + // Note: `make_future(sender)` / `sender | make_future()` for senders + // whose completion scheduler is a `run_loop::scheduler` is rejected at + // compile time by a `static_assert` in `detail::make_future`. Those + // forms previously relied on HPX recovering the parent `run_loop&` from + // the scheduler via stdexec internals; with that path removed, the only + // way to drive the loop from the future is to pass `run_loop&` + // explicitly via `make_future(loop, sender)` (covered by tests 1-8 and + // the run-loop-driven cases above). } void test_ensure_started()