Skip to content

when_all() should return just() #482

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

Merged
merged 2 commits into from
Oct 15, 2022
Merged

when_all() should return just() #482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
abc57b7
Do not constrain when_all to at least one sender
bustercopley Mar 18, 2022
88e90a4
Tests for when_all with no senders
bustercopley Mar 21, 2022
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
9 changes: 5 additions & 4 deletions include/stdexec/execution.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4763,6 +4763,9 @@ namespace stdexec {
apply([](auto&&... __child_ops) noexcept -> void {
(execution::start(__child_ops), ...);
}, __self.__child_states_);
if constexpr (sizeof...(_SenderIds) == 0) {
__self.__complete();
}
}
}

Expand Down Expand Up @@ -4812,17 +4815,15 @@ namespace stdexec {
struct when_all_t {
template <sender... _Senders>
requires tag_invocable<when_all_t, _Senders...> &&
sender<tag_invoke_result_t<when_all_t, _Senders...>> &&
(sizeof...(_Senders) > 0)
sender<tag_invoke_result_t<when_all_t, _Senders...>>
auto operator()(_Senders&&... __sndrs) const
noexcept(nothrow_tag_invocable<when_all_t, _Senders...>)
-> tag_invoke_result_t<when_all_t, _Senders...> {
return tag_invoke(*this, (_Senders&&) __sndrs...);
}

template <sender... _Senders>
requires (!tag_invocable<when_all_t, _Senders...>) &&
(sizeof...(_Senders) > 0)
requires (!tag_invocable<when_all_t, _Senders...>)
auto operator()(_Senders&&... __sndrs) const
-> __impl::__sender<__x<decay_t<_Senders>>...> {
return __impl::__sender<__x<decay_t<_Senders>>...>{
Expand Down
16 changes: 16 additions & 0 deletions test/stdexec/algos/adaptors/test_transfer_when_all.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ TEST_CASE("transfer_when_all simple example", "[adaptors][transfer_when_all]") {
auto op = ex::connect(std::move(snd1), expect_value_receiver<double>{3.1415});
ex::start(op);
}
TEST_CASE("transfer_when_all with no senders", "[adaptors][transfer_when_all]") {
auto snd = ex::transfer_when_all(inline_scheduler{});
auto op = ex::connect(std::move(snd), expect_void_receiver{});
Copy link
Collaborator

Choose a reason for hiding this comment

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

I dunno, but I would expect when_all() to be equivalent to just(tuple{}) instead of just(). Can you comment on why you opted for the current semantics?

Copy link
Collaborator

Choose a reason for hiding this comment

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

when_all concats the values from all the senders, without wrapping them onto tuples. So an empty list of values seems correct?

Copy link
Contributor Author

@bustercopley bustercopley Oct 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Thank you for the review. In libunifex, just is special-cased for 0 arguments, and ends up calling set_value(). But here in stdexec things are more regular, and just completes by calling set_value(tuple{}), which is the desired end result for when_all, allowing a generic program like the following:

#include <iostream>
#include <tuple>

#include <stdexec/execution.hpp>

auto print = [](const auto &...args) {
  (std::cout << "[" << ... << args) << "]\n";
};

auto do_test(const auto &...args) {
  auto sender = stdexec::when_all(stdexec::just(args)...);
  auto results = stdexec::this_thread::sync_wait(sender);
  std::apply(print, *results);
}

int main() {
  do_test("a", "b", "c");
  do_test("x");
  do_test();
}

This program compiles on the current branch, but if when_all added an extra layer of 'tuple', this program would need its own special-case code to work around it.

Note that my current suggestion for when_all with no senders isn't implemented in terms of just -- by removing even more special cases, everything just takes care of itself. (Sort of! I did introduce a new if constexpr test for the case of 0 senders. But that's only an optimization.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

just completes by calling set_value(tuple{})

huh? When the just() sender completes, it calls set_value() (or rather set_value(rcvr) since set_value always takes a receiver).

Copy link
Collaborator

Choose a reason for hiding this comment

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

when_all concats the values from all the senders, without wrapping them onto tuples. So an empty list of values seems correct?

Oh yeah.
/me pours himself another coffee

Copy link
Contributor Author

Choose a reason for hiding this comment

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

huh? When the just() sender completes, it calls set_value() (or rather set_value(rcvr) since set_value always takes a receiver).

Right, sorry. I'll just say that adding tuple{} here would make the case of zero senders the odd-one-out.

ex::start(op);
}

TEST_CASE("transfer_when_all transfers the result when the scheduler dictates",
"[adaptors][transfer_when_all]") {
Expand All @@ -57,6 +62,17 @@ TEST_CASE("transfer_when_all transfers the result when the scheduler dictates",
sched.start_next();
CHECK(res == 3.1415);
}
TEST_CASE("transfer_when_all with no senders transfers the result", "[adaptors][transfer_when_all]") {
impulse_scheduler sched;
auto snd = ex::transfer_when_all(sched);
auto snd1 = std::move(snd) | ex::then([]() { return true; });
bool res{false};
auto op = ex::connect(std::move(snd1), expect_value_receiver_ex<bool>{&res});
ex::start(op);
CHECK(!res);
sched.start_next();
CHECK(res);
}

TEST_CASE("transfer_when_all_with_variant returns a sender", "[adaptors][transfer_when_all]") {
auto snd = ex::transfer_when_all_with_variant(inline_scheduler{}, ex::just(3), ex::just(0.1415));
Expand Down
12 changes: 8 additions & 4 deletions test/stdexec/algos/adaptors/test_when_all.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,9 @@ TEST_CASE("when_all with just one sender", "[adaptors][when_all]") {
wait_for_value(std::move(snd), 2);
}

TEST_CASE("when_all with no senders sender -- should fail", "[adaptors][when_all]") {
// Should not compile:
// auto snd = ex::when_all();
static_assert(!std::invocable<ex::when_all_t>);
TEST_CASE("when_all with no senders", "[adaptors][when_all]") {
ex::sender auto snd = ex::when_all();
wait_for_value(std::move(snd));
}

TEST_CASE("when_all when one sender sends void", "[adaptors][when_all]") {
Expand Down Expand Up @@ -100,6 +99,11 @@ TEST_CASE("when_all_with_variant with same type", "[adaptors][when_all]") {
std::move(snd), std::variant<std::tuple<int>>{2}, std::variant<std::tuple<int>>{3});
}

TEST_CASE("when_all_with_variant with no senders", "[adaptors][when_all]") {
ex::sender auto snd = ex::when_all_with_variant();
wait_for_value(std::move(snd));
}

TEST_CASE("when_all completes when children complete", "[adaptors][when_all]") {
impulse_scheduler sched;
bool called{false};
Expand Down