atomgalaxy / review-executor-sendrecv

A board for reviewing the executors proposal P0440
1 stars 0 forks source link

as-invocable #5

Open RobertLeahy opened 4 years ago

RobertLeahy commented 4 years ago

Ref qualification of operator()

I don't understand the motivation for the function call operator of as-invocable being mutable lvalue ref qualified.

It doesn't make sense conceptually to me given that the "receiver contract" (§1.5.1) implies that invoking the function call operator of an instance of as-invocable is "terminal" (since either std::execution::set_value will be called and not throw, or ::set_error will be called). As such this function being invocable on rvalues seems perfectly legitimate. If anything it seems to me that it should be disallowed to call it on lvalues, not vice versa.

Beyond not making conceptual sense §2.2.3.4 doesn't seem to prohibit std::execution::execute from treating submitted function objects as rvalues when invoking them. In fact the example inline_executor (§1.3) potentially does exactly this (as it employs perfect forwarding).

Which means that inline_executor doesn't seem to work with the current formulation of as-invocable.

Passing *r_ to std::execution::set_value as an rvalue

By passing *r_ to std::execution::set_value as R&& rather than R& it gives an indication to the chosen implementation of that customization point that it may consume *r_. But this is clearly not the case since as-invocable may need to reuse that value later to pass it to std::execution::set_error.

Clearly passing *r_ as R&& to std::execution::set_value makes sense if that call will always be "terminal" (i.e. not throw), but given that it may throw (thereby necessitating the reuse of the object) it seems incorrect (or at least misleading) to pass it by rvalue.

dietmarkuehl commented 4 years ago

The same problem exists with the concept receiver_of which requires execution::set_value(std::move(t), (An&&) an...); while it is also stated that is still valid to set_error or set_done if set_value throws an exception. As std::move(t) is just a no-op these can be achieved if set_value isn't allowed to "consume" the argument, i.e., leave the referenced object unchanged it an exception is thrown.

It seems the intention was to indicate that the receiver can be used just once. In case of an exception from set_value that isn't really true, though. It is true for set_error and set_done as neither of these can throw.

tomaszkam commented 4 years ago

I think this is very similiar to map.try_emplace(k, args...) where args are not touched if nothing is emplaced. Similiary, we need to require that receiver r is not fullfiled if set_value(std::move(r), args...) throws, i.e. r will not be fully consumed - second set_value may fail.

RobertLeahy commented 4 years ago

I think this could be clarified in fleshing out:

[Editorial note: We should probably define what “send the value(s) Vs... to the receiver R’s value channel” means more carefully. –end editorial note]

ericniebler commented 4 years ago

Yes, the alternative being that we allow set_value, set_error, and set_done to be implicitly consuming, but not require the receiver to be moved in. This was discussed. There are points on both sides.