NVIDIA / stdexec

`std::execution`, the proposed C++ framework for asynchronous and parallel programming.
Apache License 2.0
1.56k stars 159 forks source link

Potential redesign of typed sender traits #226

Closed ericniebler closed 2 years ago

ericniebler commented 2 years ago

On the LEWG reflector, Peter Dimov had some interesting suggestions about value_types and error_types. Instead of template template types aliases, they could simply be type aliases:

struct my_typed_sender {
  using value_types = overloads<args<Ts...>, args<Us...>>;
  using error_types = overloads<E>;
//....
};

Then we could provide rebind_value_types_t and rebind_error_types_t template aliases that fill the present role of the existing value_types and error_types template template type aliases.

I really like this suggestion, and see only one downside: it would force the eager computation of a typed sender's value/error types.

I'll note that Peter's suggestion permits short forms that omit "overloads" when there is only one signature.

We could even employ function types here, which would even give us a way to specify that a particular overload of set_value is noexcept:

struct my_typed_sender {
  using value_types = signatures<void(Ts...) noexcept, void(Us...)>;
  using error_types = signatures<void(E)>; // Implicitly noexcept like a destructor
//....
};
kirkshoop commented 2 years ago

My preference is to have something uniform to replace the existing traits. I do not like having three different ways to describe the same thing.

This godbolt shows the approach I would prefer.

https://godbolt.org/z/Ee796Mq7q

Some of the advantages:

I can include any cpo I can select out the queries by const this_& and forward only them. I have a uniform interface so that customizing, building and consuming will share a uniform set of tools that work the same for all of the cpos. I have one name that represents all of this. I can define that name as a private friend. I do not need to use any ::template stuff or hide that behind name specific tools.

Etc..

ericniebler commented 2 years ago

My preference is to have something uniform to replace the existing traits. I do not like having three different ways to describe the same thing. [...]

I have thought about this suggestion and I have some concerns. The biggest of which is that it seems to increase the amount of metaprogramming that sender authors must do, whereas I'm trying to reduce it. As for your "three different ways to do the same thing", you don't disagree that having three separate receiver channels for value, error, and done is the right design. Why should it be the wrong design for the sender's metadata?

Many senders adaptors fiddle with only one or two of the channels. Putting all the metadata into a single alias would require users to write a metaprogram to filter and transform out only the bits they want to. There is no longer a simple way for a sender adaptor to delegate some of its metadata to its inner sender. By keeping the channels separate, a then sender, for example, could simply declare its error_types and sends_done as:

using error_types = typename sender_traits<Pred>::error_types;
static constexpr bool sends_done = sender_traits<Pred>::sends_done;

It would also simplify the computation of the then sender's value_types since it wouldn't have to worry about filtering out signals its not interested in.

The sender metadata is easily the ugliest, most complex part of P2300. My primary goal is to make it approachable. I don't think putting all the metadata into one flat list is actually a simplification.

miscco commented 2 years ago

From my point of view, both the current state and the proposal are way out of scope even for advanced developers. The complexity that is required to do simple things is quite staggeFrom my point of view, both the current state and the proposal are way out of scope even for advanced developers. The complexity that is required to do simple things is quite staggering. It should be easy to do the simple thing and possible to do the complex one. From that I believe the baseline should be

struct mySender {
    using value_types = int;
    using error_types = E;
    static inline constexpr bool sets_done = true;

    friend constexpr auto tag_invoke(execution::set_value_t, int) { ... }
}

As @ericniebler suggested this would simply propagate via

using error_types = typename sender_traits<Pred>::error_types;
static constexpr bool sends_done = sender_traits<Pred>::sends_done;

This is a clean and simple design for simple use cases. So what about a set_value that takes multiple arguments

struct mySender {
    using value_types = args<int, double>
    using error_types = E;
    static inline constexpr bool sets_done = true;

    friend constexpr auto tag_invoke(execution::set_value_t, int, double) { ... }
}

Similarly, a multichannel set_value could look like

struct mySender {
    using value_types = overloads<args<int, double>, args<string>>
    using error_types = overloads<E1, E2>;
    static inline constexpr bool sets_done = true;

    friend constexpr auto tag_invoke(execution::set_value_t, int, double) { ... }
    friend constexpr auto tag_invoke(execution::set_value_t, string) { ... }
}

