Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No way to do ES6 'for .. of' loops for generators? #3832

Closed
jagill opened this issue Feb 5, 2015 · 67 comments
Closed

No way to do ES6 'for .. of' loops for generators? #3832

jagill opened this issue Feb 5, 2015 · 67 comments
Labels

Comments

@jagill
Copy link

jagill commented Feb 5, 2015

Given a generator

gen = ->
  j = 0
  while j < 9
    yield j++
  return

how do we loop through it via the javascript for .. of loops? In javascript, we'd do

for (i of gen()) {
  console.log(i);
}

The coffeescript for i in gen() gives the C-style for (_i = 0; _i < gen().length; i++) loops, and for i of gen() gives for (i in gen()). Is there some way to get for (i of gen()) that I didn't see in the documentation?

@ssboisen
Copy link

ssboisen commented Feb 5, 2015

I came to ask the same question. Having to manually run next and check for done is quite counter to the syntax-goodness that CS normally prides itself of.

@jashkenas
Copy link
Owner

Shoot. That's a major bummer.

@alubbe et. al — any ideas here? Do all of the runtimes that support generators also support ES2015's for of?

@jashkenas jashkenas added the bug label Feb 5, 2015
@alubbe
Copy link
Contributor

alubbe commented Feb 5, 2015

@jashkenas to my knowledge only chrome, firefox and opera support generators and all three support for .. of - so the answer would seem to be yes.

Syntax wise, this will be a bit of a challenge. for all .. of/in? each ... of/in?

@michaelficarra
Copy link
Collaborator

We might have to add a flag for this to compile CoffeeScript for-in to ES6 for-of. I don't see any better way.

@jagill
Copy link
Author

jagill commented Feb 5, 2015

@michaelficarra That sound problematic -- a given script could either loop through generators, or arrays.

Node 0.11 and iojs also support for of.

I'd much rather have a special syntax, like for gen x of myGen().

@michaelficarra
Copy link
Collaborator

@jagill: ES6 for-of works with arrays because they are iterables.

@epidemian
Copy link
Contributor

I think @michaelficarra's suggestion is the most sensible approach.

I personally would prefer releasing a backward incompatible version of CS (2.0, 3.0, what have you) that aligned for in and for of semantics with JS and have a conversion tool that can be run reliably and transform CS code from one version to the other. After all, it's just a deterministic syntax transformation.

I'm not in favour of adding yet another looping construct to CS.

@vendethiel
Copy link
Collaborator

if they're supposed to be used by generators, can't we use something like the currently-invalid from x from y()?

@lydell
Copy link
Collaborator

lydell commented Feb 5, 2015

@vendethiel did you mean for x from y()? I like that.

A current workaround would be something like:

forOf = (gen, fn) ->
  `for (value of gen()) fn(value)`

forOf gen, (i) -> console.log i
@vendethiel
Copy link
Collaborator

Yes, sorry.

@alubbe
Copy link
Contributor

alubbe commented Feb 6, 2015

I dislike creating type-specific loop declarations.
The very power of for ... of lies in looping through all iterables: arrays, (invoked) generators, sets, maps and, interestingly, arguments.

So fundamentally, there are two routes and we'll need @jashkenas to give us a pointer:

  1. Backwards-compatible: will need a new syntax like each ... of
  2. Breaking compatibility: fundamentally rethinking how for ... in and for ... of work in coffee-script. @michaelficarra and @epidemian have offered two ways this could be done
@ssboisen
Copy link

ssboisen commented Feb 6, 2015

Couldn’t the same syntax be used? If the context of the for is a generator it compiles to for of otherwise we compile using the current strategy. We can assume the for of feature is available if users use yield and the required information about weather the context is a generator should also be available?

@Artazor
Copy link
Contributor

Artazor commented Feb 6, 2015

@ssboisen, the problem is that we can't determine the context of the for at the compile time (imagine it comes from a function parameter). So we can't choose the right way to compile. Runtime check will have significant performance hit.

@jashkenas, I'd vote for backward incompatible CS 2.0 (according to semver!)
If we'll introduce a new looping construct (to maintain backward compatibility) then it will replace the original in eventually in everyday usage, and we will have an obsolete syntax: why to use in if we can use ??? for looping over iterables polymorphicaly and cheaply?

