Assorted fixes to async: yield may suspend in async* methods; yield* skips emptysequences; definiti…

docs/language/dartLangSpec.tex

Index: docs/language/dartLangSpec.tex
===================================================================
--- docs/language/dartLangSpec.tex	(revision 44473)
+++ docs/language/dartLangSpec.tex	(working copy)
@@ -3092,10 +3092,12 @@
 The static type of a function literal of the form 
 
 $(T_1$ $a_1, \ldots, T_n$ $a_n, [T_{n+1}$ $x_{n+1} = d_1, \ldots,  T_{n+k}$ $x_{n+k} = d_k])$ \ASYNC{} $=> e$
-is $(T_1 \ldots, T_n, [T_{n+1}$ $x_{n+1}, \ldots, T_{n+k}$ $x_{n+k}]) \rightarrow Future<flatten(T_0)>$, where $T_0$ is the static type of $e$ and  $flatten(T) = flatten(S)$ if $T = Future<S>$, and $T$ otherwise. 
+is $(T_1 \ldots, T_n, [T_{n+1}$ $x_{n+1}, \ldots, T_{n+k}$ $x_{n+k}]) \rightarrow Future<flatten(T_0)>$, where $T_0$ is the static type of $e$ and  $flatten(T) = flatten(S)$ if $T = Future<S>$ and $T \ne S$, and $T$ otherwise. 

[Paul Berry, 2015/03/13 19:59:11]

I'm confused as to why $T \ne S$ is necessary.  Doesn't $T = Future<S>$ imply $T
\ne S$, since infinite types are impossible to create?

[Lasse Reichstein Nielsen, 2015/03/15 11:53:15]

I think we should flatten if T is a subtype of Future<S>, not just if it is
Future<S>. It is possible to have subtypes of Future in the language, and they
should also be flattened. Since you can't be a subtype of Future<S> and
Future<R> at the same time (S != R), there should be at most one Future<?> in
the type hierarchy of T (right?).

[gbracha, 2015/03/16 20:25:23]

The intent was to use subtyping, not equality, but somehow that got missed in
the end. Howevr, I thnk there can be multiple Futures in the hierarchy.


class A implements Future<A>

class B extends A implements Future<B>

No B <: Future<B> and B <: Future<A> (not to mention Future<Object> and
Future<dynamic>). So strictly speaking, flatten would be ill-defined- flatten(B)
could yield B, A, Object or dynamic. But we really want B.  I'll try and reword
to address all this.

 
 \rationale{
 We collapse multiple layers of futures into one. If $e$ evaluates to a future $f$, the future will not invoke its \code{then()} callback until f completes to a non-future value, and so the result of an await is never a future, and the result of an async function will never have type \code{Future$<X>$} where $X$ itself is an invocation of \code{Future}. 
+
+The sole exception to that would be a type $X$ that extended or implemented \code{Future$<X>$}. In that case, the result type is $X$ and no further unwrapping takes place.

[Paul Berry, 2015/03/13 19:59:11]

This is in a \rationale block, so I would expect to see some normative text to
back up this statement.  But the normative definition of flatten above doesn't
seem to account for this case.

Did you perhaps mean to change the definition of flatten to something like this?

"$flatten(T) = flatten(S)$ if $T = Future<S>$, $S$ if $T \ne Future<S>$ but
extends or implements $Future<S>$, and $T$ otherwise."

[Lasse Reichstein Nielsen, 2015/03/15 11:53:15]

So, as stated above, I disagree with that choice.
It also makes the runtime-semantics different from the static semantics, because
at runtime, we don't distinguish between being exactly Future<T> or
extending/implementing Future<T> (all runtime classes are subclasses of the
abstract Future class).

[Paul Berry, 2015/03/15 13:57:22]

Understood.  What I wish we could say is:

  flatten(T) = flatten(S) if T is Future<S> or a subtype of Future<S>, and T
otherwise.

However there's a problem with that definition: it's possible to construct a
legal class hierarchy for which it is undefined:

  class C implements Future<C> { ... }

With this definition, flatten(C) = flatten(C) = flatten(C) = ..., which never
terminates but does reach a fixed point.

Here's an even more pathological example:

  class C<T> implements Future<C<C<T>>> { ... }

With this definition, flatten(C) = flatten(C<C>) = flatten(C<C<C>>) = ..., which
doesn't terminate and doesn't even reach a fixed point.

So we need some way to ensure that the computation of flatten() terminates.  I
think Gilad's approach of unwrapping just once in the case where the type
extends or implements Future is a reasonable compromise: it will do the right
thing in just about every conceivable non-pathological case, and it's
well-defined even in the pathological cases.

(Perhaps we should consider adding a paragraph to the rationale section with one
of these two pathological examples, to clarify why the more obvious definition
of flatten() wasn't chosen).

 }
 
 
@@ -5913,6 +5915,14 @@
 }
 
 \LMHash{}