Now the obvious issue is how to discriminate the three different cases, set_value(args<int, double>) should probably be valid after all. My proposal would be to add some traits like enable_borrowed_range (I would be super happy if someone would come up with better names)

template<class T>
static inline constexpr multi_argument_set_value = false;

template<class T>
static inline constexpr overloaded_set_value = false;

With that one could write

struct mySimpleSender {
    using value_types = int;
    using error_types = E;
    static inline constexpr bool sets_done = true;

    friend constexpr auto tag_invoke(execution::set_value_t, int) { ... }
}

struct myMultiArgSender {
    using value_types = args<int, double>;
    using error_types = E;
    static inline constexpr bool sets_done = true;

    friend constexpr auto tag_invoke(execution::set_value_t, int, double) { ... }
}

template<>
static inline constexpr multi_argument_set_value<myMultiArgSender> = true;

struct myOverloadedSender {
    using value_types = overloads<args<int, double>, args<string>>;
    using error_types = overloads<E1, E2>;
    static inline constexpr bool sets_done = true;

    friend constexpr auto tag_invoke(execution::set_value_t, int, double) { ... }
    friend constexpr auto tag_invoke(execution::set_value_t, string) { ... }
}

template<>
static inline constexpr overloaded_set_value<myOverloadedSender > = true;

We could then use sender_traits to discriminate those cases and also handle the sender_base and no_sender cases. Finally, an algorithm like then could take those traits and filter out what he needs to know and propagate them properly

griwes commented 2 years ago

Having the aliases be of different categories may simplify life for authors of sender factories, but I am expecting that this would make it much harder for authors of sender adaptors, since they would need to deal with all of this complexity every time, instead of simply consuming a canonical form of a given alias uniformly.

I would expect that there would be far more authors of sender adaptors than authors of sender factories. Does anybody disagree with this expectation? If not, then we should optimize for authors of adaptors, not for authors of factories.

ericniebler commented 2 years ago

I am expecting that this would make it much harder for authors of sender adaptors

I seriously doubt that. Here's an exercise: Show me what the metadata for the then sender would look like using Kirk's approach. Then we can discuss relative complexity.

griwes commented 2 years ago

