fixes #221, initial sync*, async, async* implementation

fixes #221, initial sync*, async, async* implementation
also fixes #270

R=vsm@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/5e0e7d27c36a8d7d6858ddf3b7a66892de6539ae

[Jennifer Messerly, 2015-07-24 23:56:12]

Patchset #1 (id:1) has been deleted

[Jennifer Messerly, 2015-07-24 23:56:21]

Patchset #1 (id:20001) has been deleted

[Jennifer Messerly, 2015-07-24 23:56:32]

Patchset #1 (id:40001) has been deleted

[Jennifer Messerly, 2015-07-24 23:58:59]

jmesserly@google.com changed reviewers:
+ vsm@google.com

[Jennifer Messerly, 2015-07-25 00:01:41]

Hi Vijay, here's initial support for sync*/async/async*, as well as
yield/yield*/await/await for.

It's passing pretty much all of the tests I pulled over ... a few are skipped
due to unrelated DDC bugs, a few are skipped because dart2js/vm also skip them.

Main inspiration for async* and await for desugaring was Dart VM, so it might be
helpful to pull up parser.cc and core_patch.dart when reviewing those.

Thanks in advance!

[Jennifer Messerly, 2015-07-27 16:32:44]

On 2015/07/25 00:01:41, John Messerly wrote:
> Hi Vijay, here's initial support for sync*/async/async*, as well as
> yield/yield*/await/await for.
> 
> It's passing pretty much all of the tests I pulled over ... a few are skipped
> due to unrelated DDC bugs, a few are skipped because dart2js/vm also skip
them.
> 
> Main inspiration for async* and await for desugaring was Dart VM, so it might
be
> helpful to pull up parser.cc and core_patch.dart when reviewing those.
> 
> Thanks in advance!

fyi, I'm thinking of moving some stuff around:

* codegen_test/testing.dart -> revert changes. Instead add a "build tests"
script.
* test/codegen/language -> test/language
* test/codegen/expect/language (this is the output folder, not checked in) ->
maybe test/out/language ?

could be in a separate CL after this though

[vsm, 2015-07-27 21:03:24]

lgtm

https://codereview.chromium.org/1243503007/diff/70001/karma.conf.js
File karma.conf.js (right):

https://codereview.chromium.org/1243503007/diff/70001/karma.conf.js#newcode29
karma.conf.js:29: 'test/codegen/expect/language/*.js',
:-)

https://codereview.chromium.org/1243503007/diff/70001/lib/runtime/_generators.js
File lib/runtime/_generators.js (right):

