Bug 854037 - Make lexical declarations in the initializing component of a for(;;) loop create a fresh binding for each iteration of the loop. r=shu
authorJeff Walden <jwalden@mit.edu>
Fri, 27 Mar 2015 12:29:50 -0400
changeset 265094 b05e10ed40c4e6569a9bb48e73b732ffb7a13257
parent 265093 7a476e71ecfab3ff0af074bee59b3c750ecabbf4
child 265095 dedb3abd07fc4393b29dfcb0b86237334a08777e
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu
bugs854037
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 854037 - Make lexical declarations in the initializing component of a for(;;) loop create a fresh binding for each iteration of the loop. r=shu
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/FoldConstants.cpp
js/src/frontend/FullParseHandler.h
js/src/frontend/ParseNode.cpp
js/src/frontend/ParseNode.h
js/src/frontend/Parser.cpp
js/src/jit-test/tests/basic/testBug763950.js
js/src/jit/BaselineCompiler.cpp
js/src/jit/BaselineCompiler.h
js/src/jit/BaselineFrame-inl.h
js/src/jit/BaselineFrame.h
js/src/jit/IonBuilder.cpp
js/src/jit/VMFunctions.cpp
js/src/jit/VMFunctions.h
js/src/jsobj.h
js/src/jsreflect.cpp
js/src/tests/ecma_6/LexicalEnvironment/for-loop.js
js/src/vm/Interpreter.cpp
js/src/vm/Opcodes.h
js/src/vm/ScopeObject.cpp
js/src/vm/ScopeObject.h
js/src/vm/Stack-inl.h
js/src/vm/Stack.cpp
js/src/vm/Stack.h
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -5042,29 +5042,40 @@ BytecodeEmitter::emitNormalFor(ParseNode
 {
     LoopStmtInfo stmtInfo(cx);
     pushLoopStatement(&stmtInfo, STMT_FOR_LOOP, top);
 
     ParseNode *forHead = pn->pn_left;
     ParseNode *forBody = pn->pn_right;
 
     /* C-style for (init; cond; update) ... loop. */
-    JSOp op = JSOP_POP;
-    ParseNode *pn3 = forHead->pn_kid1;
-    if (!pn3) {
-        // No initializer, but emit a nop so that there's somewhere to put the
+    bool forLoopRequiresFreshening = false;
+    JSOp op;
+    ParseNode *init = forHead->pn_kid1;
+    if (!init) {
+        // If there's no init, emit a nop so that there's somewhere to put the
         // SRC_FOR annotation that IonBuilder will look for.
         op = JSOP_NOP;
+    } else if (init->isKind(PNK_FRESHENBLOCK)) {
+        // Also emit a nop, as above.
+        op = JSOP_NOP;
+
+        // The loop's init declaration was hoisted into an enclosing lexical
+        // scope node.  Note that the block scope must be freshened each
+        // iteration.
+        forLoopRequiresFreshening = true;
     } else {
         emittingForInit = true;
-        if (!updateSourceCoordNotes(pn3->pn_pos.begin))
-            return false;
-        if (!emitTree(pn3))
+        if (!updateSourceCoordNotes(init->pn_pos.begin))
+            return false;
+        if (!emitTree(init))
             return false;
         emittingForInit = false;
+
+        op = JSOP_POP;
     }
 
     /*
      * NB: the SRC_FOR note has offsetBias 1 (JSOP_{NOP,POP}_LENGTH).
      * Use tmp to hold the biased srcnote "top" offset, which differs
      * from the top local variable by the length of the JSOP_GOTO
      * emitted in between tmp and top if this loop has a condition.
      */
@@ -5094,29 +5105,48 @@ BytecodeEmitter::emitNormalFor(ParseNode
     if (jmp == -1 && !emitLoopEntry(forBody))
         return false;
     if (!emitTree(forBody))
         return false;
 
     /* Set the second note offset so we can find the update part. */
     ptrdiff_t tmp2 = offset();
 
-    /* Set loop and enclosing "update" offsets, for continue. */
+    // Set loop and enclosing "update" offsets, for continue.  Note that we
+    // continue to immediately *before* the block-freshening: continuing must
+    // refresh the block.
     StmtInfoBCE *stmt = &stmtInfo;
     do {
         stmt->update = offset();
     } while ((stmt = stmt->down) != nullptr && stmt->type == STMT_LABEL);
 
+    // Freshen the block on the scope chain to expose distinct bindings for each loop
+    // iteration.
+    if (forLoopRequiresFreshening) {
+        // The scope chain only includes an actual block *if* the scope object
+        // is captured and therefore requires cloning.  Get the static block
+        // object from the parent let-block statement (which *must* be the
+        // let-statement for the guarding condition to have held) and freshen
+        // if the block object needs cloning.
+        StmtInfoBCE *parent = stmtInfo.down;
+        MOZ_ASSERT(parent->type == STMT_BLOCK);
+        MOZ_ASSERT(parent->isBlockScope);
+
+        if (parent->staticScope->as<StaticBlockObject>().needsClone()) {
+            if (!emit1(JSOP_FRESHENBLOCKSCOPE))
+                return false;
+        }
+    }
+
     /* Check for update code to do before the condition (if any). */
-    pn3 = forHead->pn_kid3;
-    if (pn3) {
-        if (!updateSourceCoordNotes(pn3->pn_pos.begin))
+    if (ParseNode *update = forHead->pn_kid3) {
+        if (!updateSourceCoordNotes(update->pn_pos.begin))
             return false;
         op = JSOP_POP;
-        if (!emitTree(pn3))
+        if (!emitTree(update))
             return false;
 
         /* Always emit the POP or NOP to help IonBuilder. */
         if (!emit1(op))
             return false;
 
         /* Restore the absolute line number for source note readers. */
         uint32_t lineNum = parser->tokenStream.srcCoords.lineNum(pn->pn_pos.end);
--- a/js/src/frontend/FoldConstants.cpp
+++ b/js/src/frontend/FoldConstants.cpp
@@ -410,16 +410,17 @@ ContainsHoistedDeclaration(ExclusiveCont
       case PNK_GENEXP:
       case PNK_ARRAYCOMP:
       case PNK_ARGSBODY:
       case PNK_CATCHLIST:
       case PNK_CATCH:
       case PNK_FORIN:
       case PNK_FOROF:
       case PNK_FORHEAD:
+      case PNK_FRESHENBLOCK:
       case PNK_CLASSMETHOD:
       case PNK_CLASSMETHODLIST:
       case PNK_CLASSNAMES:
         MOZ_CRASH("ContainsHoistedDeclaration should have indicated false on "
                   "some parent node without recurring to test this node");
 
       case PNK_GLOBALCONST:
         MOZ_CRASH("ContainsHoistedDeclaration is only called on nested nodes where "
--- a/js/src/frontend/FullParseHandler.h
+++ b/js/src/frontend/FullParseHandler.h
@@ -494,16 +494,20 @@ class FullParseHandler
 
     ParseNode *newForHead(ParseNodeKind kind, ParseNode *pn1, ParseNode *pn2, ParseNode *pn3,
                           const TokenPos &pos)
     {
         MOZ_ASSERT(kind == PNK_FORIN || kind == PNK_FOROF || kind == PNK_FORHEAD);
         return new_<TernaryNode>(kind, JSOP_NOP, pn1, pn2, pn3, pos);
     }
 
+    ParseNode *newFreshenBlock(const TokenPos &pos) {
+        return new_<NullaryNode>(PNK_FRESHENBLOCK, pos);
+    }
+
     ParseNode *newSwitchStatement(uint32_t begin, ParseNode *discriminant, ParseNode *caseList) {
         TokenPos pos(begin, caseList->pn_pos.end);
         return new_<BinaryNode>(PNK_SWITCH, JSOP_NOP, pos, discriminant, caseList);
     }
 
     ParseNode *newCaseOrDefault(uint32_t begin, ParseNode *expr, ParseNode *body) {
         TokenPos pos(begin, body->pn_pos.end);
         return new_<BinaryNode>(expr ? PNK_CASE : PNK_DEFAULT, JSOP_NOP, pos, expr, body);
--- a/js/src/frontend/ParseNode.cpp
+++ b/js/src/frontend/ParseNode.cpp
@@ -203,16 +203,17 @@ PushNodeChildren(ParseNode *pn, NodeStac
       case PNK_ELISION:
       case PNK_GENERATOR:
       case PNK_NUMBER:
       case PNK_BREAK:
       case PNK_CONTINUE:
       case PNK_DEBUGGER:
       case PNK_EXPORT_BATCH_SPEC:
       case PNK_OBJECT_PROPERTY_NAME:
+      case PNK_FRESHENBLOCK:
         MOZ_ASSERT(pn->isArity(PN_NULLARY));
         MOZ_ASSERT(!pn->isUsed(), "handle non-trivial cases separately");
         MOZ_ASSERT(!pn->isDefn(), "handle non-trivial cases separately");
         return PushResult::Recyclable;
 
       // Nodes with a single non-null child.
       case PNK_TYPEOF:
       case PNK_VOID:
--- a/js/src/frontend/ParseNode.h
+++ b/js/src/frontend/ParseNode.h
@@ -141,16 +141,17 @@ class UpvarCookie
     F(EXPORT_FROM) \
     F(EXPORT_SPEC_LIST) \
     F(EXPORT_SPEC) \
     F(EXPORT_BATCH_SPEC) \
     F(SEQ) \
     F(FORIN) \
     F(FOROF) \
     F(FORHEAD) \
+    F(FRESHENBLOCK) \
     F(ARGSBODY) \
     F(SPREAD) \
     F(MUTATEPROTO) \
     F(CLASS) \
     F(CLASSMETHOD) \
     F(CLASSMETHODLIST) \
     F(CLASSNAMES) \
     \
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -4704,21 +4704,60 @@ Parser<FullParseHandler>::forStatement()
             if (!pn1)
                 return null();
         }
     }
 
     MOZ_ASSERT_IF(isForDecl, pn1->isArity(PN_LIST));
     MOZ_ASSERT(!!blockObj == (isForDecl && pn1->isOp(JSOP_NOP)));
 
-    // The form 'for (let <vars>; <expr2>; <expr3>) <stmt>' generates an
-    // implicit block even if stmt is not a BlockStatement.
-    // If the loop has that exact form, then:
-    // - forLetImpliedBlock is the node for the implicit block scope.
-    // - forLetDecl is the node for the decl 'let <vars>'.
+    // All forms of for-loop (for(;;), for-in, for-of) generate an implicit
+    // block to store any lexical variables declared by the loop-head.  We
+    // implement this by desugaring such loops.  These:
+    //
+    //   for (let/const <pattern-and-assigns>; <test>; <update>) <stmt>
+    //   for (let <pattern> in <expr>) <stmt>
+    //   for (let <pattern> of <expr>) <stmt>
+    //
+    // transform into almost these desugarings:
+    //
+    //   let (<pattern-and-assigns>) { for (; <test>; <update>) <stmt> }
+    //   let (<pattern>) { for (<pattern> in <expr>) <stmt> }
+    //   let (<pattern>) { for (<pattern> of <expr>) <stmt> }
+    //
+    // This desugaring is not *quite* correct.  Assignments in the head of a
+    // let-block are evaluated *outside* the scope of the variables declared by
+    // the let-block-head.  But ES6 mandates that they be evaluated in the same
+    // scope, triggering used-before-initialization temporal dead zone errors
+    // as necessary.  Bug 1069480 will fix this.
+    //
+    // Additionally, ES6 mandates that *each iteration* of a for-loop create a
+    // fresh binding of loop variables.  For example:
+    //
+    //   var funcs = [];
+    //   for (let i = 0; i < 2; i++)
+    //     funcs.push(function() { return i; });
+    //   assertEq(funcs[0](), 0);
+    //   assertEq(funcs[1](), 1);
+    //
+    // These semantics are implemented by "freshening" the implicit block --
+    // changing the scope chain to a fresh clone of the instantaneous block
+    // object -- each iteration, just before evaluating the "update" in
+    // for(;;) loops.  (We don't implement this freshening for for-in/of loops,
+    // but soon: bug 449811.)  No freshening occurs in for (const ...;;) as
+    // there's no point (you can't reassign consts), and moreover the spec
+    // requires it (which fact isn't exposed in-language but can be observed
+    // through the Debugger API).
+    //
+    // If the for-loop head includes a lexical declaration, then we create an
+    // implicit block scope, and:
+    //
+    //   * forLetImpliedBlock is the node for the implicit block scope.
+    //   * forLetDecl is the node for the decl 'let/const <pattern>'.
+    //
     // Otherwise both are null.
     ParseNode *forLetImpliedBlock = nullptr;
     ParseNode *forLetDecl = nullptr;
 
     // If non-null, the node for the decl 'var v = expr1' in the weirdo form
     // 'for (var v = expr1 in expr2) stmt'.
     ParseNode *hoistedVar = nullptr;
 
@@ -4880,33 +4919,55 @@ Parser<FullParseHandler>::forStatement()
         if (isForEach) {
             reportWithOffset(ParseError, false, begin, JSMSG_BAD_FOR_EACH_LOOP);
             return null();
         }
 
         headKind = PNK_FORHEAD;
 
         if (blockObj) {
-            /*
-             * Desugar 'for (let A; B; C) D' into 'let (A) { for (; B; C) D }'
-             * to induce the correct scoping for A. Ensure here that the previously
-             * unchecked assignment mandate for const declarations holds.
-             */
+            // Ensure here that the previously-unchecked assignment mandate for
+            // const declarations holds.
             if (!checkForHeadConstInitializers(pn1)) {
                 report(ParseError, false, nullptr, JSMSG_BAD_CONST_DECL);
                 return null();
             }
 
+            // Desugar
+            //
+            //   for (let INIT; TEST; UPDATE) STMT
+            //
+            // into
+            //
+            //   let (INIT) { for (; TEST; UPDATE) STMT }
+            //
+            // to provide a block scope for INIT.
             forLetImpliedBlock = pushLetScope(blockObj, &letStmt);
             if (!forLetImpliedBlock)
                 return null();
             letStmt.isForLetBlock = true;
 
             forLetDecl = pn1;
-            pn1 = nullptr;
+
+            // The above transformation isn't enough to implement |INIT|
+            // scoping, because each loop iteration must see separate bindings
+            // of |INIT|.  We handle this by replacing the block on the scope
+            // chain with a new block, copying the old one's contents, each
+            // iteration.  We supply a special PNK_FRESHENBLOCK node as the
+            // |let INIT| node for |for(let INIT;;)| loop heads to distinguish
+            // such nodes from *actual*, non-desugared use of the above syntax.
+            // (We don't do this for PNK_CONST nodes because the spec says no
+            // freshening happens -- observable with the Debugger API.)
+            if (pn1->isKind(PNK_CONST)) {
+                pn1 = nullptr;
+            } else {
+                pn1 = handler.newFreshenBlock(pn1->pn_pos);
+                if (!pn1)
+                    return null();
+            }
         }
 
         /* Parse the loop condition or null into pn2. */
         MUST_MATCH_TOKEN(TOK_SEMI, JSMSG_SEMI_AFTER_FOR_INIT);
         TokenKind tt;
         if (!tokenStream.peekToken(&tt, TokenStream::Operand))
             return null();
         if (tt == TOK_SEMI) {
--- a/js/src/jit-test/tests/basic/testBug763950.js
+++ b/js/src/jit-test/tests/basic/testBug763950.js
@@ -1,6 +1,6 @@
 (function() {
     var x;
     for (let j = 0; j < 1; j = j + 1) 
         x = function() { return j; };
-    assertEq(x(), 1);
+    assertEq(x(), 0);
 })();
--- a/js/src/jit/BaselineCompiler.cpp
+++ b/js/src/jit/BaselineCompiler.cpp
@@ -3006,25 +3006,39 @@ BaselineCompiler::emit_JSOP_PUSHBLOCKSCO
 }
 
 typedef bool (*PopBlockScopeFn)(JSContext *, BaselineFrame *);
 static const VMFunction PopBlockScopeInfo = FunctionInfo<PopBlockScopeFn>(jit::PopBlockScope);
 
 bool
 BaselineCompiler::emit_JSOP_POPBLOCKSCOPE()
 {
-    // Call a stub to pop the block from the block chain.
     prepareVMCall();
 
     masm.loadBaselineFramePtr(BaselineFrameReg, R0.scratchReg());
     pushArg(R0.scratchReg());
 
     return callVM(PopBlockScopeInfo);
 }
 
+typedef bool (*FreshenBlockScopeFn)(JSContext *, BaselineFrame *);
+static const VMFunction FreshenBlockScopeInfo =
+    FunctionInfo<FreshenBlockScopeFn>(jit::FreshenBlockScope);
+
+bool
+BaselineCompiler::emit_JSOP_FRESHENBLOCKSCOPE()
+{
+    prepareVMCall();
+
+    masm.loadBaselineFramePtr(BaselineFrameReg, R0.scratchReg());
+    pushArg(R0.scratchReg());
+
+    return callVM(FreshenBlockScopeInfo);
+}
+
 typedef bool (*DebugLeaveBlockFn)(JSContext *, BaselineFrame *, jsbytecode *);
 static const VMFunction DebugLeaveBlockInfo = FunctionInfo<DebugLeaveBlockFn>(jit::DebugLeaveBlock);
 
 bool
 BaselineCompiler::emit_JSOP_DEBUGLEAVEBLOCK()
 {
     if (!compileDebugInstrumentation_)
         return true;
--- a/js/src/jit/BaselineCompiler.h
+++ b/js/src/jit/BaselineCompiler.h
@@ -167,16 +167,17 @@ namespace jit {
     _(JSOP_THROW)              \
     _(JSOP_THROWING)           \
     _(JSOP_TRY)                \
     _(JSOP_FINALLY)            \
     _(JSOP_GOSUB)              \
     _(JSOP_RETSUB)             \
     _(JSOP_PUSHBLOCKSCOPE)     \
     _(JSOP_POPBLOCKSCOPE)      \
+    _(JSOP_FRESHENBLOCKSCOPE)  \
     _(JSOP_DEBUGLEAVEBLOCK)    \
     _(JSOP_EXCEPTION)          \
     _(JSOP_DEBUGGER)           \
     _(JSOP_ARGUMENTS)          \
     _(JSOP_RUNONCE)            \
     _(JSOP_REST)               \
     _(JSOP_TOID)               \
     _(JSOP_TABLESWITCH)        \
--- a/js/src/jit/BaselineFrame-inl.h
+++ b/js/src/jit/BaselineFrame-inl.h
@@ -40,16 +40,23 @@ BaselineFrame::popWith(JSContext *cx)
 {
     if (MOZ_UNLIKELY(isDebuggee()))
         DebugScopes::onPopWith(this);
 
     MOZ_ASSERT(scopeChain()->is<DynamicWithObject>());
     popOffScopeChain();
 }
 
+inline void
+BaselineFrame::replaceInnermostScope(ScopeObject &scope)
+{
+    MOZ_ASSERT(scope.enclosingScope() == scopeChain_->as<ScopeObject>().enclosingScope());
+    scopeChain_ = &scope;
+}
+
 inline bool
 BaselineFrame::pushBlock(JSContext *cx, Handle<StaticBlockObject *> block)
 {
     MOZ_ASSERT(block->needsClone());
 
     ClonedBlockObject *clone = ClonedBlockObject::create(cx, block, this);
     if (!clone)
         return false;
@@ -61,16 +68,28 @@ BaselineFrame::pushBlock(JSContext *cx, 
 inline void
 BaselineFrame::popBlock(JSContext *cx)
 {
     MOZ_ASSERT(scopeChain_->is<ClonedBlockObject>());
 
     popOffScopeChain();
 }
 
+inline bool
+BaselineFrame::freshenBlock(JSContext *cx)
+{
+    Rooted<ClonedBlockObject*> current(cx, &scopeChain_->as<ClonedBlockObject>());
+    ClonedBlockObject *clone = ClonedBlockObject::clone(cx, current);
+    if (!clone)
+        return false;
+
+    replaceInnermostScope(*clone);
+    return true;
+}
+
 inline CallObject &
 BaselineFrame::callObj() const
 {
     MOZ_ASSERT(hasCallObj());
     MOZ_ASSERT(fun()->isHeavyweight());
 
     JSObject *obj = scopeChain();
     while (!obj->is<CallObject>())
--- a/js/src/jit/BaselineFrame.h
+++ b/js/src/jit/BaselineFrame.h
@@ -128,16 +128,17 @@ class BaselineFrame
     }
 
     inline Value *addressOfScratchValue() {
         return reinterpret_cast<Value *>(&loScratchValue_);
     }
 
     inline void pushOnScopeChain(ScopeObject &scope);
     inline void popOffScopeChain();
+    inline void replaceInnermostScope(ScopeObject &scope);
 
     inline void popWith(JSContext *cx);
 
     CalleeToken calleeToken() const {
         uint8_t *pointer = (uint8_t *)this + Size() + offsetOfCalleeToken();
         return *(CalleeToken *)pointer;
     }
     void replaceCalleeToken(CalleeToken token) {
@@ -244,16 +245,17 @@ class BaselineFrame
         flags_ = flags;
     }
     uint32_t *addressOfFlags() {
         return &flags_;
     }
 
     inline bool pushBlock(JSContext *cx, Handle<StaticBlockObject *> block);
     inline void popBlock(JSContext *cx);
+    inline bool freshenBlock(JSContext *cx);
 
     bool strictEvalPrologue(JSContext *cx);
     bool heavyweightFunPrologue(JSContext *cx);
     bool initFunctionScopeObjects(JSContext *cx);
 
     void initArgsObjUnchecked(ArgumentsObject &argsobj) {
         flags_ |= HAS_ARGS_OBJ;
         argsObj_ = &argsobj;
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -2000,16 +2000,27 @@ IonBuilder::inspectOpcode(JSOp op)
 
       case JSOP_GIMPLICITTHIS:
         if (!script()->hasPollutedGlobalScope())
             return pushConstant(UndefinedValue());
 
         // Just fall through to the unsupported bytecode case.
         break;
 
+#ifdef DEBUG
+      case JSOP_PUSHBLOCKSCOPE:
+      case JSOP_FRESHENBLOCKSCOPE:
+      case JSOP_POPBLOCKSCOPE:
+        // These opcodes are currently unhandled by Ion, but in principle
+        // there's no reason they couldn't be.  Whenever this happens, OSR will
+        // have to consider that JSOP_FRESHENBLOCK mutates the scope chain --
+        // right now it caches the scope chain in MBasicBlock::scopeChain().
+        // That stale value will have to be updated when JSOP_FRESHENBLOCK is
+        // encountered.
+#endif
       default:
         break;
     }
 
     // Track a simpler message, since the actionable abort message is a
     // static string, and the internal opcode name isn't an actionable
     // thing anyways.
     trackActionableAbort("Unsupported bytecode");
@@ -3145,23 +3156,28 @@ IonBuilder::forLoop(JSOp op, jssrcnote *
     // for loops have the following structures:
     //
     //   NOP or POP
     //   [GOTO cond | NOP]
     //   LOOPHEAD
     // body:
     //    ; [body]
     // [increment:]
+    //   [FRESHENBLOCKSCOPE, if needed by a cloned block]
     //    ; [increment]
     // [cond:]
     //   LOOPENTRY
     //   GOTO body
     //
     // If there is a condition (condpc != ifne), this acts similar to a while
     // loop otherwise, it acts like a do-while loop.
+    //
+    // Note that currently Ion does not compile pushblockscope/popblockscope as
+    // necessary prerequisites to freshenblockscope.  So the code below doesn't
+    // and needn't consider the implications of freshenblockscope.
     jsbytecode *bodyStart = pc;
     jsbytecode *bodyEnd = updatepc;
     jsbytecode *loopEntry = condpc;
     if (condpc != ifne) {
         MOZ_ASSERT(JSOp(*bodyStart) == JSOP_GOTO);
         MOZ_ASSERT(bodyStart + GetJumpOffset(bodyStart) == condpc);
         bodyStart = GetNextPc(bodyStart);
     } else {
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -990,16 +990,22 @@ PushBlockScope(JSContext *cx, BaselineFr
 bool
 PopBlockScope(JSContext *cx, BaselineFrame *frame)
 {
     frame->popBlock(cx);
     return true;
 }
 
 bool
+FreshenBlockScope(JSContext *cx, BaselineFrame *frame)
+{
+    return frame->freshenBlock(cx);
+}
+
+bool
 DebugLeaveBlock(JSContext *cx, BaselineFrame *frame, jsbytecode *pc)
 {
     MOZ_ASSERT(frame->script()->baselineScript()->hasDebugInstrumentation());
     if (cx->compartment()->isDebuggee())
         DebugScopes::onPopBlock(cx, frame, pc);
     return true;
 }
 
--- a/js/src/jit/VMFunctions.h
+++ b/js/src/jit/VMFunctions.h
@@ -723,16 +723,17 @@ bool OnDebuggerStatement(JSContext *cx, 
 bool GlobalHasLiveOnDebuggerStatement(JSContext *cx);
 
 bool EnterWith(JSContext *cx, BaselineFrame *frame, HandleValue val,
                Handle<StaticWithObject *> templ);
 bool LeaveWith(JSContext *cx, BaselineFrame *frame);
 
 bool PushBlockScope(JSContext *cx, BaselineFrame *frame, Handle<StaticBlockObject *> block);
 bool PopBlockScope(JSContext *cx, BaselineFrame *frame);
+bool FreshenBlockScope(JSContext *cx, BaselineFrame *frame);
 bool DebugLeaveBlock(JSContext *cx, BaselineFrame *frame, jsbytecode *pc);
 
 bool InitBaselineFrameForOsr(BaselineFrame *frame, InterpreterFrame *interpFrame,
                              uint32_t numStackValues);
 
 JSObject *CreateDerivedTypedObj(JSContext *cx, HandleObject descr,
                                 HandleObject owner, int32_t offset);
 
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -142,17 +142,17 @@ class JSObject : public js::gc::Cell
         return groupRaw();
     }
 
     js::ObjectGroup *groupRaw() const {
         return group_;
     }
 
     /*
-     * Whether this is the only object which has its specified gbroup. This
+     * Whether this is the only object which has its specified group. This
      * object will have its group constructed lazily as needed by analysis.
      */
     bool isSingleton() const {
         return group_->singleton();
     }
 
     /*
      * Whether the object's group has not been constructed yet. If an object
@@ -175,17 +175,17 @@ class JSObject : public js::gc::Cell
      */
     static inline JSObject *create(js::ExclusiveContext *cx,
                                    js::gc::AllocKind kind,
                                    js::gc::InitialHeap heap,
                                    js::HandleShape shape,
                                    js::HandleObjectGroup group);
 
     // Set the shape of an object. This pointer is valid for native objects and
-    // some non-native objects. After creating an object, tobjects for which
+    // some non-native objects. After creating an object, the objects for which
     // the shape pointer is invalid need to overwrite this pointer before a GC
     // can occur.
     inline void setInitialShapeMaybeNonNative(js::Shape *shape);
     inline void setShapeMaybeNonNative(js::Shape *shape);
 
     // Set the initial slots and elements of an object. These pointers are only
     // valid for native objects, but during initialization are set for all
     // objects. For non-native objects, these must not be dynamically allocated
--- a/js/src/jsreflect.cpp
+++ b/js/src/jsreflect.cpp
@@ -2555,17 +2555,20 @@ ASTSerializer::statement(ParseNode *pn, 
                     : head->pn_kid1->isKind(PNK_LEXICALSCOPE)
                     ? variableDeclaration(head->pn_kid1->pn_expr, true, &var)
                     : variableDeclaration(head->pn_kid1, false, &var)) &&
                 forOf(pn, head, var, stmt, dst);
         }
 
         RootedValue init(cx), test(cx), update(cx);
 
-        return forInit(head->pn_kid1, &init) &&
+        return forInit(head->pn_kid1 && !head->pn_kid1->isKind(PNK_FRESHENBLOCK)
+                       ? head->pn_kid1
+                       : nullptr,
+                       &init) &&
                optExpression(head->pn_kid2, &test) &&
                optExpression(head->pn_kid3, &update) &&
                builder.forStatement(init, test, update, stmt, &pn->pn_pos, dst);
       }
 
       /* Synthesized by the parser when a for-in loop contains a variable initializer. */
       case PNK_SEQ:
       {
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/LexicalEnvironment/for-loop.js
@@ -0,0 +1,128 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var gTestfile = "for-loop.js";
+//-----------------------------------------------------------------------------
+var BUGNUMBER = 985733;
+var summary =
+  "ES6 for-loop semantics for for(;;) loops whose heads contain lexical "
+  "declarations";
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+function isError(code, type)
+{
+  try
+  {
+    Function(code);
+    throw new Error("didn't throw");
+  }
+  catch (e)
+  {
+    assertEq(e instanceof type, true,
+             "unexpected error for `" + code + "`: got " + e);
+  }
+}
+
+function isOK(code)
+{
+  Function(code);
+}
+
+isError("for (const x; ; ) ;", SyntaxError);
+isError("for (const x = 5, y; ; ) ;", SyntaxError);
+isError("for (const [z]; ; ) ;", SyntaxError);
+//isError("for (const [z, z]; ; ) ;", SyntaxError);
+//isError("for (const [z, z] = [0, 1]; ; ) ;", SyntaxError);
+
+isOK("for (let x; ; ) ;");
+isOK("for (let x = 5, y; ; ) ;");
+
+// I'm fairly sure this is supposed to work: the negative-lookahead rules in
+// IterationStatement ensure that |for (let| *always* is a loop header starting
+// with a lexical declaration.  But I'm not 100% certain, so these tests might
+// need to be fixed when we implement the negative-lookahead restrictions.
+isOK("for (let [z] = [3]; ; ) ;");
+isError("for (let [z, z]; ; ) ;", SyntaxError); // because missing initializer
+
+// This is wrong!  Per 13.2.1.1, "It is a Syntax Error if the BoundNames of
+// BindingList contains any duplicate entries."  But we don't implement this
+// yet, so it becomes a TypeError at runtime.
+isError("for (let [z, z] = [0, 1]; ; ) ;", TypeError);
+
+// A for-loop with lexical declarations, with a mixture of bindings that are and
+// aren't aliased.  (The mixture stress-tests any code that incorrectly assumes
+// all bindings are aliased.)
+var funcs = [];
+for (let [i, j, k] = [0, 1, 2]; i < 10; i++)
+  funcs.push(() => i);
+
+assertEq(funcs[0](), 0);
+assertEq(funcs[1](), 1);
+assertEq(funcs[2](), 2);
+assertEq(funcs[3](), 3);
+assertEq(funcs[4](), 4);
+assertEq(funcs[5](), 5);
+assertEq(funcs[6](), 6);
+assertEq(funcs[7](), 7);
+assertEq(funcs[8](), 8);
+assertEq(funcs[9](), 9);
+
+var outer = "OUTER V IGNORE";
+var save;
+for (let outer = (save = function() { return outer; }); ; )
+  break;
+assertEq(save(), "OUTER V IGNORE",
+         "this is actually a bug: fix for(;;) loops to evaluate init RHSes " +
+         "in the block scope containing all the LHS bindings!");
+
+
+
+var funcs = [];
+function t(i, name, expect)
+{
+  assertEq(funcs[i].name, name);
+  assertEq(funcs[i](), expect);
+}
+
+if (save() !== "OUTER V IGNORE")
+{
+  var v = "OUTER V IGNORE";
+  var i = 0;
+  for (let v = (funcs.push(function init() { return v; }),
+               0);
+      v = (funcs.push(function test() { return v; }),
+           v + 1);
+      v = (funcs.push(function incr() { return v; }),
+           v + 1))
+  {
+    v = (funcs.push(function body() { return v; }),
+         v + 1);
+    i++;
+    if (i >= 3)
+      break;
+  }
+  t(0, "init", 2);
+  t(1, "test", 2);
+  t(2, "body", 2);
+  t(3, "incr", 5);
+  t(4, "test", 5);
+  t(5, "body", 5);
+  t(6, "incr", 8);
+  t(7, "test", 8);
+  t(8, "body", 8);
+  assertEq(funcs.length, 9);
+}
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("Tests complete");
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -1676,17 +1676,16 @@ CASE(JSOP_UNUSED182)
 CASE(JSOP_UNUSED183)
 CASE(JSOP_UNUSED185)
 CASE(JSOP_UNUSED186)
 CASE(JSOP_UNUSED187)
 CASE(JSOP_UNUSED189)
 CASE(JSOP_UNUSED190)
 CASE(JSOP_UNUSED191)
 CASE(JSOP_UNUSED192)
-CASE(JSOP_UNUSED196)
 CASE(JSOP_UNUSED209)
 CASE(JSOP_UNUSED210)
 CASE(JSOP_UNUSED211)
 CASE(JSOP_UNUSED212)
 CASE(JSOP_UNUSED213)
 CASE(JSOP_UNUSED219)
 CASE(JSOP_UNUSED220)
 CASE(JSOP_UNUSED221)
@@ -3435,16 +3434,23 @@ CASE(JSOP_DEBUGLEAVEBLOCK)
     // FIXME: This opcode should not be necessary.  The debugger shouldn't need
     // help from bytecode to do its job.  See bug 927782.
 
     if (MOZ_UNLIKELY(cx->compartment()->isDebuggee()))
         DebugScopes::onPopBlock(cx, REGS.fp(), REGS.pc);
 }
 END_CASE(JSOP_DEBUGLEAVEBLOCK)
 
+CASE(JSOP_FRESHENBLOCKSCOPE)
+{
+    if (!REGS.fp()->freshenBlock(cx))
+        goto error;
+}
+END_CASE(JSOP_FRESHENBLOCKSCOPE)
+
 CASE(JSOP_GENERATOR)
 {
     MOZ_ASSERT(!cx->isExceptionPending());
     MOZ_ASSERT(REGS.stackDepth() == 0);
     JSObject *obj = GeneratorObject::create(cx, REGS.fp());
     if (!obj)
         goto error;
     PUSH_OBJECT(*obj);
--- a/js/src/vm/Opcodes.h
+++ b/js/src/vm/Opcodes.h
@@ -1644,32 +1644,42 @@ 1234567890123456789012345678901234567890
      * throwing ReferenceError if the identified property does not exist.
      *   Category: Literals
      *   Type: Object
      *   Operands: uint32_t nameIndex
      *   Stack: obj => obj[name]
      */ \
     macro(JSOP_GETXPROP,      195,"getxprop",    NULL,    5,  1,  1, JOF_ATOM|JOF_PROP|JOF_TYPESET) \
     \
-    macro(JSOP_UNUSED196,     196,"unused196",   NULL,    1,  0,  0, JOF_BYTE) \
-    \
     /*
      * Pops the top stack value as 'val' and pushes 'typeof val'.  Note that
      * this opcode isn't used when, in the original source code, 'val' is a
      * name -- see 'JSOP_TYPEOF' for that.
      * (This is because 'typeof undefinedName === "undefined"'.)
      *   Category: Operators
      *   Type: Special Operators
      *   Operands:
      *   Stack: val => (typeof val)
      */ \
-    macro(JSOP_TYPEOFEXPR,    197,"typeofexpr",  NULL,    1,  1,  1, JOF_BYTE|JOF_DETECTING) \
+    macro(JSOP_TYPEOFEXPR,    196,"typeofexpr",  NULL,    1,  1,  1, JOF_BYTE|JOF_DETECTING) \
     \
     /* Block-local scope support. */ \
     /*
+     * Replaces the current block on the scope chain with a fresh block
+     * that copies all the bindings in the bock.  This operation implements the
+     * behavior of inducing a fresh block scope for every iteration of a
+     * for(let ...; ...; ...) loop, if any declarations induced by such a loop
+     * are captured within the loop.
+     *   Category: Variables and Scopes
+     *   Type: Block-local Scope
+     *   Operands:
+     *   Stack: =>
+     */ \
+    macro(JSOP_FRESHENBLOCKSCOPE,197,"freshenblockscope", NULL, 1, 0, 0, JOF_BYTE) \
+    /*
      * Pushes block onto the scope chain.
      *   Category: Variables and Scopes
      *   Type: Block-local Scope
      *   Operands: uint32_t staticBlockObjectIndex
      *   Stack: =>
      */ \
     macro(JSOP_PUSHBLOCKSCOPE,198,"pushblockscope", NULL, 5,  0,  0,  JOF_OBJECT) \
     /*
--- a/js/src/vm/ScopeObject.cpp
+++ b/js/src/vm/ScopeObject.cpp
@@ -655,16 +655,45 @@ ClonedBlockObject::copyUnaliasedValues(A
     for (unsigned i = 0; i < numVariables(); ++i) {
         if (!block.isAliased(i)) {
             Value &val = frame.unaliasedLocal(block.blockIndexToLocalIndex(i));
             setVar(i, val, DONT_CHECK_ALIASING);
         }
     }
 }
 
+/* static */ ClonedBlockObject *
+ClonedBlockObject::clone(ExclusiveContext *cx, Handle<ClonedBlockObject*> block)
+{
+    RootedObject enclosing(cx, &block->enclosingScope());
+
+    MOZ_ASSERT(block->getClass() == &BlockObject::class_);
+
+    RootedObjectGroup cloneGroup(cx, block->group());
+    RootedShape cloneShape(cx, block->lastProperty());
+
+    JSObject *obj = JSObject::create(cx, FINALIZE_KIND, gc::TenuredHeap, cloneShape, cloneGroup);
+    if (!obj)
+        return nullptr;
+
+    ClonedBlockObject &copy = obj->as<ClonedBlockObject>();
+
+    MOZ_ASSERT(!copy.inDictionaryMode());
+    MOZ_ASSERT(copy.isDelegate());
+    MOZ_ASSERT(block->slotSpan() == copy.slotSpan());
+    MOZ_ASSERT(copy.slotSpan() >= copy.numVariables() + RESERVED_SLOTS);
+
+    copy.setReservedSlot(SCOPE_CHAIN_SLOT, block->getReservedSlot(SCOPE_CHAIN_SLOT));
+
+    for (uint32_t i = 0, count = copy.numVariables(); i < count; i++)
+        copy.setVar(i, block->var(i, DONT_CHECK_ALIASING), DONT_CHECK_ALIASING);
+
+    return &copy;
+}
+
 StaticBlockObject *
 StaticBlockObject::create(ExclusiveContext *cx)
 {
     RootedObjectGroup group(cx, ObjectGroup::defaultNewGroup(cx, &BlockObject::class_,
                                                              TaggedProto(nullptr)));
     if (!group)
         return nullptr;
 
--- a/js/src/vm/ScopeObject.h
+++ b/js/src/vm/ScopeObject.h
@@ -664,16 +664,22 @@ class ClonedBlockObject : public BlockOb
 
     void setVar(unsigned i, const Value &v, MaybeCheckAliasing checkAliasing = CHECK_ALIASING) {
         MOZ_ASSERT_IF(checkAliasing, staticBlock().isAliased(i));
         setSlotValue(i, v);
     }
 
     /* Copy in all the unaliased formals and locals. */
     void copyUnaliasedValues(AbstractFramePtr frame);
+
+    /*
+     * Create a new ClonedBlockObject with the same enclosing scope and
+     * variable values as this.
+     */
+    static ClonedBlockObject *clone(ExclusiveContext *cx, Handle<ClonedBlockObject*> block);
 };
 
 // Internal scope object used by JSOP_BINDNAME upon encountering an
 // uninitialized lexical slot.
 //
 // ES6 lexical bindings cannot be accessed in any way (throwing
 // ReferenceErrors) until initialized. Normally, NAME operations
 // unconditionally check for uninitialized lexical slots. When getting or
--- a/js/src/vm/Stack-inl.h
+++ b/js/src/vm/Stack-inl.h
@@ -202,16 +202,24 @@ InterpreterFrame::pushOnScopeChain(Scope
 
 inline void
 InterpreterFrame::popOffScopeChain()
 {
     MOZ_ASSERT(flags_ & HAS_SCOPECHAIN);
     scopeChain_ = &scopeChain_->as<ScopeObject>().enclosingScope();
 }
 
+inline void
+InterpreterFrame::replaceInnermostScope(ScopeObject &scope)
+{
+    MOZ_ASSERT(flags_ & HAS_SCOPECHAIN);
+    MOZ_ASSERT(scope.enclosingScope() == scopeChain_->as<ScopeObject>().enclosingScope());
+    scopeChain_ = &scope;
+}
+
 bool
 InterpreterFrame::hasCallObj() const
 {
     MOZ_ASSERT(isStrictEvalFrame() || fun()->isHeavyweight());
     return flags_ & HAS_CALL_OBJ;
 }
 
 inline CallObject &
@@ -794,16 +802,24 @@ AbstractFramePtr::thisValue() const
 {
     if (isInterpreterFrame())
         return asInterpreterFrame()->thisValue();
     if (isBaselineFrame())
         return asBaselineFrame()->thisValue();
     return asRematerializedFrame()->thisValue();
 }
 
+inline bool
+AbstractFramePtr::freshenBlock(JSContext *cx) const
+{
+    if (isInterpreterFrame())
+        return asInterpreterFrame()->freshenBlock(cx);
+    return asBaselineFrame()->freshenBlock(cx);
+}
+
 inline void
 AbstractFramePtr::popBlock(JSContext *cx) const
 {
     if (isInterpreterFrame()) {
         asInterpreterFrame()->popBlock(cx);
         return;
     }
     asBaselineFrame()->popBlock(cx);
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -288,16 +288,29 @@ InterpreterFrame::pushBlock(JSContext *c
     if (!clone)
         return false;
 
     pushOnScopeChain(*clone);
 
     return true;
 }
 
+bool
+InterpreterFrame::freshenBlock(JSContext *cx)
+{
+    MOZ_ASSERT(flags_ & HAS_SCOPECHAIN);
+    Rooted<ClonedBlockObject*> block(cx, &scopeChain_->as<ClonedBlockObject>());
+    ClonedBlockObject *fresh = ClonedBlockObject::clone(cx, block);
+    if (!fresh)
+        return false;
+
+    replaceInnermostScope(*fresh);
+    return true;
+}
+
 void
 InterpreterFrame::popBlock(JSContext *cx)
 {
     MOZ_ASSERT(scopeChain_->is<ClonedBlockObject>());
     popOffScopeChain();
 }
 
 void
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -23,16 +23,17 @@ struct JSCompartment;
 
 namespace js {
 
 class ArgumentsObject;
 class AsmJSModule;
 class InterpreterRegs;
 class CallObject;
 class ScopeObject;
+class ClonedBlockObject;
 class ScriptFrameIter;
 class SPSProfiler;
 class InterpreterFrame;
 class StaticBlockObject;
 
 class ScopeCoordinate;
 
 class SavedFrame;
@@ -229,16 +230,18 @@ class AbstractFramePtr
 
     JSObject *evalPrevScopeChain(JSContext *cx) const;
 
     inline HandleValue returnValue() const;
     inline void setReturnValue(const Value &rval) const;
 
     bool hasPushedSPSFrame() const;
 
+    inline bool freshenBlock(JSContext *cx) const;
+
     inline void popBlock(JSContext *cx) const;
     inline void popWith(JSContext *cx) const;
 
     friend void GDBTestInitAbstractFramePtr(AbstractFramePtr &, void *);
     friend void GDBTestInitAbstractFramePtr(AbstractFramePtr &, InterpreterFrame *);
     friend void GDBTestInitAbstractFramePtr(AbstractFramePtr &, jit::BaselineFrame *);
     friend void GDBTestInitAbstractFramePtr(AbstractFramePtr &, jit::RematerializedFrame *);
 };
@@ -582,24 +585,28 @@ class InterpreterFrame
 
     inline ScopeObject &aliasedVarScope(ScopeCoordinate sc) const;
     inline GlobalObject &global() const;
     inline CallObject &callObj() const;
     inline JSObject &varObj();
 
     inline void pushOnScopeChain(ScopeObject &scope);
     inline void popOffScopeChain();
+    inline void replaceInnermostScope(ScopeObject &scope);
 
     /*
      * For blocks with aliased locals, these interfaces push and pop entries on
-     * the scope chain.
+     * the scope chain.  The "freshen" operation replaces the current block
+     * with a fresh copy of it, to implement semantics providing distinct
+     * bindings per iteration of a for-loop.
      */
 
     bool pushBlock(JSContext *cx, StaticBlockObject &block);
     void popBlock(JSContext *cx);
+    bool freshenBlock(JSContext *cx);
 
     /*
      * With
      *
      * Entering/leaving a |with| block pushes/pops an object on the scope chain.
      * Pushing uses pushOnScopeChain, popping should use popWith.
      */