I don't know about Kirk's approach; I was commenting on @miscco's comment, in case that wasn't clear. What then would need to do would be approximately this (I'm omitting doing nice things like removing duplicates from the type list here):

struct compute_overloaded {
    using type = invoke_result_t<F, adapted_sender::value_types>;
};

struct compute_multivalue {
    template<typename ...Args>
    using rebinder = invoke_result_t<F, Args...>;

    using type = rebind<adapted_sender::value_types, rebinder>;
};

struct compute_overloaded {
    template<typename ...Args>
    using rebinder2 = args<invoke_result_t<F, Args...>>;

    template<typename ...Args>
    using rebinder1 = overloads<rebind<Args, rebinder2>...>;

    using type = rebind<adapted_sender::value_types, rebinder1>;
};

using value_types = conditional_t<overloaded_set_value<adapted_sender>,
    compute_overloaded,
    conditional_t<multi_argument_set_value<adapted_sender>,
        compute_multivalue,
        compute_single_value
    >
>::type;

and I'm not sure if this is sufficiently lazy to work - whereas if value_types is required to always follow the same structure, it turns into approximately the following (assuming we go with something like the original comment on this issue suggests):

template<typename ...Args>
using rebinder2 = invoke_result_t<F, Args...>;

template<typename ...Args>
using rebinder1 = overloads<rebind<Args, rebinder2>...>;

using value_types = rebind<adapted_sender::value_types, rebinder1>;

This is significantly shorter and easier to follow. I also agree that it's much simpler than what the paper currently requires, even though the instantiations of rebind are going to be more expensive than just following alias templates, which is what the paper at the moment allows you to do.

I am somewhat okay with what you originally proposed here, Eric. I'm much less okay with allowing value_types to have shapes different than a single canonical one.

ericniebler commented 2 years ago

Ah ok, I misunderstood your objection. Reading @miscco's comment, I had assumed that users could only use the short form as nested aliases within the sender itself, and that sender_traits would normalize it all into a single canonical form for value_types and error_types.

griwes commented 2 years ago

Ah. I see. I missed that. Yes, that would probably be viable.

miscco commented 2 years ago

Yeah, my point was that we should move the complexity from the user definition of a sender into whatever sender_traits distills from it. Thinking about it a bit more, I believe we could get around all those issues, by providing overloads and args as proper facilities to the user.

That way it would be unambiguous whether somebody defines using value_types = int or using value_types = args<int> or using value_types = overloads<args<int>>

Inside sender_traits we would then only have to check for the existence of value_types and could more easily do the filtering of the "right" channel.

We would also avoid a lot of the throughput issues that come with those heavily nested types, as the most common case will be single argument channels

miscco commented 2 years ago

BTW I cannot really give any insights whether it would be better / feasible as I have not yet started implementing sender adaptors / factories. That is the next item on the list

kirkshoop commented 2 years ago

...it seems to increase the amount of metaprogramming that sender authors must do, whereas I'm trying to reduce it.

I do not think so, and I am also trying to reduce it. The current state is awful IMO.

The list of signatures is a representation that is independent of how we program values/errors/done transformations.

...having three separate receiver channels for value, error, and done is the right design. Why should it be the wrong design for the sender's metadata?

because they are not uniform and they do not express all that needs to be expressed.

Variant<Tuple<>> is worse than void(set_value_t, this_&&) Variant<> is worse than expected_set_value_signatures_t<asender>::size == 0 Variant<> is worse than expected_set_error_signatures_t<asender>::size == 0

These can be resolved with helpers like the ones I have added to the godbolt.

Variant<> means different things when it is the result of error_types and value_types. This one is harder to resolve. we always have to keep the name of the alias that produced the type to know what the type means.

void(set_value_t, this_&&) noexcept cannot be expressed void(tag_t<get_scheduler>, const this_&) noexcept cannot be expressed

In order to add additional signals (like when we were planning to add next) would require a new member altogether and change all algorithms and wrappers etc.. to propagate it through explicitly. That is not so with a list of signatures.

I added more tools and added an example of migrating then() from current to new

https://godbolt.org/z/1xzT5d3Mx

This will define the traits for a sender of int that can produce error_code and exception_ptr and will expect the receiver to support three queries.

  signatures_from_t<
    set_value_for<int>,
    set_error_for<std::error_code, std::exception_ptr>,
    set_done_if<true>,
    queries_for<get_time_scheduler_t, tag_t<get_scheduler>, tag_t<get_stop_token>>
    > 
  expected_signatures() const noexcept;
  signatures_from_t<
    set_value_for<int>,
    set_error_for<std::error_code, std::exception_ptr>,
    set_done_if<true>,
    queries_for<get_time_scheduler_t, tag_t<get_scheduler>, tag_t<get_stop_token>>
    > 
  expected_signatures() const noexcept;

This is how then() would specify the traits. I am not happy with transform yet, I want it to be more general.

  signatures_from_t<
    typename transform_arguments_expected_set_value_signatures_t<Predecessor, result_t::template fn>::type,
    // apply all cpos not replaced by transform
    typename remove_expected_set_value_signatures_t<Predecessor>::type,
    // merge in locally expected queries
    typename queries::type>
  expected_signatures() const noexcept;
kirkshoop commented 2 years ago

I am happier with transform_.. now and I added more stuff and refined more stuff

https://godbolt.org/z/dKzE6K3oa

egorich239 commented 2 years ago

We could even employ function types here, which would even give us a way to specify that a particular overload of set_value is noexcept

Would that imply that such noexcept sender would reject to connect to a Rec receiver which provides potentially-throwing set_value((Rec&&) rec) noexcept(false) ?

kirkshoop commented 2 years ago

Would that imply that such noexcept sender would reject to connect to a Rec receiver which provides potentially-throwing set_value((Rec&&) rec) noexcept(false) ?

Yes, that is a valid constraint when a sender declares that it expects noexcept.

ericniebler commented 2 years ago

I have a proposed design up in PR #326, and a follow-on PR that extends this in the direction of dependently-typed senders in PR #320. The idea is to use a pack of function signatures to specify a sender's types:

namespace ex = std::execution;
using _traits = ex::receiver_signatures<
  ex::set_value_t(vals1...),
  ex::set_value_t(vals2...),
  ex::set_error_t(err),
  ex::set_done_t()>;

struct my_overloaded_sender : _traits {
  ....

I'm curious if this is a sufficiently simple interface.