https://codereview.chromium.org/1243503007/diff/70001/lib/runtime/_generators...
lib/runtime/_generators.js:32: class _SyncIterator {
Should this "implement" Dart's Iterator and/or subclass Object?

https://codereview.chromium.org/1243503007/diff/70001/lib/runtime/_generators...
lib/runtime/_generators.js:209: // Adds element to steam, returns true if the
caller should terminate
steam -> stream

https://codereview.chromium.org/1243503007/diff/70001/lib/src/codegen/js_code...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1243503007/diff/70001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:2512: visitForEachStatement(ForEachStatement
node) {
Return "JS.Statement"?

https://codereview.chromium.org/1243503007/diff/70001/pubspec.yaml
File pubspec.yaml (right):

https://codereview.chromium.org/1243503007/diff/70001/pubspec.yaml#newcode35
pubspec.yaml:35: analyzer: { path: ../dart/sdk/pkg/analyzer }
Why do we need this?

https://codereview.chromium.org/1243503007/diff/70001/test/browser/language_t...
File test/browser/language_tests.js (right):

https://codereview.chromium.org/1243503007/diff/70001/test/browser/language_t...
test/browser/language_tests.js:29:
dart_library.import('language/async_await_test').main();
Is there a reason this isn't in the suite below?

https://codereview.chromium.org/1243503007/diff/70001/test/browser/language_t...
test/browser/language_tests.js:56:
dart_library.import('language/async_star_test').main();
ditto?

https://codereview.chromium.org/1243503007/diff/70001/test/testing.dart
File test/testing.dart (right):

https://codereview.chromium.org/1243503007/diff/70001/test/testing.dart#newco...
test/testing.dart:39: 'package:dom/dom.dart': _testCodegenPath('sunflower',
'dom.dart')
Move out of sunflower?  And perhaps all of these into a custom dir?

[Jennifer Messerly, 2015-07-27 21:10:57]

Good catches! Taking a look at them. Wanted to comment on the pubspec situation:

https://codereview.chromium.org/1243503007/diff/70001/pubspec.yaml
File pubspec.yaml (right):

https://codereview.chromium.org/1243503007/diff/70001/pubspec.yaml#newcode35
pubspec.yaml:35: analyzer: { path: ../dart/sdk/pkg/analyzer }
On 2015/07/27 21:03:24, vsm wrote:
> Why do we need this?

yeah, I plan on reverting this, unfortunately
https://pub.dartlang.org/packages/analyzer wasn't published at the time. Am
still waiting on dart_style though:
https://github.com/dart-lang/dart_style/pull/392

[Jennifer Messerly, 2015-07-27 21:46:35]

Thanks! Moved SyncItera(ble|tor) to _js_helper and Dart. Otherwise, am planning
to land as soon as I can get the pubspec dependencies sorted out ...

https://codereview.chromium.org/1243503007/diff/70001/lib/runtime/_generators.js
File lib/runtime/_generators.js (right):

https://codereview.chromium.org/1243503007/diff/70001/lib/runtime/_generators...
lib/runtime/_generators.js:32: class _SyncIterator {
On 2015/07/27 21:03:24, vsm wrote:
> Should this "implement" Dart's Iterator and/or subclass Object?

Yeah, I think so, for soundness. It's hard to replicate everything the compiler
does, especially as we add more runtime metadata and such.

Maybe it's best to pull this into a .dart file at this point. And _SyncIterable
too... anyway. DONE :D

https://codereview.chromium.org/1243503007/diff/70001/lib/runtime/_generators...
lib/runtime/_generators.js:209: // Adds element to steam, returns true if the
caller should terminate
On 2015/07/27 21:03:24, vsm wrote:
> steam -> stream

Stand back, I'm doing chemistry!

In all seriousness ... typo was from VM code ... fixed it here at least. I'm not
personally going to send 1 char CL to them but feel free. :)

https://codereview.chromium.org/1243503007/diff/70001/lib/src/codegen/js_code...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1243503007/diff/70001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:2512: visitForEachStatement(ForEachStatement
node) {
On 2015/07/27 21:03:24, vsm wrote:
> Return "JS.Statement"?

Done.

https://codereview.chromium.org/1243503007/diff/70001/test/browser/language_t...
File test/browser/language_tests.js (right):

https://codereview.chromium.org/1243503007/diff/70001/test/browser/language_t...
test/browser/language_tests.js:29:
dart_library.import('language/async_await_test').main();
On 2015/07/27 21:03:24, vsm wrote:
> Is there a reason this isn't in the suite below?

I don't think you can nest `suite`s in the version of mocha that we have. I
tried and it seemed to cause problems. Newer ones use `describe` and those can
nested, but I guess we don't have that version, because describe wasn't
available.

https://codereview.chromium.org/1243503007/diff/70001/test/browser/language_t...
test/browser/language_tests.js:56:
dart_library.import('language/async_star_test').main();
On 2015/07/27 21:03:24, vsm wrote:
> ditto?

same answer -- it didn't work with version(s) that we have

https://codereview.chromium.org/1243503007/diff/70001/test/testing.dart
File test/testing.dart (right):

https://codereview.chromium.org/1243503007/diff/70001/test/testing.dart#newco...
test/testing.dart:39: 'package:dom/dom.dart': _testCodegenPath('sunflower',
'dom.dart')
On 2015/07/27 21:03:24, vsm wrote:
> Move out of sunflower?  And perhaps all of these into a custom dir?

Good ideas, filed https://github.com/dart-lang/dev_compiler/issues/271

[vsm, 2015-07-28 21:01:54]

lgtm!

https://codereview.chromium.org/1243503007/diff/70001/test/browser/language_t...
File test/browser/language_tests.js (right):

https://codereview.chromium.org/1243503007/diff/70001/test/browser/language_t...
test/browser/language_tests.js:29:
dart_library.import('language/async_await_test').main();
On 2015/07/27 21:46:35, John Messerly wrote:
> On 2015/07/27 21:03:24, vsm wrote:
> > Is there a reason this isn't in the suite below?
> 
> I don't think you can nest `suite`s in the version of mocha that we have. I
> tried and it seemed to cause problems. Newer ones use `describe` and those can
> nested, but I guess we don't have that version, because describe wasn't
> available.

Ahh - so the test above already has "groups" inside, so it can't be embedded in
a suite?  Not clear at all from the test names, so I was confused why they're
hoisted out here.

[Jennifer Messerly, 2015-07-28 22:24:19]

On 2015/07/28 21:01:54, vsm wrote:
> lgtm!
> 
>
https://codereview.chromium.org/1243503007/diff/70001/test/browser/language_t...
> File test/browser/language_tests.js (right):
> 
>
https://codereview.chromium.org/1243503007/diff/70001/test/browser/language_t...
> test/browser/language_tests.js:29:
> dart_library.import('language/async_await_test').main();
> On 2015/07/27 21:46:35, John Messerly wrote:
> > On 2015/07/27 21:03:24, vsm wrote:
> > > Is there a reason this isn't in the suite below?
> > 
> > I don't think you can nest `suite`s in the version of mocha that we have. I
> > tried and it seemed to cause problems. Newer ones use `describe` and those
can
> > nested, but I guess we don't have that version, because describe wasn't
> > available.
> 
> Ahh - so the test above already has "groups" inside, so it can't be embedded
in
> a suite?  Not clear at all from the test names, so I was confused why they're
> hoisted out here.

Yeah, I'll add a comment/todo there. It's worth investigating more at some
point.

[Jennifer Messerly, 2015-07-28 22:59:20]

On 2015/07/28 22:24:19, John Messerly wrote:
> On 2015/07/28 21:01:54, vsm wrote:
> > lgtm!
> > 
> >
>
https://codereview.chromium.org/1243503007/diff/70001/test/browser/language_t...
> > File test/browser/language_tests.js (right):
> > 
> >
>
https://codereview.chromium.org/1243503007/diff/70001/test/browser/language_t...
> > test/browser/language_tests.js:29:
> > dart_library.import('language/async_await_test').main();
> > On 2015/07/27 21:46:35, John Messerly wrote:
> > > On 2015/07/27 21:03:24, vsm wrote:
> > > > Is there a reason this isn't in the suite below?
> > > 
> > > I don't think you can nest `suite`s in the version of mocha that we have.
I
> > > tried and it seemed to cause problems. Newer ones use `describe` and those
> can
> > > nested, but I guess we don't have that version, because describe wasn't
> > > available.
> > 
> > Ahh - so the test above already has "groups" inside, so it can't be embedded
> in
> > a suite?  Not clear at all from the test names, so I was confused why
they're
> > hoisted out here.
> 
> Yeah, I'll add a comment/todo there. It's worth investigating more at some
> point.

Update: still haven't figured out how to make this work due to analyzer fix
needed. It was in 0.26.0-alpha.1 of analyzer, but since that's an alpha, no
one's constraints include it. We got dart_style to accept alpha.1 but now I see
package:test has the same issue, and I'm not sure if other packages do as well.
Not sure if analyzer fixes can be backported. I might see if I can work around
missing resolution info when compiling AwaitExpressions...

[Jennifer Messerly, 2015-07-28 23:49:53]

okay figured it out. Added a copy of the staticType to _AstCloner ... with that,
it should be good to land using older analyzer versions :)

[Jennifer Messerly, 2015-07-28 23:52:27]

Committed patchset #4 (id:110001) manually as
5e0e7d27c36a8d7d6858ddf3b7a66892de6539ae (presubmit successful).