User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:21.0) Gecko/20130320 Firefox/21.0 Build ID: 20130320042012 Steps to reproduce: I tried this: https://gist.github.com/getify/5225304 I was hoping we could get this ES6'ish decision about let binding to for-loop iterations added to FF so we can play around and demonstrate the value of that feature. (also, so I stop embarrassing myself during live demos in my JS workshops! ;) Actual results: // 5 // 5 // 5 // 5 // 5 Expected results: according to TC39 sometime in the last year: // 0 // 1 // 2 // 3 // 4

Reopening. This bug is about the scope of C-style for loops using let, which is a separate question from for-of and for-in loops. TC39 shifted in January 2012 to move to a semantics more like Dart, where each loop iteration gets a fresh binding that inherits as its initial value the final value of the previous iteration. See the bottom of: https://mail.mozilla.org/pipermail/es-discuss/2012-January/019784.html The semantics of this is not yet in the draft ES6 spec. There are definitely some subtleties. Dave

bump.

This will be in the Rev 23 ES6 draft. You can see a preview of the for(let;;) spec at https://twitter.com/awbjs/status/433645470431186944/photo/1

Is this still the case? This isn't what I'm understanding from reading http://people.mozilla.org/~jorendorff/es6-draft-rev-22.html#sec-for-statement-runtime-semantics-labelledevaluation

