923160 - Disallow initializers in for-of statements

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andy Wingo [:wingo]]

http://people.mozilla.org/~jorendorff/es6-draft.html#sec-for-in-and-for-of-statements-static-semantics-early-errors

Currently this "works":

  for (var x=10 of []) {}

Patch forthcoming.

Comment 1

[Andy Wingo [:wingo]]

Attachment: Disallow initializers in for-of statements

Comment 3

[Andy Wingo [:wingo]]

Comment on attachment 813145 [details] [diff] [review]
Disallow initializers in for-of statements

This patch started as a cleanup, then I found this bug.  It does a few things:

 (1) Adds PNK_FOROF.
 (2) Removes JSITER_FOR_OF.  Instead, the C++ interface is ForOfIterator.
 (3) Removes initalizer support in for-of.

Comment 4

[André Bargull [:anba]]

You may be interested in: bug 901987, bug 921551 ;-)

Comment 5

[Jason Orendorff [:jorendorff]]

Comment on attachment 813145 [details] [diff] [review]
Disallow initializers in for-of statements

Review of attachment 813145 [details] [diff] [review]:
-----------------------------------------------------------------

Great, r=me with these minor comments addressed.

Sorry for the slow review here.

::: js/src/frontend/ParseNode.h
@@ +268,5 @@
> + *                            bit set
> + *                          pn_kid2: PNK_NAME or destructuring expr
> + *                            to left of 'of'; if pn_kid1, then this
> + *                            is a clone of pn_kid1->pn_head
> + *                          pn_kid3: object expr to right of 'of'

Please delete the word "object" from the comment about pn_kid3; the RHS of for-in or for-of doesn't have to be an object.

::: js/src/frontend/Parser.cpp
@@ +3984,5 @@
>      StmtInfoPC letStmt(context); /* used if blockObj != nullptr. */
>      ParseNode *pn2, *pn3;      /* forHead->pn_kid2 and pn_kid3. */
>      bool isForOf;
>      bool isForInOrOf = pn1 && matchInOrOf(&isForOf);
> +    ParseNodeKind headKind;

How about this instead, eagerly:

    ParseNodeKind headKind = PNK_FORHEAD;
    if (pn1) {
        bool isForOf;
        if (matchInOrOf(&isForOf))
            headKind = isForOf ? PNK_FOROF : PNK_FORIN;
    }

@@ +3989,1 @@
>      if (isForInOrOf) {

Then this would be
    if (headKind == PNK_FORIN || headKind == PNK_FOROF)
and so on.

@@ +5937,5 @@
>          if (!matchInOrOf(&isForOf)) {
>              report(ParseError, false, null(), JSMSG_IN_AFTER_FOR_NAME);
>              return null();
>          }
> +        ParseNodeKind headKind = PNK_FORIN;

Same here, I think.

::: js/src/jsiter.cpp
@@ +1231,5 @@
> +
> +    if (iterable.isObject())
> +        iterableObj = &iterable.toObject();
> +    else
> +        iterableObj = js_ValueToNonNullObject(cx, iterable);

RootedObject iterableObj(cx, ToObject(iterable));

Comment 6

[Andy Wingo [:wingo]]

Attachment: Disallow initializers in for-of statements v2

Comment 7

[Andy Wingo [:wingo]]

Comment on attachment 817177 [details] [diff] [review]
Disallow initializers in for-of statements v2

r=jorendorff, thanks for the review!

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Had to s/NULL/nullptr in a few spots to get it to apply cleanly.

https://hg.mozilla.org/integration/mozilla-inbound/rev/639936b37901

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/639936b37901