Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
498873003:
async/await, async*, sync* and yield (Closed)
|
Created:
6 years, 4 months ago by gbracha Modified:
6 years, 3 months ago Reviewers:
nweiz, Lasse Reichstein Nielsen, Kevin Millikin (Google), bakster, kasperl, Bob Nystrom, floitsch CC:
reviews_dartlang.org Visibility:
Public. |
Descriptionasync/await, async*, sync* and yield
Committed: https://code.google.com/p/dart/source/detail?r=40052
Patch Set 1 #Patch Set 2 : #
Total comments: 18
Patch Set 3 : #
Total comments: 32
Patch Set 4 : #Patch Set 5 : #Patch Set 6 : #
Total comments: 4
Messages
Total messages: 23 (1 generated)
This CL integrates the complete spec for async/await and friends into the Dart spec.
On 2014/08/22 22:17:06, gbracha wrote: > This CL integrates the complete spec for async/await and friends into the Dart > spec. Also, the PDF is at https://drive.google.com/a/google.com/file/d/0BwbTAYsoT4huMUF5VUlVUzA0Z0E/vie...
On 2014/08/22 22:17:06, gbracha wrote: > This CL integrates the complete spec for async/await and friends into the Dart > spec. Function expression only allow the async modifier; not sure if it is super useful but since await is an expression inside an async* context, we may allow `(x,y,z) async* => e` as well.
On 2014/08/24 19:10:50, erik.meijer wrote: > On 2014/08/22 22:17:06, gbracha wrote: > > This CL integrates the complete spec for async/await and friends into the Dart > > spec. > > Function expression only allow the async modifier; not sure if it is super > useful but since await is an expression inside an async* context, we may allow > `(x,y,z) async* => e` as well. No, we should not go there. It is complex enough already.
Ping? I know it is a rather complex change, but it has essentially been reviewed as stand alone document.
Partial review. https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:505: \item of the form \code{=$>$ $e$} which is equivalent to a body of the form \code{\{\RETURN{} $e$;\}} or the form \code{\ASYNC{} =$>$ $e$} which is equivalent to a body of the form \code{\ASYNC{} \{\RETURN{} $e$;\}}. \rationale{The other modifiers do not apply here, because they apply only to generators, discussed below, and generators do not allow the form \code{\RETURN{} $e$}; values are added to the generated stream or iterable using \YIELD{} instead.} at "the other modifiers", maybe add "(\ASYNC* and \SYNC*)" as elaboration. This is just a rationale, so it may be ok to let some things be implicit. https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:2715: The {\em current exception} is the last unhandled exception thrown at a given moment during runtime. It's unclear what "unhandled" means, or when an exception ceases to be unhandled. https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:2725: If $e$ evaluates to \NULL{} (\ref{null}), then a \code{NullThrownError} is thrown. Otherwise the current exception is set to $v$ and the current return value (\ref{return}) becomes undefined. No mention of "current stack trace". https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:2732: If $f$ is marked \ASYNC{} or \ASYNC* (\ref{functions}) and there is a dynamically enclosing exception handler $h$ (\ref{try}) introduced by the current activation, control is transferred to $h$, otherwise $f$ terminates. Will there always be a dynamically enclosing exception handler introduced by the current activation? It is written like a condition that can be unsatisfied, but that would mean that the throw would not end up in a future or stream? https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:2738: Otherwise, control is transferred to the nearest dynamically enclosing exception handler. There is no "if" for this "otherwise". The sentence above the rationale had both "if" and "otherwise" by itself. Should this just be the "otherwise" of the paragraph above? It avoids having to say what happens when $f$ terminates. https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:2771: is $(T_1 \ldots, T_n, [T_{n+1}$ $x_{n+1}, \ldots, T_{n+k}$ $x_{n+k}]) \rightarrow T_0$, where $T_0$ is the static type of $e$. In any case where $T_i, 1 \le i \le n+k$, is not specified, it is considered to have been specified as \DYNAMIC{}. Should be Future<T_0> for the async function literal. Dittos below. https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:3070: If $f$ is marked \ASYNC{} (\ref{functions}), then a fresh instance (\ref{generativeConstructors}) $o$ implementing the built-in class \code{Future} is associated with the invocation and immediately returned to the caller. The body of $f$ is scheduled for execution at some future time. The future $o$ will complete when $f$ terminates. The value used to complete $o$ is the current return value (\ref{return}), if it is defined, and the current exception (\ref{throw}) otherwise. And current stack trace. https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:3078: \item If $s$ has been canceled then its cancellation future is completed with $x$. add "as an error" - this could be read as the expression ending up as a value of the future, not an error. https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:3087: If $f$ is asynchronous then when $f$ terminates any open stream subscriptions associated with any asynchronous for loops (\ref{asynchronousFor-in}) or yield-each statements (\ref{yieldEach}) executing within $f$ are canceled. Should there be a comma before "any". Slightly hard to read.
A few tweaks in response to Lasse's review. And a few minor corrections I encountered. https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:505: \item of the form \code{=$>$ $e$} which is equivalent to a body of the form \code{\{\RETURN{} $e$;\}} or the form \code{\ASYNC{} =$>$ $e$} which is equivalent to a body of the form \code{\ASYNC{} \{\RETURN{} $e$;\}}. \rationale{The other modifiers do not apply here, because they apply only to generators, discussed below, and generators do not allow the form \code{\RETURN{} $e$}; values are added to the generated stream or iterable using \YIELD{} instead.} On 2014/08/28 14:35:49, Lasse Reichstein Nielsen wrote: > at "the other modifiers", maybe add "(\ASYNC* and \SYNC*)" as elaboration. This > is just a rationale, so it may be ok to let some things be implicit. I think it's fine as a rationale. https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:2715: The {\em current exception} is the last unhandled exception thrown at a given moment during runtime. On 2014/08/28 14:35:49, Lasse Reichstein Nielsen wrote: > It's unclear what "unhandled" means, or when an exception ceases to be > unhandled. How about: The last exception that has been raised and not subsequently caught. https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:2715: The {\em current exception} is the last unhandled exception thrown at a given moment during runtime. On 2014/08/28 14:35:49, Lasse Reichstein Nielsen wrote: > It's unclear what "unhandled" means, or when an exception ceases to be > unhandled. Rephrased. https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:2732: If $f$ is marked \ASYNC{} or \ASYNC* (\ref{functions}) and there is a dynamically enclosing exception handler $h$ (\ref{try}) introduced by the current activation, control is transferred to $h$, otherwise $f$ terminates. On 2014/08/28 14:35:49, Lasse Reichstein Nielsen wrote: > Will there always be a dynamically enclosing exception handler introduced by the > current activation? > It is written like a condition that can be unsatisfied, but that would mean that > the throw would not end up in a future or stream? If there isn't one defined via a try statement, then there isn't one as far as the language spec is concerned. The implementation may have introduced one under the covers, but that is irrelevant. https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:2738: Otherwise, control is transferred to the nearest dynamically enclosing exception handler. On 2014/08/28 14:35:49, Lasse Reichstein Nielsen wrote: > There is no "if" for this "otherwise". The sentence above the rationale had both > "if" and "otherwise" by itself. > > Should this just be the "otherwise" of the paragraph above? It avoids having to > say what happens when $f$ terminates. Yes, it is the otherwise of (if f is marked async or async*). Reshuffled to make things clearer. https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:2771: is $(T_1 \ldots, T_n, [T_{n+1}$ $x_{n+1}, \ldots, T_{n+k}$ $x_{n+k}]) \rightarrow T_0$, where $T_0$ is the static type of $e$. In any case where $T_i, 1 \le i \le n+k$, is not specified, it is considered to have been specified as \DYNAMIC{}. On 2014/08/28 14:35:49, Lasse Reichstein Nielsen wrote: > Should be Future<T_0> for the async function literal. Dittos below. Good catch. Fixed. https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:3070: If $f$ is marked \ASYNC{} (\ref{functions}), then a fresh instance (\ref{generativeConstructors}) $o$ implementing the built-in class \code{Future} is associated with the invocation and immediately returned to the caller. The body of $f$ is scheduled for execution at some future time. The future $o$ will complete when $f$ terminates. The value used to complete $o$ is the current return value (\ref{return}), if it is defined, and the current exception (\ref{throw}) otherwise. On 2014/08/28 14:35:49, Lasse Reichstein Nielsen wrote: > And current stack trace. Not sure what you mean here? https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:3087: If $f$ is asynchronous then when $f$ terminates any open stream subscriptions associated with any asynchronous for loops (\ref{asynchronousFor-in}) or yield-each statements (\ref{yieldEach}) executing within $f$ are canceled. On 2014/08/28 14:35:49, Lasse Reichstein Nielsen wrote: > Should there be a comma before "any". Slightly hard to read. Gave you two commas. Extra breathing roon :-)
floitsch@google.com changed reviewers: + floitsch@google.com
DBC. I haven't finished reading the remainder of the changes, but here is a first round of comments on the "async" section. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:3116: If $f$ is marked \ASYNC{} (\ref{functions}), then a fresh instance (\ref{generativeConstructors}) $o$ implementing the built-in class \code{Future} is associated with the invocation and immediately returned to the caller. The body of $f$ is scheduled for execution at some future time. The future $o$ will complete when $f$ terminates. The value used to complete $o$ is the current return value (\ref{return}), if it is defined, and the current exception (\ref{throw}) otherwise. I strongly disagree with this semantics: an async function should execute until it encounters an await, or the return of the function. Basically you want the following transformation: foo async { ... } foo { return new Future.sync(() { <transformed ...> }; } This deals nicely with thrown exceptions and just needs to deal with awaits. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:3116: If $f$ is marked \ASYNC{} (\ref{functions}), then a fresh instance (\ref{generativeConstructors}) $o$ implementing the built-in class \code{Future} is associated with the invocation and immediately returned to the caller. The body of $f$ is scheduled for execution at some future time. The future $o$ will complete when $f$ terminates. The value used to complete $o$ is the current return value (\ref{return}), if it is defined, and the current exception (\ref{throw}) otherwise. You must not define something as "schedule for execution at some future time". There is no way you will get away with that. Applications must know when functions are executed. Otherwise there could be a DOM redraw in the middle. If the "some future time" was really implemented as `new Timer` the whole proposal would be unusable in browsers for UI work. This is another reason for doing the async transformation as 'Future.sync'. This way you don't have to deal with when code is executed. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:3116: If $f$ is marked \ASYNC{} (\ref{functions}), then a fresh instance (\ref{generativeConstructors}) $o$ implementing the built-in class \code{Future} is associated with the invocation and immediately returned to the caller. The body of $f$ is scheduled for execution at some future time. The future $o$ will complete when $f$ terminates. The value used to complete $o$ is the current return value (\ref{return}), if it is defined, and the current exception (\ref{throw}) otherwise. This specification does not deal with zones. You should either specify it, or base yourself on the existing Future class (and not just its interface). I think we can give ourselves some room, and not specify everything to the last detail, but there must be some guarantees, like "returns an instance of the dart:async Future class (as if created by one of its constructors)". This doesn't prohibit us from creating several of them (which would be visible with zones!) and or take shortcuts as long as we could replicate the semantics with a Future construction call. Note: if you skip zones you would break the Angular testing framework (assuming they use async/await). Note2: Streams actually are relatively simple wrt zones (they run in the zone of the `listen`). So this is mainly an "async" and not "async*" issue.
rnystrom@google.com changed reviewers: + rnystrom@google.com
https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:3116: If $f$ is marked \ASYNC{} (\ref{functions}), then a fresh instance (\ref{generativeConstructors}) $o$ implementing the built-in class \code{Future} is associated with the invocation and immediately returned to the caller. The body of $f$ is scheduled for execution at some future time. The future $o$ will complete when $f$ terminates. The value used to complete $o$ is the current return value (\ref{return}), if it is defined, and the current exception (\ref{throw}) otherwise. On 2014/08/29 09:08:21, floitsch wrote: > I strongly disagree with this semantics: an async function should execute until > it encounters an await, or the return of the function. > > Basically you want the following transformation: > foo async { ... } > foo { return new Future.sync(() { <transformed ...> }; } > > This deals nicely with thrown exceptions and just needs to deal with awaits. Natalie and I spent a little time talking to Kevin and Erik about this a few days ago. We worried about this corner of the semantics too. We have some cases in pub where we have an asynchronous method that *must* do some synchronous work at the beginning. If I remember right, our discussion boiled down to: 1. We don't know how many places in pub require this, and it may not be many. We could always not use async syntax for them. 2. We like the simpler semantics of being async from the beginning. 3. We absolutely want to ensure an async method never throws synchronously. Now that I've thought about it a bit more, I think I side with Florian. Future.sync() does address our third concern here. I don't think "sync until the first await" is much harder to reason about, and it supports cases where that actually matters. A common one for us is stuff like: Future _pendingDownload; Future startDownload() { // Only download once! if (_pendingDownload != null) return _pendingDownload; _pendingDownload = // Begin async download... return _pendingDownload; } Here, it's critical that we do the if (_pendingDownload) check *synchronously* so that we ensure we avoid duplicate work. The current async proposal doesn't support that.
On 2014/08/29 16:35:33, Bob Nystrom wrote: > https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... > File docs/language/dartLangSpec.tex (right): > > https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... > docs/language/dartLangSpec.tex:3116: If $f$ is marked \ASYNC{} > (\ref{functions}), then a fresh instance (\ref{generativeConstructors}) $o$ > implementing the built-in class \code{Future} is associated with the invocation > and immediately returned to the caller. The body of $f$ is scheduled for > execution at some future time. The future $o$ will complete when $f$ terminates. > The value used to complete $o$ is the current return value (\ref{return}), if it > is defined, and the current exception (\ref{throw}) otherwise. > On 2014/08/29 09:08:21, floitsch wrote: > > I strongly disagree with this semantics: an async function should execute > until > > it encounters an await, or the return of the function. > > > > Basically you want the following transformation: > > foo async { ... } > > foo { return new Future.sync(() { <transformed ...> }; } > > > > This deals nicely with thrown exceptions and just needs to deal with awaits. > > Natalie and I spent a little time talking to Kevin and Erik about this a few > days ago. We worried about this corner of the semantics too. We have some cases > in pub where we have an asynchronous method that *must* do some synchronous work > at the beginning. > > If I remember right, our discussion boiled down to: > > 1. We don't know how many places in pub require this, and it may not be many. We > could always not use async syntax for them. > 2. We like the simpler semantics of being async from the beginning. > 3. We absolutely want to ensure an async method never throws synchronously. > > Now that I've thought about it a bit more, I think I side with Florian. > Future.sync() does address our third concern here. I don't think "sync until the > first await" is much harder to reason about, and it supports cases where that > actually matters. A common one for us is stuff like: > > Future _pendingDownload; > > Future startDownload() { > // Only download once! > if (_pendingDownload != null) return _pendingDownload; > > _pendingDownload = // Begin async download... > > return _pendingDownload; > } > > Here, it's critical that we do the if (_pendingDownload) check *synchronously* > so that we ensure we avoid duplicate work. The current async proposal doesn't > support that. The C# experience indicates otherwise. See http://tomasp.net/blog/csharp-async-gotchas.aspx/ It's not really surprising that the clean semantics work better overall; they always do. There is a straightforward workaround for this situation. Future startDownload() { doDownload async { ... do async download ...} if (_pendingDownload != null) return _pendingDownload; _pendingDownload = doDownload(); return _pendingDowload; }
On 2014/08/29 16:35:33, Bob Nystrom wrote: > https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... > File docs/language/dartLangSpec.tex (right): > > https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... > docs/language/dartLangSpec.tex:3116: If $f$ is marked \ASYNC{} > (\ref{functions}), then a fresh instance (\ref{generativeConstructors}) $o$ > implementing the built-in class \code{Future} is associated with the invocation > and immediately returned to the caller. The body of $f$ is scheduled for > execution at some future time. The future $o$ will complete when $f$ terminates. > The value used to complete $o$ is the current return value (\ref{return}), if it > is defined, and the current exception (\ref{throw}) otherwise. > On 2014/08/29 09:08:21, floitsch wrote: > > I strongly disagree with this semantics: an async function should execute > until > > it encounters an await, or the return of the function. > > > > Basically you want the following transformation: > > foo async { ... } > > foo { return new Future.sync(() { <transformed ...> }; } > > > > This deals nicely with thrown exceptions and just needs to deal with awaits. > > Natalie and I spent a little time talking to Kevin and Erik about this a few > days ago. We worried about this corner of the semantics too. We have some cases > in pub where we have an asynchronous method that *must* do some synchronous work > at the beginning. > > If I remember right, our discussion boiled down to: > > 1. We don't know how many places in pub require this, and it may not be many. We > could always not use async syntax for them. > 2. We like the simpler semantics of being async from the beginning. > 3. We absolutely want to ensure an async method never throws synchronously. > > Now that I've thought about it a bit more, I think I side with Florian. > Future.sync() does address our third concern here. I don't think "sync until the > first await" is much harder to reason about, and it supports cases where that > actually matters. A common one for us is stuff like: > > Future _pendingDownload; > > Future startDownload() { > // Only download once! > if (_pendingDownload != null) return _pendingDownload; > > _pendingDownload = // Begin async download... > > return _pendingDownload; > } > > Here, it's critical that we do the if (_pendingDownload) check *synchronously* > so that we ensure we avoid duplicate work. The current async proposal doesn't > support that. The C# experience indicates otherwise. See http://tomasp.net/blog/csharp-async-gotchas.aspx/ It's not really surprising that the clean semantics work better overall; they always do. There is a straightforward workaround for this situation. Future startDownload() { doDownload async { ... do async download ...} if (_pendingDownload != null) return _pendingDownload; _pendingDownload = doDownload(); return _pendingDowload; }
nweiz@google.com changed reviewers: + nweiz@google.com
https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:3116: If $f$ is marked \ASYNC{} (\ref{functions}), then a fresh instance (\ref{generativeConstructors}) $o$ implementing the built-in class \code{Future} is associated with the invocation and immediately returned to the caller. The body of $f$ is scheduled for execution at some future time. The future $o$ will complete when $f$ terminates. The value used to complete $o$ is the current return value (\ref{return}), if it is defined, and the current exception (\ref{throw}) otherwise. On 2014/08/29 16:35:32, Bob Nystrom wrote: > On 2014/08/29 09:08:21, floitsch wrote: > > I strongly disagree with this semantics: an async function should execute > until > > it encounters an await, or the return of the function. > > > > Basically you want the following transformation: > > foo async { ... } > > foo { return new Future.sync(() { <transformed ...> }; } > > > > This deals nicely with thrown exceptions and just needs to deal with awaits. > > Natalie and I spent a little time talking to Kevin and Erik about this a few > days ago. We worried about this corner of the semantics too. We have some cases > in pub where we have an asynchronous method that *must* do some synchronous work > at the beginning. > > If I remember right, our discussion boiled down to: > > 1. We don't know how many places in pub require this, and it may not be many. We > could always not use async syntax for them. > 2. We like the simpler semantics of being async from the beginning. > 3. We absolutely want to ensure an async method never throws synchronously. > > Now that I've thought about it a bit more, I think I side with Florian. > Future.sync() does address our third concern here. I don't think "sync until the > first await" is much harder to reason about, and it supports cases where that > actually matters. A common one for us is stuff like: > > Future _pendingDownload; > > Future startDownload() { > // Only download once! > if (_pendingDownload != null) return _pendingDownload; > > _pendingDownload = // Begin async download... > > return _pendingDownload; > } > > Here, it's critical that we do the if (_pendingDownload) check *synchronously* > so that we ensure we avoid duplicate work. The current async proposal doesn't > support that. I also agree with Florian and Bob. I think users will find it surprising if side effects don't execute immediately upon calling a function, even if that function is asynchronous. After all, that's how asynchronous functions work today. This can affect the use case Bob mentioned as well as logging and other IO. There's also a performance implication here. The vast majority of async functions are going to hit an await when they're run. This means that an additional microtask at the beginning of the function is superfluous. Microtasks don't take a lot of time, but they do take some, and if there's an extra one at the beginning of every function in an asynchronous codebase that can add up. One data point here is that pub had a small but noticeable speed increase for IO-heavy operations when it switched from asynchronous to synchronous IO.
Comments up to, but not including, "yield". https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/498873003/diff/20001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:3070: If $f$ is marked \ASYNC{} (\ref{functions}), then a fresh instance (\ref{generativeConstructors}) $o$ implementing the built-in class \code{Future} is associated with the invocation and immediately returned to the caller. The body of $f$ is scheduled for execution at some future time. The future $o$ will complete when $f$ terminates. The value used to complete $o$ is the current return value (\ref{return}), if it is defined, and the current exception (\ref{throw}) otherwise. The returned future should be completed with both the current exception and the current stack trace, just as if you catch the throw with "catch(e,s)". The current stack trace is not part of the thrown value. The current stack trace isn't mentioned here, or at the throw. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:2726: Side not: Should we say that if the object *extends* the library class Error, the current stack trace is set on the object, if a stack trace hasn't already been set in this way, so that it is returned by the stackTrace getter on the Error class? This seems like something that should be mentioned in the spec, and I can't find it elsewhere. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:2735: If $f$ is marked \SYNC* then the dynamically enclosing exception handler encloses the call to \code{moveNext()} that initiated the evaluation of the throw expression. This sounds like "the" dynamically enclosing exception handler is always the one surrounding the entire moveNext call. Change to "... the a dynamically enclosing ..."? https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:3116: If $f$ is marked \ASYNC{} (\ref{functions}), then a fresh instance (\ref{generativeConstructors}) $o$ implementing the built-in class \code{Future} is associated with the invocation and immediately returned to the caller. The body of $f$ is scheduled for execution at some future time. The future $o$ will complete when $f$ terminates. The value used to complete $o$ is the current return value (\ref{return}), if it is defined, and the current exception (\ref{throw}) otherwise. I think I agree with Florian, and would also prefer to have an "async" function work synchronously until the first await, and then let the future awaited there decide when it continues. That may be as simple a change as replacing Future.scheduleMicrotask with Future.sync in the rewriting code. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:4043: If $e$ evaluates to an instance of \code{Future}, $f$, then execution of the function $m$ immediately enclosing $a$ is suspended until after $f$ completes. The stream associated with the innermost enclosing asynchronous for loop (\ref{asynchronousFor-in}), if any, is paused. At some time after $f$ is completed, control returns to the current invocation. The stream associated with the innermost enclosing asynchronous for loop (\ref{asynchronousFor-in}), if any, is resumed. If $f$ has completed with an exception $x$, $a$ raises $x$. If $f$ completes with a value $v$, $a$ evaluates to $v$. When raising $x$, it should also set the current stack trace to the stack trace from the future. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:4047: It is a compile-time error if the function immediately enclosing $a$ is not declared asynchronous. If the function is not asynchronous, then it would not be parsed as an await expression, and await isn't a reserved word there. The expression "await(42)" is a valid function call in a non-async function. The expression "await 42" is a normal syntax error. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:4238: It is a compile-time error if any of the identifiers \ASYNC, \AWAIT{} or \YIELD{} is used as an identifier in a function body marked with either \ASYNC{}, \ASYNC* or \SYNC*. Why is async restricted? I don't think we use it in syntax inside a function body, only as a marker before the body. How about the parameter names of an async function, can one of them be named "await"? (I guess so, you just won't be able to refer to it?) https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:4957: An \ON{}-\CATCH{} clause of the form \code{\ON{} $T$ \CATCH{} ($p_1, p_2$) $s$} introduces a new scope $CS$ in which local variables specified by $p_1$ and $p_2$ are defined. The statement $s$ is enclosed within $CS$. The static type of $p_1$ is $T$ and the static type of $p_2$ is \code{StackTrace}. The local variables should be final. The RETHROW description above assumes that p1, p2 refers to the exception and stacktrace, which might not be the case if you can assign to p1 or p2 before rethrowing. The VM implements the variables as final, dart2js has them modifiable (but still rethrows the original values), the analyzer does not complain on assignment. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:4968: The {\em active stack trace} is an object whose \code{toString()} method produces a string that is a record of exactly those function activations within the current isolate that had not completed execution at the point where the current exception (\ref{throw}) was thrown. I find it a little confusing that the current stack trace is defined at the catch, not the at the throw, and it means that the same explanation is needed where async code handles throws. And if we specify that Error.stackTrace is set when thrown, it might be easier if it is handled at the throw, not at the catch (but it might also be just the same). https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:5050: The {\em return statement} returns a result to the caller of a synchronous function, completes the future associated with an asynchronous function or closes the stream associated with an asynchronous generator (\ref{functions}). Should also address a synchronous generator - the return makes the current (and all future) calls to moveNext on the generated Iterator return false, and sets its "current" to null. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:5060: Due to \FINALLY{} clauses, the precise behavior of \RETURN{} is a little more involved. Whether the value a return statement is supposed to return is actually returned depends on the behavior of any \FINALLY{} clauses in effect when executing the return. A \FINALLY{} clause may choose to return another value, or throw an exception, or even redirect control flow leading to other returns or throws. All a return statement really does is set a value that is intended to be returned when the function terminates. It's a little harsh on "return" to say that that is *all* it does :) It also starts propagating through any finally blocks in order to terminate the function. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:5078: In the simplest case, the immediately enclosing function is an ordinary, synchronous non-generator, and upon function termination, the current return value is given to the caller. The other possibility is that the function is marked \ASYNC{}, in which case the current return value is used to complete the future associated with the function. Both these scenarios are specified in section \ref{functionInvocation}. -> ... associated with the function *invocation*.
Revised some warnings in response to feedback. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:3116: If $f$ is marked \ASYNC{} (\ref{functions}), then a fresh instance (\ref{generativeConstructors}) $o$ implementing the built-in class \code{Future} is associated with the invocation and immediately returned to the caller. The body of $f$ is scheduled for execution at some future time. The future $o$ will complete when $f$ terminates. The value used to complete $o$ is the current return value (\ref{return}), if it is defined, and the current exception (\ref{throw}) otherwise. On 2014/08/29 09:08:21, floitsch wrote: > I strongly disagree with this semantics: an async function should execute until > it encounters an await, or the return of the function. > > Basically you want the following transformation: > foo async { ... } > foo { return new Future.sync(() { <transformed ...> }; } > > This deals nicely with thrown exceptions and just needs to deal with awaits. See comments in response to Bob below. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:3116: If $f$ is marked \ASYNC{} (\ref{functions}), then a fresh instance (\ref{generativeConstructors}) $o$ implementing the built-in class \code{Future} is associated with the invocation and immediately returned to the caller. The body of $f$ is scheduled for execution at some future time. The future $o$ will complete when $f$ terminates. The value used to complete $o$ is the current return value (\ref{return}), if it is defined, and the current exception (\ref{throw}) otherwise. On 2014/08/29 09:08:21, floitsch wrote: > You must not define something as "schedule for execution at some future time". > There is no way you will get away with that. That's what they tell bank robbers and kidnappers. I could say that it will be scheduled by the Future class. However I looked at the constructor Future() and it says nothing at all about when the future it creates will be scheduled. Is there somewhere in the dart doc I've missed that actually specifies that? Besides, it is quite acceptable to view this as a quality of implementation issue. After all, if you have a crappy scheduler, the system will be unusable anyway. Some Dart implementations won't run in a browser. There is no need to overspecify things. > Applications must know when functions are executed. Otherwise there could be a > DOM redraw in the middle. > If the "some future time" was really implemented as `new Timer` the whole > proposal would be unusable in browsers for UI work. > > This is another reason for doing the async transformation as 'Future.sync'. This > way you don't have to deal with when code is executed. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:3116: If $f$ is marked \ASYNC{} (\ref{functions}), then a fresh instance (\ref{generativeConstructors}) $o$ implementing the built-in class \code{Future} is associated with the invocation and immediately returned to the caller. The body of $f$ is scheduled for execution at some future time. The future $o$ will complete when $f$ terminates. The value used to complete $o$ is the current return value (\ref{return}), if it is defined, and the current exception (\ref{throw}) otherwise. On 2014/08/29 09:08:21, floitsch wrote: > This specification does not deal with zones. > > You should either specify it, or base yourself on the existing Future class (and > not just its interface). > I think we can give ourselves some room, and not specify everything to the last > detail, but there must be some guarantees, like "returns an instance of the > dart:async Future class (as if created by one of its constructors)". This > doesn't prohibit us from creating several of them (which would be visible with > zones!) and or take shortcuts as long as we could replicate the semantics with a > Future construction call. > > Note: if you skip zones you would break the Angular testing framework (assuming > they use async/await). > Note2: Streams actually are relatively simple wrt zones (they run in the zone of > the `listen`). So this is mainly an "async" and not "async*" issue. So to be clear: you want to require that the object returned by an async function is an instance of the built-in class Future. I was inclined to go along with that, but of course that is overspecification as well. What if another implementations defines Future as an interface with factory methods? That is perfectly legitimate. So, as I said below, it all comes down to quality of implementation. If you try to enforce that in the spec, you end up precluding options you haven't thought about.
Responses to Lasse's comments. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:2726: On 2014/09/02 13:00:09, Lasse Reichstein Nielsen wrote: > Side not: > Should we say that if the object *extends* the library class Error, the current > stack trace is set on the object, if a stack trace hasn't already been set in > this way, so that it is returned by the stackTrace getter on the Error class? > > This seems like something that should be mentioned in the spec, and I can't find > it elsewhere. Take a look a few lines down :-) https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:2735: If $f$ is marked \SYNC* then the dynamically enclosing exception handler encloses the call to \code{moveNext()} that initiated the evaluation of the throw expression. On 2014/09/02 13:00:09, Lasse Reichstein Nielsen wrote: > This sounds like "the" dynamically enclosing exception handler is always the one > surrounding the entire moveNext call. Change to "... the a dynamically enclosing > ..."? Done. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:2735: If $f$ is marked \SYNC* then the dynamically enclosing exception handler encloses the call to \code{moveNext()} that initiated the evaluation of the throw expression. On 2014/09/02 13:00:09, Lasse Reichstein Nielsen wrote: > This sounds like "the" dynamically enclosing exception handler is always the one > surrounding the entire moveNext call. Change to "... the a dynamically enclosing > ..."? Done. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:4043: If $e$ evaluates to an instance of \code{Future}, $f$, then execution of the function $m$ immediately enclosing $a$ is suspended until after $f$ completes. The stream associated with the innermost enclosing asynchronous for loop (\ref{asynchronousFor-in}), if any, is paused. At some time after $f$ is completed, control returns to the current invocation. The stream associated with the innermost enclosing asynchronous for loop (\ref{asynchronousFor-in}), if any, is resumed. If $f$ has completed with an exception $x$, $a$ raises $x$. If $f$ completes with a value $v$, $a$ evaluates to $v$. On 2014/09/02 13:00:09, Lasse Reichstein Nielsen wrote: > When raising $x$, it should also set the current stack trace to the stack trace > from the future. Love that sentence. What do want: Time Travel. When do we want it: Whenever. Seriously, the future's stack trace comes from a different place entirely. While one would love to have that trace, one would also want to have the currently active one. What I think you want to say is that when we raise x, we use the stack trace that came from the future's completion. Which is implicit here already because we will use the value x that completed the future. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:4047: It is a compile-time error if the function immediately enclosing $a$ is not declared asynchronous. On 2014/09/02 13:00:09, Lasse Reichstein Nielsen wrote: > If the function is not asynchronous, then it would not be parsed as an await > expression, and await isn't a reserved word there. > > The expression "await(42)" is a valid function call in a non-async function. The > expression "await 42" is a normal syntax error. Yes, I guess this is redundant. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:4238: It is a compile-time error if any of the identifiers \ASYNC, \AWAIT{} or \YIELD{} is used as an identifier in a function body marked with either \ASYNC{}, \ASYNC* or \SYNC*. On 2014/09/02 13:00:09, Lasse Reichstein Nielsen wrote: > Why is async restricted? I don't think we use it in syntax inside a function > body, only as a marker before the body. It's restricted because we can. It makes things simpler. Ideally it would be reserved word and we'd be done. > > How about the parameter names of an async function, can one of them be named > "await"? (I guess so, you just won't be able to refer to it?) Yes. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:4957: An \ON{}-\CATCH{} clause of the form \code{\ON{} $T$ \CATCH{} ($p_1, p_2$) $s$} introduces a new scope $CS$ in which local variables specified by $p_1$ and $p_2$ are defined. The statement $s$ is enclosed within $CS$. The static type of $p_1$ is $T$ and the static type of $p_2$ is \code{StackTrace}. On 2014/09/02 13:00:09, Lasse Reichstein Nielsen wrote: > The local variables should be final. The RETHROW description above assumes that > p1, p2 refers to the exception and stacktrace, which might not be the case if > you can assign to p1 or p2 before rethrowing. > > The VM implements the variables as final, dart2js has them modifiable (but still > rethrows the original values), the analyzer does not complain on assignment. Yes, I keep meaning to make that change. Done. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:4968: The {\em active stack trace} is an object whose \code{toString()} method produces a string that is a record of exactly those function activations within the current isolate that had not completed execution at the point where the current exception (\ref{throw}) was thrown. On 2014/09/02 13:00:08, Lasse Reichstein Nielsen wrote: > I find it a little confusing that the current stack trace is defined at the > catch, not the at the throw, and it means that the same explanation is needed > where async code handles throws. Well, it could be different, but it is defined where it is either it manifests itself to the user. > > And if we specify that Error.stackTrace is set when thrown, it might be easier > if it is handled at the throw, not at the catch (but it might also be just the > same). So the special treatment of Error is given at the end of 16.9. It ha sbeen there for quite a while, and I think that is all that is needed. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:5050: The {\em return statement} returns a result to the caller of a synchronous function, completes the future associated with an asynchronous function or closes the stream associated with an asynchronous generator (\ref{functions}). On 2014/09/02 13:00:09, Lasse Reichstein Nielsen wrote: > Should also address a synchronous generator - the return makes the current (and > all future) calls to moveNext on the generated Iterator return false, and sets > its "current" to null. This is an executive summary, so I'd just say "terminates the stream or iterable associated with a generator". https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:5060: Due to \FINALLY{} clauses, the precise behavior of \RETURN{} is a little more involved. Whether the value a return statement is supposed to return is actually returned depends on the behavior of any \FINALLY{} clauses in effect when executing the return. A \FINALLY{} clause may choose to return another value, or throw an exception, or even redirect control flow leading to other returns or throws. All a return statement really does is set a value that is intended to be returned when the function terminates. On 2014/09/02 13:00:09, Lasse Reichstein Nielsen wrote: > It's a little harsh on "return" to say that that is *all* it does :) > It also starts propagating through any finally blocks in order to terminate the > function. It's just a comment. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:5078: In the simplest case, the immediately enclosing function is an ordinary, synchronous non-generator, and upon function termination, the current return value is given to the caller. The other possibility is that the function is marked \ASYNC{}, in which case the current return value is used to complete the future associated with the function. Both these scenarios are specified in section \ref{functionInvocation}. On 2014/09/02 13:00:09, Lasse Reichstein Nielsen wrote: > -> ... associated with the function *invocation*. Done. https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:5078: In the simplest case, the immediately enclosing function is an ordinary, synchronous non-generator, and upon function termination, the current return value is given to the caller. The other possibility is that the function is marked \ASYNC{}, in which case the current return value is used to complete the future associated with the function. Both these scenarios are specified in section \ref{functionInvocation}. On 2014/09/02 13:00:09, Lasse Reichstein Nielsen wrote: > -> ... associated with the function *invocation*. Done.
https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/498873003/diff/40001/docs/language/dartLangSp... docs/language/dartLangSpec.tex:3116: If $f$ is marked \ASYNC{} (\ref{functions}), then a fresh instance (\ref{generativeConstructors}) $o$ implementing the built-in class \code{Future} is associated with the invocation and immediately returned to the caller. The body of $f$ is scheduled for execution at some future time. The future $o$ will complete when $f$ terminates. The value used to complete $o$ is the current return value (\ref{return}), if it is defined, and the current exception (\ref{throw}) otherwise. On 2014/09/03 17:40:24, gbracha wrote: > On 2014/08/29 09:08:21, floitsch wrote: > > You must not define something as "schedule for execution at some future time". > > There is no way you will get away with that. > > That's what they tell bank robbers and kidnappers. I hope it wasn't interpreted in an offensive way. Not my intent. > > I could say that it will be scheduled by the Future class. However I looked at > the constructor Future() and it says nothing at all about when the future it > creates will be scheduled. Is there somewhere in the dart doc I've missed that > actually specifies that? It could use improvements... What it does say, is that it goes through `Timer.run`. > > Besides, it is quite acceptable to view this as a quality of implementation > issue. After all, if you have a crappy scheduler, the system will be unusable > anyway. Some Dart implementations won't run in a browser. There is no need to > overspecify things. If you don't specify it, you scare the devs that actually read the doc, while still enforcing a de facto standard, since it would be absolutely impossible to implement it otherwise without breaking programs. I would consider a change from microtask to Timer a bug and not a quality of implementation issue. Microtask vs Timer is independent of the browser, and exists in the VM too. > > > Applications must know when functions are executed. Otherwise there could be a > > DOM redraw in the middle. > > If the "some future time" was really implemented as `new Timer` the whole > > proposal would be unusable in browsers for UI work. > > > > This is another reason for doing the async transformation as 'Future.sync'. > This > > way you don't have to deal with when code is executed. >
Having discussed this extensively with Lars, we have decided that the semantics are as designed and this needs to move ahead so the final version is ready for TC52. So I'll be committing this now. Feel free, as always, to point out inconsistencies etc.
Message was sent while issue was closed.
Committed patchset #6 (id:100001) manually as r40052 (presubmit successful).
Message was sent while issue was closed.
On 2014/09/09 20:51:11, gbracha wrote: > Having discussed this extensively with Lars, we have decided that the semantics > are as designed and this needs to move ahead so the final version is ready for > TC52. Is this referring to sync-until-first-await versus async-from-top-of-body? - bob
Message was sent while issue was closed.
https://codereview.chromium.org/498873003/diff/100001/docs/language/dartLangS... File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/498873003/diff/100001/docs/language/dartLangS... docs/language/dartLangSpec.tex:5240: The current call to \code{moveNext()} returns \TRUE. Just noticed that the use of the value is missing, e.g.: \item The \code{current} getter on the iterator returns $o$ until \code{moveNext()} is next invoked. https://codereview.chromium.org/498873003/diff/100001/docs/language/dartLangS... docs/language/dartLangSpec.tex:5267: 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$: Still not sure we want to require the types in unchecked mode. As long as you can use .iterator on the sync value and .listen on the async one, it could still work. https://codereview.chromium.org/498873003/diff/100001/docs/language/dartLangS... docs/language/dartLangSpec.tex:5270: 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. If $u$ is paused, then the subscription on $o$ should probably also be paused until $u$ resumes. https://codereview.chromium.org/498873003/diff/100001/docs/language/dartLangS... docs/language/dartLangSpec.tex:5283: \end{itemize} and `current` returns $x$ until the next call to `moveNext()`. |
