Closed ericniebler closed 3 months ago
Comment by miscco Sunday Jan 30, 2022 at 15:25 GMT
You just beat me to it. Thanks for the write up.
I would also like to add that we should reconsider whether we want to constraint the arguments of execution::start
with operation_state
.
While this creates a circular dependency we could either introduce a helper concept _Operation_state_impl
or let implementors figure out how to solve the issue.
I think from a user perspective it is better to explicitly tell what expect.
Comment by dietmarkuehl Sunday Jan 30, 2022 at 18:18 GMT
I'm pretty sure I have submitted a rather similar dependency issue to get the cyclic dependency between a concept and a CPO removed (in this case between schedule
and scheduler
): I don't think they'd fly in a LWG review either. If the CPO should match the concept conditions exactly, impose the requirements on the CPO and have the concept just check for the CPO being usable.
Comment by dietmarkuehl Sunday Jan 30, 2022 at 18:33 GMT
Looking at the next thing to implement to enable implementation of the various senders, i.e., receiver
shows the specification which I would imagine goes into [exec]: paragraph 3 lists the life-time requirements and also restricts calling the completion operations prior to start
, again somewhat hiding the higher-level requirements in the specification of a particular component, in this case a concept.
Comment by lewissbaker Wednesday Feb 02, 2022 at 06:16 GMT
There was some discussion in Prague, I think, about potentially defining start(rvalueOpState)
(if such a call was well-formed) to mean that the operation-state was not required to be kept alive after the call to start()
returns. e.g. because it enqueues all state elsewhere.
Regarding the circular dependency - I think the concept should depend on the CPO and not the other way around.
An operation_state
is something that supports start()
and that is destructible
.
The connect()
CPO could require that what it returns satisfies operation_state
, however (I believe it does already?).
After reviewing the wording in P2300R10, the remaining issue from the list described above is:
start()
which might call a const-qualified op.start()
member-function. We might want to specify that start(op)
is ill-formed if op
is not a non-const lvalue.The issues with description of the lifetime of an operation I think are addressed by [async.ops] p2 and p7 of P2300R10.
The issues with allowing start() on rvalues is addressed - now says that calling start() on an rvalue is ill-formed.
The circular dependency between CPO and concept is resovled by having the concept require that the CPO invocation is valid, the CPO does not require that the object itself satisfies the concept.
I've moved the remaining issue out into #273. Closing this issue as resolved.
Issue by dietmarkuehl Sunday Jan 30, 2022 at 13:36 GMT Originally opened as https://github.com/NVIDIA/stdexec/issues/440
The specification for
start
uses "The expressionexecution::start(O)
for some lvalue subexpressionO
is expression-equivalent to:" to then go on what happens for lvalues. It seems there is no specification what happens for rvalue subexpressions although my guess is that they should also be ill-formed but I don't think it says so. Separately, the current formulation allows both non-const
andconst
lvalues and I wonder whether this choice is intentional: it may very well be although I think callers ofstart(o)
should always use a non-const
argumento
, at least for operation states obtained from some generic function.The requirement for lvalues is probably driven by the life-time specification: if the argument isn't an lvalue, there is no [named] object whose life time can be required to last a certain amount. Considering the mess we got with iterators originally requiring lvalue references being returned from from
operator*
it isn't immediately clear to me whether there may be situations using a proxy object referring to the actual underlying operation state. For example, I could imagine a setup where the operation state isshared_ptr
meddling with its reference counts to commit suicide upon receiving a completion signal.Speaking of the life time requirement: it seems the requirement on the operation state object is a little bit hidden in the specification of the
start
CPO. The specification as is also makes it conceptually hard to test whetherstart
actually does the correct thing: can I create an objecto
without using theconnect
CPO which can be passed intostart(o)
? It currently somewhat ties multiple things together. Maybe it would be reasonable to have a specification at the top-level section [exec] bringing things together. In particular I think it would be useful to point out thatoperation_state
objects can be destroyed any time beforestart()
is called and any time after any completion function was called.Also, the use of a Mandates clause which isn't a top-level paragraph seems unusual and sort of in the wrong location: Mandates precedes Effects and the existing paragraph, although not labeled as such, seems like an Effects clause.
I don't have a PR for modified wording ready, yet, as I don't know whether there is agreement on the proposed direction for the specification. My personal preferences are:
connect()
, andstart()
, including the life-time constraints onstart()
ed but not completed operation states, at higher level.const
lvalues. When lifting the constraint to use lvalues the life-time constraint would probably need to be a phrased in terms of some underlying object.If there is an agreed direction I can have a go at creating a corresponding PR.