Merge pull request #6910 from Microsoft/port-6898

Ports #6898 into release-1.8

Author: vladima

src/compiler/checker.ts

@@ -7242,6 +7242,15 @@ namespace ts {
                     // mark iteration statement as containing block-scoped binding captured in some function
                     getNodeLinks(current).flags |= NodeCheckFlags.LoopWithCapturedBlockScopedBinding;
                 }
+
+                // mark variables that are declared in loop initializer and reassigned inside the body of ForStatement.
+                // if body of ForStatement will be converted to function then we'll need a extra machinery to propagate reassigned values back.
+                if (container.kind === SyntaxKind.ForStatement &&
+                    getAncestor(symbol.valueDeclaration, SyntaxKind.VariableDeclarationList).parent === container &&
+                    isAssignedInBodyOfForStatement(node, <ForStatement>container)) {
+                    getNodeLinks(symbol.valueDeclaration).flags |= NodeCheckFlags.NeedsLoopOutParameter;
+                }
+
                 // set 'declared inside loop' bit on the block-scoped binding
                 getNodeLinks(symbol.valueDeclaration).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
             }
@@ -7251,6 +7260,41 @@ namespace ts {
             }
         }
 
+        function isAssignedInBodyOfForStatement(node: Identifier, container: ForStatement): boolean {
+            let current: Node = node;
+            // skip parenthesized nodes
+            while (current.parent.kind === SyntaxKind.ParenthesizedExpression) {
+                current = current.parent;
+            }
+
+            // check if node is used as LHS in some assignment expression
+            let isAssigned = false;
+            if (current.parent.kind === SyntaxKind.BinaryExpression) {
+                isAssigned = (<BinaryExpression>current.parent).left === current && isAssignmentOperator((<BinaryExpression>current.parent).operatorToken.kind);
+            }
+
+            if ((current.parent.kind === SyntaxKind.PrefixUnaryExpression || current.parent.kind === SyntaxKind.PostfixUnaryExpression)) {
+                const expr = <PrefixUnaryExpression | PostfixUnaryExpression>current.parent;
+                isAssigned = expr.operator === SyntaxKind.PlusPlusToken || expr.operator === SyntaxKind.MinusMinusToken;
+            }
+
+            if (!isAssigned) {
+                return false;
+            }
+
+            // at this point we know that node is the target of assignment
+            // now check that modification happens inside the statement part of the ForStatement
+            while (current !== container) {
+                if (current === container.statement) {
+                    return true;
+                }
+                else {
+                    current = current.parent;
+                }
+            }
+            return false;
+        }
+
         function captureLexicalThis(node: Node, container: Node): void {
             getNodeLinks(node).flags |= NodeCheckFlags.LexicalThis;
             if (container.kind === SyntaxKind.PropertyDeclaration || container.kind === SyntaxKind.Constructor) {

src/compiler/emitter.ts

@@ -287,6 +287,54 @@ namespace ts {
         _i        = 0x10000000,  // Use/preference flag for '_i'
     }
 
+    const enum CopyDirection {
+        ToOriginal,
+        ToOutParameter
+    }
+
+    /**
+     * If loop contains block scoped binding captured in some function then loop body is converted to a function.
+     * Lexical bindings declared in loop initializer will be passed into the loop body function as parameters,
+     * however if this binding is modified inside the body - this new value should be propagated back to the original binding.
+     * This is done by declaring new variable (out parameter holder) outside of the loop for every binding that is reassigned inside the body. 
+     * On every iteration this variable is initialized with value of corresponding binding.
+     * At every point where control flow leaves the loop either explicitly (break/continue) or implicitly (at the end of loop body)
+     * we copy the value inside the loop to the out parameter holder.
+     * 
+     * for (let x;;) {
+     *     let a = 1;
+     *     let b = () => a;
+     *     x++
+     *     if (...) break;
+     *     ...
+     * }
+     * 
+     * will be converted to
+     * 
+     * var out_x;
+     * var loop = function(x) {
+     *     var a = 1;
+     *     var b = function() { return a; }
+     *     x++;
+     *     if (...) return out_x = x, "break";
+     *     ...
+     *     out_x = x;
+     * }
+     * for (var x;;) {
+     *     out_x = x;
+     *     var state = loop(x);
+     *     x = out_x;
+     *     if (state === "break") break;
+     * }
+     * 
+     * NOTE: values to out parameters are not copies if loop is abrupted with 'return' - in this case this will end the entire enclosing function
+     * so nobody can observe this new value.
+     */
+    interface LoopOutParameter {
+        originalName: Identifier;
+        outParamName: string;
+    }
+
     // targetSourceFile is when users only want one file in entire project to be emitted. This is used in compileOnSave feature
     export function emitFiles(resolver: EmitResolver, host: EmitHost, targetSourceFile: SourceFile): EmitResult {
         // emit output for the __extends helper function
@@ -419,6 +467,11 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
              * for (var x;;) loop(x);
              */
             hoistedLocalVariables?: Identifier[];
+
+            /**
+             * List of loop out parameters - detailed descripion can be found in the comment to LoopOutParameter
+             */
+            loopOutParameters?: LoopOutParameter[];
         }
 
         function setLabeledJump(state: ConvertedLoopState, isBreak: boolean, labelText: string, labelMarker: string): void {
@@ -2944,11 +2997,12 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
                 }
 
                 let loopParameters: string[];
+                let loopOutParameters: LoopOutParameter[];
                 if (loopInitializer && (getCombinedNodeFlags(loopInitializer) & NodeFlags.BlockScoped)) {
                     // if loop initializer contains block scoped variables - they should be passed to converted loop body as parameters
                     loopParameters = [];
                     for (const varDeclaration of loopInitializer.declarations) {
-                        collectNames(varDeclaration.name);
+                        processVariableDeclaration(varDeclaration.name);
                     }
                 }
 
@@ -2958,14 +3012,8 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
                 writeLine();
                 write(`var ${functionName} = function(${paramList})`);
 
-                if (!bodyIsBlock) {
-                    write(" {");
-                    writeLine();
-                    increaseIndent();
-                }
-
                 const convertedOuterLoopState = convertedLoopState;
-                convertedLoopState = {};
+                convertedLoopState = { loopOutParameters };
 
                 if (convertedOuterLoopState) {
                     // convertedOuterLoopState !== undefined means that this converted loop is nested in another converted loop.
@@ -2989,16 +3037,38 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
                     }
                 }
 
-                emitEmbeddedStatement(node.statement);
+                write(" {");
+                writeLine();
+                increaseIndent();
 
-                if (!bodyIsBlock) {
-                    decreaseIndent();
-                    writeLine();
-                    write("}");
+                if (bodyIsBlock) {
+                    emitLines((<Block>node.statement).statements);
                 }
-                write(";");
+                else {
+                    emit(node.statement);
+                }
+
+                writeLine();
+                // end of loop body -> copy out parameter 
+                copyLoopOutParameters(convertedLoopState, CopyDirection.ToOutParameter, /*emitAsStatements*/true);
+
+                decreaseIndent();
+                writeLine();
+                write("};");
                 writeLine();
 
+                if (loopOutParameters) {
+                    // declare variables to hold out params for loop body
+                    write(`var `);
+                    for (let i = 0; i < loopOutParameters.length; i++) {
+                        if (i !== 0) {
+                            write(", ");
+                        }
+                        write(loopOutParameters[i].outParamName);
+                    }
+                    write(";");
+                    writeLine();
+                }
                 if (convertedLoopState.argumentsName) {
                     // if alias for arguments is set
                     if (convertedOuterLoopState) {
@@ -3062,14 +3132,21 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
 
                 return { functionName, paramList, state: currentLoopState };
 
-                function collectNames(name: Identifier | BindingPattern): void {
+                function processVariableDeclaration(name: Identifier | BindingPattern): void {
                     if (name.kind === SyntaxKind.Identifier) {
-                        const nameText = isNameOfNestedBlockScopedRedeclarationOrCapturedBinding(<Identifier>name) ? getGeneratedNameForNode(name) : (<Identifier>name).text;
+                        const nameText = isNameOfNestedBlockScopedRedeclarationOrCapturedBinding(<Identifier>name)
+                            ? getGeneratedNameForNode(name)
+                            : (<Identifier>name).text;
+
                         loopParameters.push(nameText);
+                        if (resolver.getNodeCheckFlags(name.parent) & NodeCheckFlags.NeedsLoopOutParameter) {
+                            const reassignedVariable = { originalName: <Identifier>name, outParamName: makeUniqueName(`out_${nameText}`) };
+                            (loopOutParameters || (loopOutParameters = [])).push(reassignedVariable);
+                        }
                     }
                     else {
                         for (const element of (<BindingPattern>name).elements) {
-                            collectNames(element.name);
+                            processVariableDeclaration(element.name);
                         }
                     }
                 }
@@ -3100,6 +3177,28 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
                 }
             }
 
+            function copyLoopOutParameters(state: ConvertedLoopState, copyDirection: CopyDirection, emitAsStatements: boolean) {
+                if (state.loopOutParameters) {
+                    for (const outParam of state.loopOutParameters) {
+                        if (copyDirection === CopyDirection.ToOriginal) {
+                            emitIdentifier(outParam.originalName);
+                            write(` = ${outParam.outParamName}`);
+                        }
+                        else {
+                            write(`${outParam.outParamName} = `);
+                            emitIdentifier(outParam.originalName);
+                        }
+                        if (emitAsStatements) {
+                            write(";");
+                            writeLine();
+                        }
+                        else {
+                            write(", ");
+                        }
+                    }
+                }
+            }
+
             function emitConvertedLoopCall(loop: ConvertedLoop, emitAsBlock: boolean): void {
                 if (emitAsBlock) {
                     write(" {");
@@ -3120,6 +3219,9 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
                 }
 
                 write(`${loop.functionName}(${loop.paramList});`);
+                writeLine();
+
+                copyLoopOutParameters(loop.state, CopyDirection.ToOriginal, /*emitAsStatements*/ true);
 
                 if (!isSimpleLoop) {
                     // for non simple loops we need to store result returned from converted loop function and use it to do dispatching
@@ -3138,7 +3240,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
                         }
                         else {
                             // top level converted loop - return unwrapped value
-                            write(`return ${loopResult}.value`);
+                            write(`return ${loopResult}.value;`);
                         }
                         writeLine();
                     }
@@ -3439,14 +3541,17 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
                         (!node.label && (convertedLoopState.allowedNonLabeledJumps & jump));
 
                     if (!canUseBreakOrContinue) {
+                        write ("return ");
+                        // explicit exit from loop -> copy out parameters
+                        copyLoopOutParameters(convertedLoopState, CopyDirection.ToOutParameter, /*emitAsStatements*/ false);
                         if (!node.label) {
                             if (node.kind === SyntaxKind.BreakStatement) {
                                 convertedLoopState.nonLocalJumps |= Jump.Break;
-                                write(`return "break";`);
+                                write(`"break";`);
                             }
                             else {
                                 convertedLoopState.nonLocalJumps |= Jump.Continue;
-                                write(`return "continue";`);
+                                write(`"continue";`);
                             }
                         }
                         else {
@@ -3459,7 +3564,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
                                 labelMarker = `continue-${node.label.text}`;
                                 setLabeledJump(convertedLoopState, /*isBreak*/ false, node.label.text, labelMarker);
                             }
-                            write(`return "${labelMarker}";`);
+                            write(`"${labelMarker}";`);
                         }
 
                         return;

src/compiler/types.ts

@@ -2071,6 +2071,7 @@ namespace ts {
         HasSeenSuperCall           = 0x00080000,         // Set during the binding when encounter 'super'
         ClassWithBodyScopedClassBinding = 0x00100000,    // Decorated class that contains a binding to itself inside of the class body.
         BodyScopedClassBinding      = 0x00200000,        // Binding to a decorated class inside of the class's body.
+        NeedsLoopOutParameter               = 0x00400000, // Block scoped binding whose value should be explicitly copied outside of the converted loop
     }
 
     /* @internal */

tests/baselines/reference/blockScopedBindingsReassignedInLoop1.js

@@ -0,0 +1,22 @@
+//// [blockScopedBindingsReassignedInLoop1.ts]
+declare function use(n: number): void;
+(function () {
+  'use strict'
+  for (let i = 0; i < 9; ++i) {
+    (() => use(++i))();
+  }
+})();
+
+//// [blockScopedBindingsReassignedInLoop1.js]
+(function () {
+    'use strict';
+    var _loop_1 = function(i) {
+        (function () { return use(++i); })();
+        out_i_1 = i;
+    };
+    var out_i_1;
+    for (var i = 0; i < 9; ++i) {
+        _loop_1(i);
+        i = out_i_1;
+    }
+})();

tests/baselines/reference/blockScopedBindingsReassignedInLoop1.symbols

@@ -0,0 +1,17 @@
+=== tests/cases/compiler/blockScopedBindingsReassignedInLoop1.ts ===
+declare function use(n: number): void;
+>use : Symbol(use, Decl(blockScopedBindingsReassignedInLoop1.ts, 0, 0))
+>n : Symbol(n, Decl(blockScopedBindingsReassignedInLoop1.ts, 0, 21))
+
+(function () {
+  'use strict'
+  for (let i = 0; i < 9; ++i) {
+>i : Symbol(i, Decl(blockScopedBindingsReassignedInLoop1.ts, 3, 10))
+>i : Symbol(i, Decl(blockScopedBindingsReassignedInLoop1.ts, 3, 10))
+>i : Symbol(i, Decl(blockScopedBindingsReassignedInLoop1.ts, 3, 10))
+
+    (() => use(++i))();
+>use : Symbol(use, Decl(blockScopedBindingsReassignedInLoop1.ts, 0, 0))
+>i : Symbol(i, Decl(blockScopedBindingsReassignedInLoop1.ts, 3, 10))
+  }
+})();

tests/baselines/reference/blockScopedBindingsReassignedInLoop1.types

@@ -0,0 +1,32 @@
+=== tests/cases/compiler/blockScopedBindingsReassignedInLoop1.ts ===
+declare function use(n: number): void;
+>use : (n: number) => void
+>n : number
+
+(function () {
+>(function () {  'use strict'  for (let i = 0; i < 9; ++i) {    (() => use(++i))();  }})() : void
+>(function () {  'use strict'  for (let i = 0; i < 9; ++i) {    (() => use(++i))();  }}) : () => void
+>function () {  'use strict'  for (let i = 0; i < 9; ++i) {    (() => use(++i))();  }} : () => void
+
+  'use strict'
+>'use strict' : string
+
+  for (let i = 0; i < 9; ++i) {
+>i : number
+>0 : number
+>i < 9 : boolean
+>i : number
+>9 : number
+>++i : number
+>i : number
+
+    (() => use(++i))();
+>(() => use(++i))() : void
+>(() => use(++i)) : () => void
+>() => use(++i) : () => void
+>use(++i) : void
+>use : (n: number) => void
+>++i : number
+>i : number
+  }
+})();