Skip to content

exec(fork_join): coalesce empty and unary calls #2302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
romintomasetti wants to merge 3 commits into
base: main
Choose a base branch
from
+59 −2
Open

exec(fork_join): coalesce empty and unary calls #2302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d307bf1
stdexec(then): noexcept-ness
romintomasetti Oct 6, 2026
e43b788
exec(fork_join): coalesce empty and unary calls
romintomasetti Oct 5, 2026
8881582
Merge branch 'main' into fork-join-p4269
ericniebler Oct 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion include/exec/fork_join.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,25 @@ namespace experimental::execution

struct fork_join_t
{
template <STDEXEC::sender Sndr, class... Closures>
/// No closure given.
template <STDEXEC::sender Sndr>
STDEXEC_ATTRIBUTE(host, device)
constexpr auto operator()(Sndr&& sndr) const noexcept(STDEXEC::__nothrow_decay_copyable<Sndr>)
{
return static_cast<Sndr&&>(sndr);
}

/// Unary closure.
template <STDEXEC::sender Sndr, class Closure> requires (!STDEXEC::sender<Closure>)
STDEXEC_ATTRIBUTE(host, device)
constexpr auto operator()(Sndr&& sndr, Closure&& clsr) const
Comment thread
romintomasetti marked this conversation as resolved.
noexcept(STDEXEC::__nothrow_callable<Closure, Sndr>)
{
return static_cast<Closure&&>(clsr)(static_cast<Sndr&&>(sndr));
}

/// One sender and multiple closures.
template <STDEXEC::sender Sndr, class... Closures> requires (sizeof...(Closures) > 1)
STDEXEC_ATTRIBUTE(host, device)
constexpr auto operator()(Sndr&& sndr, Closures&&... closures) const //
-> STDEXEC::__well_formed_sender auto
Expand All @@ -292,6 +310,7 @@ namespace experimental::execution
static_cast<Sndr&&>(sndr));
}

/// One or more closures.
template <class... Closures>
requires((!STDEXEC::sender<Closures>) && ...)
STDEXEC_ATTRIBUTE(host, device)
Expand Down
5 changes: 4 additions & 1 deletion include/stdexec/__detail/__then.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,9 @@ namespace STDEXEC
//! the program is ill-formed at the point where the resulting sender
//! is connected to a receiver.
template <sender _Sender, __movable_value _Fun>
constexpr auto operator()(_Sender&& __sndr, _Fun __fun) const -> __well_formed_sender auto
constexpr auto operator()(_Sender&& __sndr, _Fun __fun) const
noexcept(__nothrow_decay_copyable<_Fun, _Sender>)
Comment thread
romintomasetti marked this conversation as resolved.
Comment thread
ericniebler marked this conversation as resolved.
-> __well_formed_sender auto
{
return __make_sexpr<then_t>(static_cast<_Fun&&>(__fun), static_cast<_Sender&&>(__sndr));
}
Expand All @@ -235,6 +237,7 @@ namespace STDEXEC
template <__movable_value _Fun>
STDEXEC_ATTRIBUTE(always_inline)
constexpr auto operator()(_Fun __fun) const
noexcept(__nothrow_decay_copyable<_Fun>)
{
return __closure(*this, static_cast<_Fun&&>(__fun));
}
Expand Down
24 changes: 24 additions & 0 deletions test/exec/test_fork_join.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,30 @@ namespace
completion_signatures<set_value_t(), set_error_t(std::exception_ptr)>>);
}

