Implement await for statement

Implement await for statement

Implement await for statement to iterate over streams. Uses
the built-in class StreamIterator.

await for (var e in stream_expr) {
  S;
}

Is translated to:

var it = new StreamIterator(stream_expr);
while (await it.moveNext()) {
  var e = it.current;
  S;
}

Committed: https://code.google.com/p/dart/source/detail?r=41328

[hausner, 2014-10-16 17:57:41]

hausner@google.com changed reviewers:
+ iposva@chromium.org

[floitsch, 2014-10-16 19:03:55]

floitsch@google.com changed reviewers:
+ floitsch@google.com

[floitsch, 2014-10-16 19:03:56]

DBC:
You probably know this, but eventually there needs to be a try/finally around
the while loop:
var it = new StreamIterator(stream_expr);
try {
  while (await it.moveNext()) {
    var e = it.current;
    S;
  }
} finally {
  await it.cancel();
}

https://codereview.chromium.org/662603003/diff/20001/tests/language/await_for...
File tests/language/await_for_test.dart (right):

https://codereview.chromium.org/662603003/diff/20001/tests/language/await_for...
tests/language/await_for_test.dart:52: Future.wait([f1, f2]).then((_) {
You should use the async-helper package to make sure that your asynchronous
calls are checked correctly. Look, for example, into lib/async/future_test.dart.

https://codereview.chromium.org/662603003/diff/20001/tests/language/await_for...
tests/language/await_for_test.dart:77: timer = new Timer.periodic(const
Duration(milliseconds: 200), tick);
waiting 200 milliseconds for every tick is a lot. The whole test will take more
than a second to complete...

[hausner, 2014-10-16 21:39:28]

Thanks for taking a look, Florian. I suspected that I need a try-finally as
we've discussed previously. I'll add it in a separate change later.

https://codereview.chromium.org/662603003/diff/20001/tests/language/await_for...
File tests/language/await_for_test.dart (right):

https://codereview.chromium.org/662603003/diff/20001/tests/language/await_for...
tests/language/await_for_test.dart:52: Future.wait([f1, f2]).then((_) {
On 2014/10/16 19:03:56, floitsch wrote:
> You should use the async-helper package to make sure that your asynchronous
> calls are checked correctly. Look, for example, into
lib/async/future_test.dart.

I added the asyncStart() and asyncEnd() calls. Not sure what this does, though.
The dart process won't quit until there are no more messages for the isolate to
handle.

https://codereview.chromium.org/662603003/diff/20001/tests/language/await_for...
tests/language/await_for_test.dart:77: timer = new Timer.periodic(const
Duration(milliseconds: 200), tick);
On 2014/10/16 19:03:56, floitsch wrote:
> waiting 200 milliseconds for every tick is a lot. The whole test will take
more
> than a second to complete...

Shortened to 10ms.

[floitsch, 2014-10-17 09:27:30]

https://codereview.chromium.org/662603003/diff/20001/tests/language/await_for...
File tests/language/await_for_test.dart (right):

https://codereview.chromium.org/662603003/diff/20001/tests/language/await_for...
tests/language/await_for_test.dart:52: Future.wait([f1, f2]).then((_) {
On 2014/10/16 21:39:27, hausner wrote:
> On 2014/10/16 19:03:56, floitsch wrote:
> > You should use the async-helper package to make sure that your asynchronous
> > calls are checked correctly. Look, for example, into
> lib/async/future_test.dart.
> 
> I added the asyncStart() and asyncEnd() calls. Not sure what this does,
though.
> The dart process won't quit until there are no more messages for the isolate
to
> handle.

It might not make a difference for the VM (although I'm not sure, in the context
of Dartium).
It will make a difference for dart2js in the browser. Since this test is in the
language directory it should work for all engines.

[Ivan Posva, 2014-10-17 21:49:28]

iposva@google.com changed reviewers:
+ iposva@google.com
- iposva@chromium.org

[hausner, 2014-10-22 18:34:13]

Ping.

[Ivan Posva, 2014-10-27 06:55:24]

LGTMwC

-Ivan

https://codereview.chromium.org/662603003/diff/60001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/662603003/diff/60001/runtime/vm/parser.cc#new...
runtime/vm/parser.cc:7258: loop_var_name = ExpectIdentifier("variable name
expected");
This line can be moved out of the if-else. The if-else is just handling whether
a variable is being "reused" or not.

https://codereview.chromium.org/662603003/diff/60001/runtime/vm/parser.cc#new...
runtime/vm/parser.cc:7267: loop_var_name = ExpectIdentifier("variable name
expected");
loop_var_pos is not being set in this case. Why?

[hausner, 2014-10-27 17:58:25]

Thank you.

https://codereview.chromium.org/662603003/diff/60001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/662603003/diff/60001/runtime/vm/parser.cc#new...
runtime/vm/parser.cc:7258: loop_var_name = ExpectIdentifier("variable name
expected");
On 2014/10/27 06:55:24, Ivan Posva wrote:
> This line can be moved out of the if-else. The if-else is just handling
whether
> a variable is being "reused" or not.

Done.

https://codereview.chromium.org/662603003/diff/60001/runtime/vm/parser.cc#new...
runtime/vm/parser.cc:7267: loop_var_name = ExpectIdentifier("variable name
expected");
On 2014/10/27 06:55:24, Ivan Posva wrote:
> loop_var_pos is not being set in this case. Why?

The position isn't used in this case, but I agree it's better to have a
meaningful value in the variable in all cases. Simplified.

[hausner, 2014-10-27 18:03:50]

Committed patchset #5 (id:80001) manually as r41328 (presubmit successful).