fixes #130, accept anything with `iterator` in for loops

fixes #130, accept anything with `iterator` in for loops

R=vsm@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/65fce02dfd9f8fb59b18d6cf09188c31041ce24a

[Jennifer Messerly, 2015-05-05 00:08:22]

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

[Jennifer Messerly, 2015-05-05 00:08:23]

Iterators were 100% broken before ... this makes them at least work sometimes :)

Now this works from JS console:

    var y = new core.Set.from([1,2,3]);
    for (var x of y) console.log(x);

prints 1, 2, 3 on three lines in the console.

[vsm, 2015-05-05 14:19:28]

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

https://codereview.chromium.org/1126713002/diff/40001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:587: jsMethods.add(_emitIterable(type));
Should we check the return type of iterator here?  We could either:

(a) Only generate a JS iterator if it is Iterable or a subtype.

or

(b) Only generate a JS iterator if the return type has a moveNext and current
with the right signatures.

On Dart for-in loops, we currently require (a), but the Dart spec allows (b).

[vsm, 2015-05-05 14:20:45]

On 2015/05/05 14:19:28, vsm wrote:
>
https://codereview.chromium.org/1126713002/diff/40001/lib/src/codegen/js_code...
> File lib/src/codegen/js_codegen.dart (right):
> 
>
https://codereview.chromium.org/1126713002/diff/40001/lib/src/codegen/js_code...
> lib/src/codegen/js_codegen.dart:587: jsMethods.add(_emitIterable(type));
> Should we check the return type of iterator here?  We could either:
> 
> (a) Only generate a JS iterator if it is Iterable or a subtype.
> 
> or
> 
> (b) Only generate a JS iterator if the return type has a moveNext and current
> with the right signatures.
> 
> On Dart for-in loops, we currently require (a), but the Dart spec allows (b).

LGTM btw.  We can address the above in separate CL if you wish.

[Jennifer Messerly, 2015-05-05 15:38:25]

Thanks!

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

https://codereview.chromium.org/1126713002/diff/40001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:587: jsMethods.add(_emitIterable(type));
On 2015/05/05 14:19:28, vsm wrote:
> Should we check the return type of iterator here?  We could either:

First off, just confirming: most of this is the job of static checker, right?
Dart for-in loops should do the right static checks, and issue errors, e.g. if
my Iterator is missing `moveNext` or `current`. So all statically typed cases
will be fine.

there's a bunch of issues with the dynamic case, all of which existed before
this CL, and span various parts of our checker/reifier/codegen/runtime, so I'd
vote to fix those at some later point in time :). Some of these seem rather
obscure, too.

Anyway, lots more to say on this in
https://github.com/dart-lang/dev_compiler/issues/166


> (a) Only generate a JS iterator if it is Iterable or a subtype.
> 
> or
> 
> (b) Only generate a JS iterator if the return type has a moveNext and current
> with the right signatures.
> 
> On Dart for-in loops, we currently require (a), but the Dart spec allows (b).

[Jennifer Messerly, 2015-05-05 16:06:50]

Committed patchset #3 (id:40001) manually as
65fce02dfd9f8fb59b18d6cf09188c31041ce24a (presubmit successful).