+Otherwise, if the enclosing function $m$ is marked \ASYNC* (\ref{functions}) then the enclosing function may suspend.

[Lasse Reichstein Nielsen, 2015/03/15 11:53:15]

I still think it should be "must suspend". If that's too expensive, then it's
probably because suspending should be cheaper (maybe not having to pause
incoming streams from nesting await-for statements).

When you put more than one value into a stream in the same microtask, they will
be buffered. There is no need for this buffering - you know that there is an
unpaused listener (otherwise the yield would block or exit), so you might as
well yield to it immediately after adding the value.
This is also consistent with the use of the word "yield" which is traditionally
used to yield control.

[gbracha, 2015/03/16 20:25:22]

Per discussion with Lars, we are not doing that. There are other ways to skin
this cat and we won't overspecify them here.

+
+\rationale {
+If a \YIELD{} occurred inside an infinite loop and the enclosing function never suspended, there might not be an opportunity for consumers of the  enclosing stream to run and access the data in the stream.  The stream might then accumulate an unbounded number of elements. Such a situation is untenable. Therefore, we allow the enclosing function to be suspended when a new value is added to its associated stream. However, it is not essential (and in fact, can be quite costly) to suspend the function on every \YIELD{}. The implementation is free to decide how often to suspend the enclosing function. The only requirement is that consumers are not blocked indefinitely.
+}
+
+
+\LMHash{}
 If the enclosing function $m$ is marked \SYNC* (\ref{functions}) then:
 \begin{itemize}
 \item
@@ -5947,28 +5957,40 @@
 \end{grammar}
 
 \LMHash{}
-Execution of a statement s of the form \code{\YIELD* $e$;}  proceeds as follows:
+Execution of a statement $s$ of the form \code{\YIELD* $e$;}  proceeds as follows:
 
 \LMHash{}
-First, the expression $e$ is evaluated to an object $o$. If the immediately enclosing function $m$ is synchronous, then it is a dynamic error if the class of $o$ does not implement \code{Iterable}.  If $m$ asynchronous, then it is a dynamic error if the class of $o$ does not implement \code{Stream}. Next, for each element $x$ of $o$:
-\begin{itemize}
+First, the expression $e$ is evaluated to an object $o$. 
+
+\LMHash{}
+If the immediately enclosing function $m$ is marked \SYNC* (\ref{functions}), then:
+\begin{enumerate}
+\item It is a dynamic error if the class of $o$ does not implement \code{Iterable}.  Otherwise

[Lasse Reichstein Nielsen, 2015/03/15 11:53:15]

Still think this line should be dropped. If you can get "iterator" on the object
and call "moveNext" and "current" on the result, it should work, just as for-in
does.
You don't *need* the "Iterable" check for anything, because you need to check
every the result of every call anyway. Having this check, even in production
mode, is pure overhead.

[gbracha, 2015/03/16 20:25:22]

My goal in this CL is to deal with oversights and errors in the existing spec,
not to change it. That is still being discussed with TC52, and may show up in
another CL.

+\item The method \cd{iterator} is invoked upon $o$ returning an object $i$. 

[Lasse Reichstein Nielsen, 2015/03/15 11:53:16]

Or it may throw - implementing an interface doesn't force you to do it
correctly. Do we need to say that if it throws, the error is propagated
(execution of s completes and throws the same error)?