(In reply to Shu-yu Guo [:shu] from comment #6) > Is this still the case? This isn't what I'm understanding from reading > http://people.mozilla.org/~jorendorff/es6-draft-rev-22.html#sec-for- > statement-runtime-semantics-labelledevaluation See comment #5. Changes are not present in the 22nd draft.

(In reply to Shu-yu Guo [:shu] from comment #6) > Is this still the case? This isn't what I'm understanding from reading > http://people.mozilla.org/~jorendorff/es6-draft-rev-22.html#sec-for- > statement-runtime-semantics-labelledevaluation This is the current URL: http://people.mozilla.org/~jorendorff/es6-draft.html#sec-for-statement-runtime-semantics-labelledevaluation

(In reply to Michał Gołębiowski [:m_gol] from comment #8) > (In reply to Shu-yu Guo [:shu] from comment #6) > > Is this still the case? This isn't what I'm understanding from reading > > http://people.mozilla.org/~jorendorff/es6-draft-rev-22.html#sec-for- > > statement-runtime-semantics-labelledevaluation > > This is the current URL: > http://people.mozilla.org/~jorendorff/es6-draft.html#sec-for-statement- > runtime-semantics-labelledevaluation Ack, thanks.

This doesn't change for (let x = x; ; ); scoping to deal with the TDZ. It only handles creating new bindings each iteration. I'm looking into doing the TDZ thing atop this -- it might (I'm not certain) be possible to refactor the current thing into a block with a let-declaration and a modified for-loop in it. More as I learn it.

Comment on attachment 8582823 [details] [diff] [review] Make lexical declarations in the initializing component of a for(;;) loop create a fresh binding for each iteration of the loop Review of attachment 8582823 [details] [diff] [review]: ----------------------------------------------------------------- Certified fresh. My fears for how complicated this was going to be were unfounded. It would be ideal to desugar into a non-binary let block, even as part of this bug. Could that be part 2? ::: js/src/frontend/Parser.cpp @@ +4754,5 @@ > + // assertEq(funcs[0](), 0); > + // assertEq(funcs[1](), 1); > + // > + // These semantics are implemented by "freshening" the implicit block each > + // iteration, before evaluating the "update" in for(;;) loops. (We don't Could use a parenthetical on what "freshening" actually does (clone). @@ +4760,5 @@ > + // No freshening occurs in for (const ...;;) -- there's no point (you can't > + // reassign consts), and the spec requires it (which fact is exposed by the > + // debugger API, if not in-language). > + // > + // Setting aside our inconsistencies with the spec. If the for-loop head "Setting aside out inconsistencies with the spec" doesn't seem like a useful sentence in a comment. @@ +4951,5 @@ > + // for (let INIT; TEST; UPDATE) STMT > + // > + // into > + // > + // let (INIT) { for (; TEST; UPDATE) STMT } I want binary let blocks gone. Would it be difficult, while you're here, to desugar into { let INIT; for (; TEST; UPDATE) STMT } instead? @@ +4962,5 @@ > > forLetDecl = pn1; > + > + // The above transformation isn't enough to implement INIT scoping, > + // because each loop iteration must see separate |INIT|. We handle separate |INIT| -> separate binding of INIT. @@ +4968,5 @@ > + // each iteration. We supply a special PNK_FRESHENBLOCK node as > + // the init node for for(let A;;) loop heads to distinguish such > + // nodes from an *actual*, non-desugared use of the above syntax. > + // (We don't do this for PNK_CONST nodes because the spec says no > + // freshening happens -- and this is observable with the debugger nit: capitalize Debugger ::: js/src/jit/BaselineCompiler.cpp @@ +3003,5 @@ > + > +bool > +BaselineCompiler::emit_JSOP_FRESHENBLOCKSCOPE() > +{ > + // Call a stub to freshen the block on the block chain. Style nit: I'd remove this comment. Code is pretty self-explanatory. ::: js/src/jit/BaselineFrame-inl.h @@ +43,5 @@ > popOffScopeChain(); > } > > +inline void > +BaselineFrame::replaceScopeChain(ScopeObject &scope) Given what it does, perhaps this is clearer as "replaceInnermostScope" @@ +73,5 @@ > > +inline bool > +BaselineFrame::freshenBlock(JSContext *cx) > +{ > + MOZ_ASSERT(scopeChain_->is<ClonedBlockObject>()); Superfluous assert, the as<ClonedBlockObject> call below will assert. ::: js/src/tests/ecma_6/LexicalEnvironment/for-loop.js @@ +44,5 @@ > +isOK("for (let x; ; ) ;"); > +isOK("for (let x = 5, y; ; ) ;"); > +isOK("for (let [z] = [3]; ; ) ;"); // XXX ??? or syntax error? > +isError("for (let [z, z]; ; ) ;", SyntaxError); // XXX or TypeError? > +isError("for (let [z, z] = [0, 1]; ; ) ;", TypeError); // XXX or SyntaxError? What's up with these XXXs? ::: js/src/vm/ScopeObject.cpp @@ +693,5 @@ > + > + copy.setReservedSlot(SCOPE_CHAIN_SLOT, block->getReservedSlot(SCOPE_CHAIN_SLOT)); > + > + for (uint32_t i = 0, count = copy.numVariables(); i < count; i++) > + copy.setSlot(RESERVED_SLOTS + i, block->getSlot(RESERVED_SLOTS + i)); Should use setVar and var here for readability. ::: js/src/vm/ScopeObject.h @@ +672,5 @@ > + /* > + * Create a new ClonedBlockObject with the same enclosing scope and > + * variable values as this. > + */ > + static ClonedBlockObject * clone(ExclusiveContext *cx, Handle<ClonedBlockObject*> block); Nit: extra space after * ::: js/src/vm/Stack-inl.h @@ +207,5 @@ > scopeChain_ = &scopeChain_->as<ScopeObject>().enclosingScope(); > } > > +inline void > +InterpreterFrame::replaceScopeChain(ScopeObject &scope) Ditto on the name. Maybe replaceInnermostScope.

I do want to very quickly follow up converting this to a { let ...; for (; ...; ...) ... } form, yes. Let-expressions complicate this some; the divergent scoping implementations for for(;;) loops versus for-in/of complicate some more. But I do plan to fix all of this in probably the next month -- fixing all the rest of for-loop scoping seems likely to be a quarterly goal if I can swing it.

Backed out because bug 1146644 was backed out and it wasn't clear to me if this depended on that or not and I didn't want to risk an extended tree closure finding out. https://hg.mozilla.org/integration/mozilla-inbound/rev/67f8d63b2cad