For-in inheritance

[pvdz]

The following code does not transform as expected:

var A = function(){
  this.foo = 1;
};
A.prototype.bar = 1;
function *range(max, step) {
  for (var key in new A) yield key;
}

var gen = range(), info;

while (!(info = gen.next()).done) {
  console.log(info.value);
}

Inherited property is not iterated over, though it should be. This only iterates over foo.

Have a look at http://github.com/qfox/unyield

[benjamn]

You're totally right! Check out the Context.prototype.keys method in the runtime—that's what's being used to enumerate keys, and it just uses Object.keys, which only returns own properties.

Would you care to rewrite that method and add a test? I'd really appreciate the contribution (or I can just do it if you're busy).

[benjamn]

That's https://github.com/facebook/regenerator/blob/master/runtime/dev.js to be clear.

[pvdz]

Hmmm, so what I do is first normalize all loops to regular whiles. In the case of a for-in, that means doing an actual for-in and capturing all properties in an array. The loops then just loops over those keys, in the same order. Each iteration an in check is made to be sure that the key wasn't deleted.

I'm not sure what you mean wrt test case, this is a pretty small and concise test?

[benjamn]

Oh, what I mean is to add this regression test to the test/tests.es6.js file.

[benjamn]

The code I generate for for-in loops is similar to a while loop except that it calls context.keys before the loop and then assigns each key to the loop variable on each iteration. Fixing context.keys should be enough to make your test case pass.

[amasad]

@benjamn the point re deletions, it's not only a runtime change. compiler should emit checks after each iterations to handle deleted keys. Similar to length checks there should be:

$ctx.t2 = $ctx.t1.pop();
if (!($ctx.t2 in a){
// etc

[amasad]

try

var foo = {a: 1, b:2};
for (var p in foo) {
  console.log(p);
  delete foo.b
}

will run once for the a prop.

[pvdz]

Ah sorry, it's 2:40am here. I'm gonna get some z's. If you haven't figured it out before then, I'll have a look tomorrow :) But sounds like you got it.

[benjamn]

Fixed by 10c361e, 36bd1c6, and b23a26a.

[pvdz]

nice

[pvdz]

I can't really see it through the esprima syntax, but is the comma edge case taken care of?

for (var key in a,b) {}

In my approach I have to wrap the caching of the rhs to a temporary object or it might introduce a syntax error. It's an edge case, but still :)

[benjamn]

As long as that parses to a sequence expression (and it should, if Esprima is doing its job correctly), the AST->code generator (https://github.com/benjamn/recast) should realize that extra parentheses are necessary.

[benjamn]

@qfox Added some tests for the SequenceExpression case, thanks!
https://github.com/facebook/regenerator/blob/79609e4db8/test/tests.es6.js#L503-L530