437521 - ES4 array comprehensions

Status: RESOLVED INCOMPLETE

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

Attachment: WIP 1

ES4 will add let clauses to array comprehensions and allows for, if, and let clauses to mix in any order.

The patch implements.  Includes tests.  I didn't fix up the decompiler, though, so it crashes if you go there.

As usual, implementing this new feature raised a question or two.  There isn't a complete proposal for how scoping is supposed work.  In JS1.8, for-in and for-each-in clauses in in array comprehensions and generator expressions don't work *quite* like for-let-in and for-each-let-in statements.  Instead the array comprehension or generator expression introduces a single scope, and all the variables live there.  The difference is visible when some subexpression captures a variable, as in:

  // In JS1.8, all these functions capture the same 'y'
  [function() y for each (x in values) for each (y in [x])]

I kept this behavior, but I think it should be changed.  (In the cases where a variable is *not* captured, it should be possible to optimize away the extraneous scopes.  Haven't tried it though.)

However, I couldn't see my way to having the let keyword *not* introduce a scope, so:

  // With WIP 1 patch, each function captures a different 'y'
  [function() y for each (x in values) let (y=x)]

Comment 1

[Mike Shaver (:shaver emeritus)]

Comment on attachment 323944 [details] [diff] [review]
WIP 1

>      * then we need to generate the special variable opcodes.  We determine
>-     * this by looking up the variable's id in the current variable object.
>+     * this by looking the variable's id in the current variable object.
>      * Fortunately, we can avoid doing this for let declared variables.

Looks like you accidentally deleted the "up" there; maybe you meant to move it somewhere else in the sentence?

Comment 2

[Jason Orendorff [:jorendorff]]

Quick update on this, as I don't have time to work on it at the moment.

Brendan agreed that "for" and "for each" comprehension-clauses should behave like "for (let ...)" and "for each (let ...)" instead of hoisting to a single scope per comprehension, the way they do now.  I've implemented this.

The decompiler is the only remaining obstacle.  Right now the code for comprehensions in case JSOP_ARRAYPUSH is pretty simple, as the SprintStack is pretty constrained at that point.  "let" isn't quite as simple.  My first attempt failed because I used too many SprintStack entries (rookie mistake).

It's not a huge problem--the decompiled let-head just has to be stored *somewhere* such that case JSOP_ARRAYPUSH/YIELD can find it.  All the available SprintStack entries are being used, but maybe I can append to one of them.

Comment 3

[Jan de Mooij [:jandem]]

As discussed with Jason on IRC, here is a much simpler (no let-clauses) version of the patch. This patch is not ready yet, the decompiler needs to be updated. With this patch it crashes on e.g. the following input: 

(function(x) { return [1 if(x)]; })

Comment 4

[Jan de Mooij [:jandem]]

Attachment: WIP 2

Comment 5

[Jeff Walden [:Waldo]]

This bug seems to have outlived its sell-by date.  If comprehensions are ever standardized, we're not going to be working on it in an ES4-denominated bug with irrelevant patches.