+\item \label{moveNext} The \cd{moveNext} method of $i$ is invoked. If \cd{moveNext} returns \FALSE{} execution of $s$ is complete. Otherwise

[Lasse Reichstein Nielsen, 2015/03/15 11:53:16]

Should we say the the moveNext method of i is invoked with zero arguments?

[Lasse Reichstein Nielsen, 2015/03/15 11:53:16]

The result of moveNext should be subjected to boolean conversion, and if the
result of that isn't true, execution is complete (and boolean conversion may
throw in checked mode).

[gbracha, 2015/03/16 22:21:40]

Done.

+\item An element $x$ is extracted from $i$ and $x$ is added to the iterable or stream associated with $m$.

[Lasse Reichstein Nielsen, 2015/03/15 11:53:16]

"extracted from" doesn't say how.

What you do is to try to call the current" getter on i.
If that throws, execution of $s$ is complete and throws the same error.
Otherwise x is the result of calling the getter.

[gbracha, 2015/03/16 22:21:40]

But for streams it's a bit different. So I abstracted form that. 

[Lasse Reichstein Nielsen, 2015/03/17 07:11:21]

It's very different for Stream, but this text only applies to sync*, so there is
no need for abstraction.

Maybe all this can be reduced to: 
   yield* e
is equivalent to
   for (var tmp in e as Iterable) {  // "as Iterable" only so it throws if not.
     yield tmp;
   }
or directly:
   var itl = e as Iterable;
   var itr = itl.iterator;
   while (itr.moveNext()) {
     yield itr.current;
   }
just as it's done for the synchronous "for-in".

+Execution of the function $m$ immediately enclosing $s$ is suspended until the method \code{moveNext()} is invoked upon the iterator used to initiate the current invocation of $m$, at which point execution of $s$ continues at \ref{moveNext}. 
 \item
-If $m$ is marked \ASYNC* (\ref{functions}) and the stream $u$ associated with $m$ has been paused,  then execution of $m$ is suspended until $u$ is resumed or canceled.
-\item
- $x$ is added to the iterable or stream associated with $m$ in the order it appears in $o$.  
- \item
-If $m$ is marked \ASYNC* and the stream $u$ associated with $m$ has been canceled, then let $c$ be the \FINALLY{} clause (\ref{try}) of the innermost enclosing try-finally statement, if any. If $c$ is defined,  let $h$ be the handler induced by $c$. If $h$ is defined, control is transferred to $h$. If $h$ is undefined, the immediately enclosing function terminates.
-\end{itemize}
+The current call to \code{moveNext()} returns \TRUE.
+\end{enumerate}
 
 \LMHash{}
-If the enclosing function is marked \SYNC* (\ref{functions}) then:
+If $m$ is marked \ASYNC* (\ref{functions}), then:
 \begin{itemize}
+\item  It is a dynamic error if the class of $o$ does not implement \code{Stream}. Otherwise
+\item For each element $x$ of $o$:
+\begin{itemize}
 \item
-Execution of the function $m$ immediately enclosing $s$ is suspended until the method \code{moveNext()} is invoked upon the iterator used to initiate the current invocation of $m$. 
+If the stream $u$ associated with $m$ has been paused,  then execution of $m$ is suspended until $u$ is resumed or canceled.
+ \item
+If the stream $u$ associated with $m$ has been canceled, then let $c$ be the \FINALLY{} clause (\ref{try}) of the innermost enclosing try-finally statement, if any. If $c$ is defined,  let $h$ be the handler induced by $c$. If $h$ is defined, control is transferred to $h$. If $h$ is undefined, the immediately enclosing function terminates.
 \item
-The current call to \code{moveNext()} returns \TRUE.
+Otherwise,  $x$ is added to the stream associated with $m$ in the order it appears in $o$.  The function $m$ may suspend.
 \end{itemize}
+\item If the stream $o$ is done, execution of $s$ is complete. 
+\end{itemize}
 
+
 \LMHash{}
 It is a compile-time error if a yield-each statement appears in a function that is not a generator function.