TEST_CASE("fork_join coalesces empty and unary calls", "[adaptors][fork_join]")
{
/// Empty (no closure given).
STDEXEC::sender auto empty = exec::fork_join(STDEXEC::just());
using empty_t = decltype(empty);
STATIC_REQUIRE(std::same_as<empty_t, decltype(STDEXEC::just())>);
STATIC_REQUIRE(!exec::sender_for<empty_t, exec::fork_join_t>);
STATIC_REQUIRE(noexcept(exec::fork_join(STDEXEC::just())));

auto then = STDEXEC::then([]() noexcept {});

/// Unary closure.
STDEXEC::sender auto unary = exec::fork_join(STDEXEC::just(), then);
using unary_t = decltype(unary);
STATIC_REQUIRE(std::same_as<unary_t, decltype(STDEXEC::just() | then)>);
STATIC_REQUIRE(!exec::sender_for<unary_t, exec::fork_join_t>);
STATIC_REQUIRE(noexcept(exec::fork_join(STDEXEC::just(), then)));

/// Multiple closures.
STDEXEC::sender auto multiple = STDEXEC::just() | exec::fork_join(then, then);
STATIC_REQUIRE(exec::sender_for<decltype(multiple), exec::fork_join_t>);
STATIC_REQUIRE(!noexcept(exec::fork_join(STDEXEC::just(), then, then)));

@maartenarnst maartenarnst Oct 6, 2026 •
edited
Loading

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.

We could also omit this line. Because it seems the reason is that the function has no noexcept clause, rather than that one operations may throw.

@romintomasetti romintomasetti Oct 6, 2026

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.

Yeah, but at least it's tested.

@maartenarnst maartenarnst Oct 6, 2026

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.

OK.

@RobertLeahy RobertLeahy Oct 6, 2026

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.

I would expect constructing such an exec::fork_join sender not to throw, which noexcept is missing that prevents that?

@romintomasetti romintomasetti Oct 6, 2026 •
edited
Loading

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.

There are 2 functions in the fork_join_t struct I haven't touched yet.

@maartenarnst maartenarnst Oct 6, 2026

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.

My two cents would be to add the noexcept clause and flip the static_assert here.

We could write something like (perhaps there's a way to make it less verbose with other concepts from __concepts.hpp):

noexcept(STDEXEC::__nothrow_decay_copyable<Closures..., Sndr>
         && (std::is_nothrow_move_constructible_v<STDEXEC::__decay_t<Closures>> && ...)
         && std::is_nothrow_move_constructible_v<STDEXEC::__decay_t<Sndr>>)

The last two checks would be needed for the moves here:

stdexec/include/stdexec/__detail/__basic_sender.hpp

Lines 460 to 470 in f4c123f

template <class _Tag>
struct __make_sexpr_t
{
template <class _Data = __, class... _Child>
constexpr auto operator()(_Data __data = {}, _Child... __child) const
{
return __sexpr_t<_Tag, _Data, _Child...>{
{_Tag(), static_cast<_Data&&>(__data), static_cast<_Child&&>(__child)...}
};
}
};

@romintomasetti romintomasetti Oct 7, 2026

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.

@ericniebler What do you think?

@ericniebler ericniebler Oct 7, 2026

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.

we should probably fix __make_sexpr_t to perfectly forward the arguments and give it a noexcept clause. i can make a PR for that.

@ericniebler ericniebler Oct 7, 2026

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 change has now landed and i have merged it into this PR, so the noexcept clause here can be simply noexcept(STDEXEC::__nothrow_decay_copyable<Closures..., Sndr>).

@maartenarnst maartenarnst Oct 7, 2026

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.

Ok! Thank you very much for the pr and merging it here. To write the noexcept clause, one remaining question seems to be whether we should take into account the move of the __tuple that fork_join constructs and then passes to __make_sexpr?

}

struct ForwardingThen
{
template <typename Value>
Expand Down
11 changes: 11 additions & 0 deletions test/stdexec/algos/adaptors/test_then.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,17 @@ namespace
| ex::then([] { return std::string{"hello"}; }));
}

TEST_CASE("then noexceptness", "[adaptors][then]")
{
auto func = [](){};
Comment thread
ericniebler marked this conversation as resolved.

STATIC_REQUIRE(noexcept(ex::then(func)));

STATIC_REQUIRE(noexcept(STDEXEC::just() | STDEXEC::then(func)));

STATIC_REQUIRE(noexcept(STDEXEC::then(STDEXEC::just(), func)));
}

TEST_CASE("then keeps error_types from input sender", "[adaptors][then]")
{
inline_scheduler sched1{};
Expand Down
Loading