Breaking changes will allow introduce new keywords for existing problems as well (I mean await prefix operator for #3813 and #3757)

@jashkenas
Copy link
Owner

why to use in if we can use ??? for looping over iterables polymorphicaly and cheaply?

Ha, ha, suckers! 😉

Never assume that a new feature in JS is going to be "cheap" or smart to use — not when its freshly implemented, and probably not even after many years of use.

Check this out. Run it yourself.

http://jsperf.com/for-in-vs-for-of

@alubbe
Copy link
Contributor

alubbe commented Feb 6, 2015

nice! that is some next level performance if I've ever seen one.

@jashkenas can you help me interpret whether your response means that your are leaning towards a new third syntax or changing CS's loops? :)

@jashkenas
Copy link
Owner

I don't know of a great solution just yet — but I haven't really had a chance to sit down and give it a think. There are, however, ground rules:

  • We're not adding flags to CoffeeScript.
  • It would be better to revert yield and remove generators for now, than to have to add a flag.
  • We're not going to hurt regular array iteration performance to support this new feature.
  • It's undesirable (but perhaps not unavoidable) to introduce a new looping form.
  • Since for of performance is so shitty — maybe we can compile down to something else and still beat it handily?
  • I wish we had considered this wrinkle before merging the yield PR ;(
@alubbe
Copy link
Contributor

alubbe commented Feb 6, 2015

Regarding 2: please please please no! for ... of is a feature independent of generators and should have no impact on whether we keep this new awesome feature around or not. It is a loop over iterables, and an instantiated generator just happens to be iterable.
Regarding 6: (see 2) Also, it's awesome because finally more people are joining the discussion, reporting bugs and actually writing better code than before.

@alubbe
Copy link
Contributor

alubbe commented Feb 6, 2015

Here is more information on what for ... of does: https://developer.mozilla.org/en/docs/Web/JavaScript/Guide/The_Iterator_protocol

Basically, it converts strings, arrays, maps, etc. into a generator, instantiates it and continually calls .next() on it. I assume that is where all of the performance is lost.

Looking at it, I think CS could go without for ... of for the time being. It's quite slow and, if you really ever need for x of y, you can use

_ref = y[Symbol.iterator]()
while _next = _ref.next(), val = _next.value, !_next.done
  ...
@jashkenas
Copy link
Owner

That's sort of the kind of thing I'm talking about ... Is a direct:

_ref = y[Symbol.iterator]()
while x = _ref.next()
  ...

... a lot faster than a for of call? If it is — should we have a construct or a looping hint that compiles into it?

@alubbe
Copy link
Contributor

alubbe commented Feb 6, 2015

No, it is just as slow: http://jsperf.com/for-in-vs-for-of/2

@vendethiel
Copy link
Collaborator

@jashkenas you don't want "for x from y" as a third syntax then?

@jashkenas
Copy link
Owner

Apropos of nothing — what the hell is up with the:

generator = array[Symbol.iterator]()

... bullshit? Are we trying to pretend like we're not in a dynamic language that embraces duck typing any more? On what planet would that be preferable to:

generator = array.iterator()

Maybe for ES2019, we'll need a new third type of strings, after strings and symbols, to avoid name clashes again, so that we can have:

newFancyIterator = array[Symbol2019.iterator]()

</grump>

@jagill
Copy link
Author

jagill commented Feb 6, 2015

One of the things I've liked about CoffeeScript is that it allows me to do anything I could do in Javascript, modulo a couple bad constructs. Assuming we don't think the for of loop for generators is a bad construct, IMO CS should support it (or an equivalent), at least someday. It also looks like there's no possibility any time soon to use the same syntax for normal array loops and for of. So I don't see an alternative to a third syntax.

That being said, I have less absolute feelings about when it happens. For my own CS use (since iojs and "soon" Node 0.12 support generators), I'd prefer it to be sooner rather than later.

@epidemian
Copy link
Contributor

Apropos of nothing — what the hell is up with the:

generator = arraySymbol.iterator

... bullshit? Are we trying to pretend like we're not in a dynamic language that embraces duck typing any more

Yeah, it's lamentable. But the philosophy of never breaking backwards compatibility, nor breaking existing applications, plus the common practice of extending native objects, makes it impossible to extend the language in a sane way like adding a normal iterator method to arrays.

Hell, not even something as simple as Array#contains can be added to the standard library because some library monkey-patched it and there are tons of applications relying on it being something else by now.

@devongovett
Copy link

I think for x from gen (or another syntax) is the only way that really makes sense here, unfortunately. Otherwise we basically have to either know the type in the compiler (not currently possible), or do a runtime check to see if its an object or iterator, which will be slow.

@lydell
Copy link
Collaborator

lydell commented Feb 10, 2015

ES5 has two for loops. So does CS. CS for-in is ES5 for with shorter syntax. CS for-of is ES5 for-in. ES6 added a third for loop. Why can't CS too? CS for-from would be ES6 for-of.

@jashkenas
Copy link
Owner

But wouldn't it just be super tragic for:

CoffeeScript ES6
for-in for
for-of for-in
for-from for-of

... especially when, IIRC, ES6's for-of was partially a syntax "cowpaved path" borrowed from CoffeeScript?

It's even more tragic when you consider that in a perfect language, you wouldn't have three different syntaxes for these loops — you would only have one that handled arrays, objects and iterables.

@devongovett
Copy link

Yeah, it sucks and there should definitely be one loop type ideally. But unless you want to implement type-inference in the compiler (hard), or type checks at runtime (slow), we're kinda stuck right?

@edgebal
Copy link

edgebal commented Feb 18, 2015

My two cents: What about adding a keyword to for...of to let the interpreter know it's a generator?

for i of yielded gen()
  code...
  • or -
for i in yielded gen()
  code...

One of those would compile to for (i of gen()) { code... }

Cheers!

@lydell
Copy link
Collaborator

lydell commented Jun 21, 2015

@michaelficarra Thanks for explaining!

@bernhard-42
Copy link

@michaelficarra So the only way to access elements of a generator in coffescript right now is to iterate over it using .next(), e.g.

iterator = squares()
until ((it = iterator.next()).done)
  doSomething it.value

No for loops and no deconstruction, right?

@michaelficarra
Copy link
Collaborator

@bernhard-42 As of right now, yes.

@bernhard-42
Copy link

@igl
Copy link

igl commented Jun 24, 2015

Bad performance is not specific to the for of feature. All es6 features are not optimized and v8 will just bail out to the interpreter. Afaik it does that on functions including a try-catch too. ES5 features also took a long time to be optimized by V8.

Therefor the performance issue is less relevant than generally thinking about how breaking changes will be introduced into coffee in the future. TC39 is just starting and es7 and es8 are not too far off... They will certainly not stop advancing the language now like they did 15 years ago and if coffee wants to stay relevant it has to move on too. Hack or Break(+1)?

@JHawkley
Copy link

I tend to think the optimal syntax for using iterators/generators would be for ... using since it just sounds right and suggests the iterated object carries special meaning (IE: for n using fibonacci()), but that would introduce a new keyword. The next best thing would have to be for ... with since the with keyword is JavaScript reserved and is not used in CoffeeScript at all.

Neither of these syntax suggestions would interfere with legacy CoffeeScript code either.

This is all assuming you want to keep iterator syntax separate from array (for ... in) and object (for ... of) iteration syntax. It would seem to me to be a good idea to do so, as long as CoffeeScript is maintaining ES5 compatibility, so as not to require weird conditionals at every for ... of block to figure out what the nature of the object being iterated is.

Plus, ES5 could take advantage of iterators too; so long as the object being used follows the iterator protocol, a structure not unlike what was suggested by @bernhard-42 could be used to run through it.

@bernhard-42
Copy link

Unfortunately for standard arrays in the current nodejs, iojs, Chrome and Firefox typeof arr[Symbol.iterator] is also "function". The performance of for of for standard arrays is really different across all platforms - from poor to good:

                                  for(;;)  while()  for of  iterator via while
nodejs 0.12.5                         4       4      363         367
iojs v2.3.1                           5       4       93          64
Firefox 38.0.5                      160     154       59         255
Chrome 43.0.2357.130 (64-bit) Mac   306     325      125         694

(see https://gist.github.com/bernhard-42/27836f4ce719de6bee3e)

For the nodejs world I would really not like to see ES6 for...of used for standard arrays for the time being. The penalty is just too big. Any conditional around for...of needs to take care to omit arrays, e.g.

if (!Array.isArray(ref) && (typeof Symbol != "undefined") && (typeof ref[Symbol.iterator] == "function"))

From a performance perspective not critical, but ugly ...

@fa7ad
Copy link

fa7ad commented Jul 20, 2015

I'm probably not qualified enough to be anpart of thus discussion but here's my 2 cents

What if we had a special shebang-esque comment to put CS in ES2015 mode?

Something like
#! es2k15

What I'm proposing is everything by default compiles to ES5 (discount the es6 features already implemented) but when the shebang-thing is present, things like for..of are compiled assuming ES2015 (compiled to for..of rather than a es5 for..in).

This -i think- would not break backwards compatibility and allow the es2015 users to use the parts they love about es2015 in CS without the need for new keywords, etc.

@fa7ad
Copy link

fa7ad commented Jul 20, 2015

*anpart = a part
(Typo)

@nilskp
Copy link

nilskp commented Dec 15, 2015

Bikeshedding here, but what about this (no new keyword):

for yield value in squares()
  alert value
@dyoder
Copy link

dyoder commented Jan 20, 2016

@nilskp I don't think would work, because that's already a valid expression for a function returning an array. You can't assume that it's returning an iterator.

@nilskp
Copy link

nilskp commented Jan 20, 2016

@dyoder is it valid syntax? I get Error on line 1: unexpected yield.

@babakness
Copy link

Coffeescript 2.0 should break backward compatibility and compile to ES6 & ES7 or become itself "backwards" from an era progressively ever behind us.

@dyoder
Copy link

dyoder commented Jan 25, 2016

@nilskp I stand corrected. :)

My brain parsed that as an expression and move right along. But of course that's an assignment so an expression isn't valid. So what you're suggesting is basically a for yield construct?

My only concern with that is semantic. Iterators aren't related to yield, which is associated with Generators. Generators, of course, are Iterable, but so are arrays and so forth.

@nilskp
Copy link

nilskp commented Jan 25, 2016

@dyoder My proposal was for generators specifically, which was the original subject. I haven't thought about Iterators in general.

@blitmap
Copy link

blitmap commented Mar 8, 2016

Maybe:

for next x in generator()
  console.log x

I'm not sure anyone here wants a new keyword, but I'd prefer this over for yield x... since it makes it very obvious we're going by the generated next value and not yielding something.

Considering that generators in Coffeescript are defined in the same way regular functions are: It seems like there's an effort to make creating and using generators as transparent as possible. Iterating over a generated iterator is done in an each()-like way, so it might make sense to have the identical syntax:

for x in generator()
    console.log x

Coffeescript could reference a utility function to detect if it should iterate by .length or by .done. Afaik in ES6 arrays have a defined iterator anyway. ([Symbol.iterator])

eachIter = (a, f) -> (isArray(a) and walkArray or walkIter) a, f

walkArray and walkIter would do what you expect. It'd be important to separate it into 2 different code paths so you're not testing whether to treat the 'array-like' as an array or an iterator on each iteration. (performance concern?)

@samuelhorwitz
Copy link

I'd like to voice support for for...from. I thought of this independently the other day and saw it way up the thread. There is already precedent for CoffeeScript to deviate from standard loop names and improve them.

Javascript had for (var key in obj). "For key in object" doesn't even make semantic sense compared to CoffeeScript's replacement: for key, val of obj. It does exactly what one would expect and "for key/value pair of object" makes way more sense.

Then, using for val in arr completely altering the native Javascript meaning of for...in was also was an improvement. "For value in array". Or optionally, for val, idx in arr also read as "for value and index in array". Both these read way better semantically. Finally, CoffeeScript exposes for key of obj which is actually full circle back to the only original native Javascript possibility: for (var key in obj).

So basically for...in and for...of were added to CoffeeScript to completely supersede the sad version of for...in that Javascript supported, even with completely disparate syntax.

Now Javascript finally has for...of and they choose to make it not even support proper enumeration of objects but just iterators. It's perfectly in line with previous decisions made by the CoffeeScript team to say "this syntax is garbage and we will make our own".

tl;dr
Just like CoffeeScript's for...in and for...of make semantic sense in use, for...from makes semantic sense for an iterator or generator. You are saying "for every value I take from this iterator". When looping over an array you are taking every value in it. When looping over a collection of key/value pairs you taking every key and value of it. And when looping over an iterator that yields a new result each time you are taking values from it.

@atg
Copy link

atg commented Sep 2, 2016

I'm totally convinced by the rationale for for ... from. Having to state awareness the loop may (or may not) empty the thing being looped over, is definitely a feature.

I can't tell you the number of times I've accidentally tried to loop over the same generator twice in Python. Of course the second loop never executes because the generator is now empty! Having for...from to annotate which of my loops consciously support generators would be very useful.

atg added a commit to atg/coffeescript that referenced this issue Sep 12, 2016
GeoffreyBooth added a commit that referenced this issue Nov 8, 2016
* Added support for for-from loop, see #3832

* for-from: remove extra newline and add support for ranges

* for-from: tidy up the lexer

* for-from: add support for patterns

* for-from: fix bad alignment

* for-from: add two more tests

* for-from: fix test "for-from loops over generators"

See explanation here: #4306 (comment)

* for-from: delete leftover console.log

* Refactor the big `if` block in the lexer to be as minimal a change from `master` as we can get away with

* Cleanup to make more idiomatic, remove trailing whitespace, minor performance improvements

* for-from: move code from one file to another

* for-from: clean up whitespace

* for-from: lexer bikeshedding

* Move "own is not supported in for-from loops" test into error_messages.coffee; improve error message so that "own" is underlined

* Revert unnecessary changes, to minimize the lines of code modified by this PR
@GeoffreyBooth
Copy link
Collaborator

This has been merged into master per #4355. Anyone up for writing some documentation?

@joeytwiddle
Copy link

Great stuff! ✨

in a perfect language, you wouldn't have three different syntaxes for these loops — you would only have one

That is still an option. A universal for .. within .. loop that accepts the performance hit and determines the type at runtime?

@vendethiel
Copy link
Collaborator

That is still an option. A universal for .. within .. loop that accepts the performance hit and determines the type at runtime?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment