Normative: Allow initializers in ForInStatement heads

[bakkot]

Fixes #260.

Per discussion, this adds a section in Annex B with a production for for ( var BindingIdentifier Initializer in Expression) Statement with its original semantics, in light of the usage statistics collected by V8 and the fact that implementers have had to allow this production.

Edit (July 27, 2016): This comment was slightly wrong; it was only added to non-strict code.

[jugglinmike]

We may also want to amend the line-item in Annex E which describes this production.

13.7: Prior to ECMAScript 2015, an initialization expression could appear as
part of the VariableDeclaration that precedes the in keyword. The value of
that expression was always discarded. In ECMAScript 2015, the ForBinding in
that same position does not allow the occurrence of such an initializer.

[bakkot]

The current note in Annex E is actually wrong; the value was not discarded and was observable if the object had no properties or if the rhs Expression referred to the variable declared in the head.

I'm not sure if Annex E is maintaining a running list of changes between all version, or only changes between the current version and previous versions. If the former, I propose we append "In ECMAScript 2017, this is only disallowed in strict mode code." If the latter, it should instead read

Prior to ECMAScript 2015, an initialization expression could appear as part of the VariableDeclaration that precedes the in keyword. In ECMAScript 2017, in strict-mode code the ForBinding in that same position does not allow the occurrence of such an initializer.

[jugglinmike]

Oh, also: it would be helpful if the section that this extension concerns included a reference, like in Runtime Semantics: PropertyDefinitionEvaluation

[bakkot]

@jugglinmike, this extension augments 8 different sections, mostly in trivial ways (i.e. adding the new production to the list of possible IterationStatement productions) and always strictly additively. Should there be a note in all of them?

[jugglinmike]

It may be sufficient to document the extension in a NOTE for The for-in and for-of Statements. Personally, I think duplicating the note in relevant sections would be preferable, though.

Still a third option would be to write all of this text in the main specification itself, and then introduce an early SyntaxError for it that could be disabled by Annex B. I think there is some precedent for disabled grammar like this; the ArrowParameters supports yield so that a SyntaxError can be unilaterally applied to its presence, strict mode or no.

Whatever the case, my motivation is discoverability. I've been surprised by annex B extensions in the past, and I think an explicit reference from the "mainline" text is helpful in avoiding this (I'm proposing an unrelated one in gh-607). I'm sure @bterlson and @allenwb have a much clearer idea of how this ought to be executed.

[littledan]

I like @jugglinmike 's idea of a NOTE. For tc39/ecma402#84 , the plan is to make a new facility in 402 to write the compatibilty fix in-line, but still marked as normative-optional. Maybe we should consider doing something like that in 262 as well if it works well for that change. This would be a purely editorial and not semantic change.

[jugglinmike]

LabelledStatement has a better example of the "restricted grammar" approach I suggested above:

LabelledItem : FunctionDeclaration

• It is a Syntax Error if any source text matches this rule.

NOTE An alternative definition for this rule is provided in B.3.2.

[bakkot]

That seems reasonable, though I find it a little odd to give static and runtime semantics in the main text for a production which is forbidden unless Annex B is applied. This is how labelled function declarations are done.

I'm going to hold off rewriting it pending more feedback. It'll be fairly straightforward either way.

[bterlson]

I am not sure what the intent of Annex E is either. I think there is value in keeping a running list, though, so unless there are objections, let's go with that! Should update the Annex E copy to be more accurate. @bakkot I like your copy, want to add it to this PR?

In terms of whether to do a note in all the relevant sections, write the spec text inline, or use the nascent 402 convention for normative-optional, I'm ok with just doing the notes (in all the relevant places). I don't mind the labeled function declaration convention for small things, but I'm not much a fan of adding a ton of semantics that aren't used unless an annex B extension is in play. @bakkot want to just add the notes? Then this should be good to go.

[bakkot]

@bterlson, I've added notes to all affected sections and updated the note in Annex E (with a slightly different wording than proposed above).

[bakkot]

We can maybe get away with doing this only in sloppy mode, since that's what Chrome's currently shipping.

[concavelenz]

I'm never really seen the need to drop it at all. It seems like a lot of
effort over nothing.

On Jul 22, 2016 5:54 PM, "Kevin Gibbons" [email protected] wrote:

We can maybe get away with doing this only in sloppy mode, since that's
what Chrome's currently shipping.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#614 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABMDKkLxwu6pO3J0zE893OfU9Bhz9MoUks5qYWZEgaJpZM4I5-2I
.

[bterlson]

We have consensus for this, modulo: should only be allowed in strict mode, needs test262 collateral.

[bakkot]

I've added tests. The spec text as written only adds the new production in non-strict code (although I invite people who have written more spec text to review that).

[saambarati]

I haven't looked at the spec text (so maybe this is in there), but want to emphasize that we should also disallow let/const and destructuring for var. That is, we should only allow
for (var {ident} = {expr} in ...)

[saambarati]

I think we can get away with this restriction because this has been JSC's behavior for a long time.

[bakkot]

saambarati, that is indeed the case - see slides.

[leobalter]

The tests are merged.