Skip to content

Commit 0dae547

Browse files
vladimavladima
committed
Merge pull request microsoft#6898 from Microsoft/modifiedBlockScopedBinding
propagate back assignments to block scoped binding from the loop body
2 parents f058117 + d436d15 commit 0dae547

31 files changed

Lines changed: 2038 additions & 20 deletions

‎src/compiler/checker.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7264,6 +7264,15 @@ namespace ts {
72647264
// mark iteration statement as containing block-scoped binding captured in some function
72657265
getNodeLinks(current).flags |= NodeCheckFlags.LoopWithCapturedBlockScopedBinding;
72667266
}
7267+
7268+
// mark variables that are declared in loop initializer and reassigned inside the body of ForStatement.
7269+
// if body of ForStatement will be converted to function then we'll need a extra machinery to propagate reassigned values back.
7270+
if (container.kind === SyntaxKind.ForStatement &&
7271+
getAncestor(symbol.valueDeclaration, SyntaxKind.VariableDeclarationList).parent === container &&
7272+
isAssignedInBodyOfForStatement(node, <ForStatement>container)) {
7273+
getNodeLinks(symbol.valueDeclaration).flags |= NodeCheckFlags.NeedsLoopOutParameter;
7274+
}
7275+
72677276
// set 'declared inside loop' bit on the block-scoped binding
72687277
getNodeLinks(symbol.valueDeclaration).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
72697278
}
@@ -7273,6 +7282,41 @@ namespace ts {
72737282
}
72747283
}
72757284

7285+
function isAssignedInBodyOfForStatement(node: Identifier, container: ForStatement): boolean {
7286+
let current: Node = node;
7287+
// skip parenthesized nodes
7288+
while (current.parent.kind === SyntaxKind.ParenthesizedExpression) {
7289+
current = current.parent;
7290+
}
7291+
7292+
// check if node is used as LHS in some assignment expression
7293+
let isAssigned = false;
7294+
if (current.parent.kind === SyntaxKind.BinaryExpression) {
7295+
isAssigned = (<BinaryExpression>current.parent).left === current && isAssignmentOperator((<BinaryExpression>current.parent).operatorToken.kind);
7296+
}
7297+
7298+
if ((current.parent.kind === SyntaxKind.PrefixUnaryExpression || current.parent.kind === SyntaxKind.PostfixUnaryExpression)) {
7299+
const expr = <PrefixUnaryExpression | PostfixUnaryExpression>current.parent;
7300+
isAssigned = expr.operator === SyntaxKind.PlusPlusToken || expr.operator === SyntaxKind.MinusMinusToken;
7301+
}
7302+
7303+
if (!isAssigned) {
7304+
return false;
7305+
}
7306+
7307+
// at this point we know that node is the target of assignment
7308+
// now check that modification happens inside the statement part of the ForStatement
7309+
while (current !== container) {
7310+
if (current === container.statement) {
7311+
return true;
7312+
}
7313+
else {
7314+
current = current.parent;
7315+
}
7316+
}
7317+
return false;
7318+
}
7319+
72767320
function captureLexicalThis(node: Node, container: Node): void {
72777321
getNodeLinks(node).flags |= NodeCheckFlags.LexicalThis;
72787322
if (container.kind === SyntaxKind.PropertyDeclaration || container.kind === SyntaxKind.Constructor) {

‎src/compiler/emitter.ts‎

Lines changed: 125 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,54 @@ namespace ts {
287287
_i = 0x10000000, // Use/preference flag for '_i'
288288
}
289289

290+
const enum CopyDirection {
291+
ToOriginal,
292+
ToOutParameter
293+
}
294+
295+
/**
296+
* If loop contains block scoped binding captured in some function then loop body is converted to a function.
297+
* Lexical bindings declared in loop initializer will be passed into the loop body function as parameters,
298+
* however if this binding is modified inside the body - this new value should be propagated back to the original binding.
299+
* This is done by declaring new variable (out parameter holder) outside of the loop for every binding that is reassigned inside the body.
300+
* On every iteration this variable is initialized with value of corresponding binding.
301+
* At every point where control flow leaves the loop either explicitly (break/continue) or implicitly (at the end of loop body)
302+
* we copy the value inside the loop to the out parameter holder.
303+
*
304+
* for (let x;;) {
305+
* let a = 1;
306+
* let b = () => a;
307+
* x++
308+
* if (...) break;
309+
* ...
310+
* }
311+
*
312+
* will be converted to
313+
*
314+
* var out_x;
315+
* var loop = function(x) {
316+
* var a = 1;
317+
* var b = function() { return a; }
318+
* x++;
319+
* if (...) return out_x = x, "break";
320+
* ...
321+
* out_x = x;
322+
* }
323+
* for (var x;;) {
324+
* out_x = x;
325+
* var state = loop(x);
326+
* x = out_x;
327+
* if (state === "break") break;
328+
* }
329+
*
330+
* NOTE: values to out parameters are not copies if loop is abrupted with 'return' - in this case this will end the entire enclosing function
331+
* so nobody can observe this new value.
332+
*/
333+
interface LoopOutParameter {
334+
originalName: Identifier;
335+
outParamName: string;
336+
}
337+
290338
// targetSourceFile is when users only want one file in entire project to be emitted. This is used in compileOnSave feature
291339
export function emitFiles(resolver: EmitResolver, host: EmitHost, targetSourceFile: SourceFile): EmitResult {
292340
// emit output for the __extends helper function
@@ -419,6 +467,11 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
419467
* for (var x;;) loop(x);
420468
*/
421469
hoistedLocalVariables?: Identifier[];
470+
471+
/**
472+
* List of loop out parameters - detailed descripion can be found in the comment to LoopOutParameter
473+
*/
474+
loopOutParameters?: LoopOutParameter[];
422475
}
423476

424477
function setLabeledJump(state: ConvertedLoopState, isBreak: boolean, labelText: string, labelMarker: string): void {
@@ -2968,11 +3021,12 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
29683021
}
29693022

29703023
let loopParameters: string[];
3024+
let loopOutParameters: LoopOutParameter[];
29713025
if (loopInitializer && (getCombinedNodeFlags(loopInitializer) & NodeFlags.BlockScoped)) {
29723026
// if loop initializer contains block scoped variables - they should be passed to converted loop body as parameters
29733027
loopParameters = [];
29743028
for (const varDeclaration of loopInitializer.declarations) {
2975-
collectNames(varDeclaration.name);
3029+
processVariableDeclaration(varDeclaration.name);
29763030
}
29773031
}
29783032

@@ -2982,14 +3036,8 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
29823036
writeLine();
29833037
write(`var ${functionName} = function(${paramList})`);
29843038

2985-
if (!bodyIsBlock) {
2986-
write(" {");
2987-
writeLine();
2988-
increaseIndent();
2989-
}
2990-
29913039
const convertedOuterLoopState = convertedLoopState;
2992-
convertedLoopState = {};
3040+
convertedLoopState = { loopOutParameters };
29933041

29943042
if (convertedOuterLoopState) {
29953043
// convertedOuterLoopState !== undefined means that this converted loop is nested in another converted loop.
@@ -3013,16 +3061,38 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
30133061
}
30143062
}
30153063

3016-
emitEmbeddedStatement(node.statement);
3064+
write(" {");
3065+
writeLine();
3066+
increaseIndent();
30173067

3018-
if (!bodyIsBlock) {
3019-
decreaseIndent();
3020-
writeLine();
3021-
write("}");
3068+
if (bodyIsBlock) {
3069+
emitLines((<Block>node.statement).statements);
30223070
}
3023-
write(";");
3071+
else {
3072+
emit(node.statement);
3073+
}
3074+
3075+
writeLine();
3076+
// end of loop body -> copy out parameter
3077+
copyLoopOutParameters(convertedLoopState, CopyDirection.ToOutParameter, /*emitAsStatements*/true);
3078+
3079+
decreaseIndent();
3080+
writeLine();
3081+
write("};");
30243082
writeLine();
30253083

3084+
if (loopOutParameters) {
3085+
// declare variables to hold out params for loop body
3086+
write(`var `);
3087+
for (let i = 0; i < loopOutParameters.length; i++) {
3088+
if (i !== 0) {
3089+
write(", ");
3090+
}
3091+
write(loopOutParameters[i].outParamName);
3092+
}
3093+
write(";");
3094+
writeLine();
3095+
}
30263096
if (convertedLoopState.argumentsName) {
30273097
// if alias for arguments is set
30283098
if (convertedOuterLoopState) {
@@ -3086,14 +3156,21 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
30863156

30873157
return { functionName, paramList, state: currentLoopState };
30883158

3089-
function collectNames(name: Identifier | BindingPattern): void {
3159+
function processVariableDeclaration(name: Identifier | BindingPattern): void {
30903160
if (name.kind === SyntaxKind.Identifier) {
3091-
const nameText = isNameOfNestedBlockScopedRedeclarationOrCapturedBinding(<Identifier>name) ? getGeneratedNameForNode(name) : (<Identifier>name).text;
3161+
const nameText = isNameOfNestedBlockScopedRedeclarationOrCapturedBinding(<Identifier>name)
3162+
? getGeneratedNameForNode(name)
3163+
: (<Identifier>name).text;
3164+
30923165
loopParameters.push(nameText);
3166+
if (resolver.getNodeCheckFlags(name.parent) & NodeCheckFlags.NeedsLoopOutParameter) {
3167+
const reassignedVariable = { originalName: <Identifier>name, outParamName: makeUniqueName(`out_${nameText}`) };
3168+
(loopOutParameters || (loopOutParameters = [])).push(reassignedVariable);
3169+
}
30933170
}
30943171
else {
30953172
for (const element of (<BindingPattern>name).elements) {
3096-
collectNames(element.name);
3173+
processVariableDeclaration(element.name);
30973174
}
30983175
}
30993176
}
@@ -3124,6 +3201,28 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
31243201
}
31253202
}
31263203

3204+
function copyLoopOutParameters(state: ConvertedLoopState, copyDirection: CopyDirection, emitAsStatements: boolean) {
3205+
if (state.loopOutParameters) {
3206+
for (const outParam of state.loopOutParameters) {
3207+
if (copyDirection === CopyDirection.ToOriginal) {
3208+
emitIdentifier(outParam.originalName);
3209+
write(` = ${outParam.outParamName}`);
3210+
}
3211+
else {
3212+
write(`${outParam.outParamName} = `);
3213+
emitIdentifier(outParam.originalName);
3214+
}
3215+
if (emitAsStatements) {
3216+
write(";");
3217+
writeLine();
3218+
}
3219+
else {
3220+
write(", ");
3221+
}
3222+
}
3223+
}
3224+
}
3225+
31273226
function emitConvertedLoopCall(loop: ConvertedLoop, emitAsBlock: boolean): void {
31283227
if (emitAsBlock) {
31293228
write(" {");
@@ -3144,6 +3243,9 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
31443243
}
31453244

31463245
write(`${loop.functionName}(${loop.paramList});`);
3246+
writeLine();
3247+
3248+
copyLoopOutParameters(loop.state, CopyDirection.ToOriginal, /*emitAsStatements*/ true);
31473249

31483250
if (!isSimpleLoop) {
31493251
// for non simple loops we need to store result returned from converted loop function and use it to do dispatching
@@ -3463,14 +3565,17 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
34633565
(!node.label && (convertedLoopState.allowedNonLabeledJumps & jump));
34643566

34653567
if (!canUseBreakOrContinue) {
3568+
write ("return ");
3569+
// explicit exit from loop -> copy out parameters
3570+
copyLoopOutParameters(convertedLoopState, CopyDirection.ToOutParameter, /*emitAsStatements*/ false);
34663571
if (!node.label) {
34673572
if (node.kind === SyntaxKind.BreakStatement) {
34683573
convertedLoopState.nonLocalJumps |= Jump.Break;
3469-
write(`return "break";`);
3574+
write(`"break";`);
34703575
}
34713576
else {
34723577
convertedLoopState.nonLocalJumps |= Jump.Continue;
3473-
write(`return "continue";`);
3578+
write(`"continue";`);
34743579
}
34753580
}
34763581
else {
@@ -3483,7 +3588,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
34833588
labelMarker = `continue-${node.label.text}`;
34843589
setLabeledJump(convertedLoopState, /*isBreak*/ false, node.label.text, labelMarker);
34853590
}
3486-
write(`return "${labelMarker}";`);
3591+
write(`"${labelMarker}";`);
34873592
}
34883593

34893594
return;

‎src/compiler/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2044,6 +2044,7 @@ namespace ts {
20442044
HasSeenSuperCall = 0x00080000, // Set during the binding when encounter 'super'
20452045
ClassWithBodyScopedClassBinding = 0x00100000, // Decorated class that contains a binding to itself inside of the class body.
20462046
BodyScopedClassBinding = 0x00200000, // Binding to a decorated class inside of the class's body.
2047+
NeedsLoopOutParameter = 0x00400000, // Block scoped binding whose value should be explicitly copied outside of the converted loop
20472048
}
20482049

20492050
/* @internal */

‎tests/baselines/reference/blockScopedBindingsReassignedInLoop1.js‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
//// [blockScopedBindingsReassignedInLoop1.ts]
2+
declare function use(n: number): void;
3+
(function () {
4+
'use strict'
5+
for (let i = 0; i < 9; ++i) {
6+
(() => use(++i))();
7+
}
8+
})();
9+
10+
//// [blockScopedBindingsReassignedInLoop1.js]
11+
(function () {
12+
'use strict';
13+
var _loop_1 = function(i) {
14+
(function () { return use(++i); })();
15+
out_i_1 = i;
16+
};
17+
var out_i_1;
18+
for (var i = 0; i < 9; ++i) {
19+
_loop_1(i);
20+
i = out_i_1;
21+
}
22+
})();

‎tests/baselines/reference/blockScopedBindingsReassignedInLoop1.symbols‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
=== tests/cases/compiler/blockScopedBindingsReassignedInLoop1.ts ===
2+
declare function use(n: number): void;
3+
>use : Symbol(use, Decl(blockScopedBindingsReassignedInLoop1.ts, 0, 0))
4+
>n : Symbol(n, Decl(blockScopedBindingsReassignedInLoop1.ts, 0, 21))
5+
6+
(function () {
7+
'use strict'
8+
for (let i = 0; i < 9; ++i) {
9+
>i : Symbol(i, Decl(blockScopedBindingsReassignedInLoop1.ts, 3, 10))
10+
>i : Symbol(i, Decl(blockScopedBindingsReassignedInLoop1.ts, 3, 10))
11+
>i : Symbol(i, Decl(blockScopedBindingsReassignedInLoop1.ts, 3, 10))
12+
13+
(() => use(++i))();
14+
>use : Symbol(use, Decl(blockScopedBindingsReassignedInLoop1.ts, 0, 0))
15+
>i : Symbol(i, Decl(blockScopedBindingsReassignedInLoop1.ts, 3, 10))
16+
}
17+
})();

‎tests/baselines/reference/blockScopedBindingsReassignedInLoop1.types‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
=== tests/cases/compiler/blockScopedBindingsReassignedInLoop1.ts ===
2+
declare function use(n: number): void;
3+
>use : (n: number) => void
4+
>n : number
5+
6+
(function () {
7+
>(function () { 'use strict' for (let i = 0; i < 9; ++i) { (() => use(++i))(); }})() : void
8+
>(function () { 'use strict' for (let i = 0; i < 9; ++i) { (() => use(++i))(); }}) : () => void
9+
>function () { 'use strict' for (let i = 0; i < 9; ++i) { (() => use(++i))(); }} : () => void
10+
11+
'use strict'
12+
>'use strict' : string
13+
14+
for (let i = 0; i < 9; ++i) {
15+
>i : number
16+
>0 : number
17+
>i < 9 : boolean
18+
>i : number
19+
>9 : number
20+
>++i : number
21+
>i : number
22+
23+
(() => use(++i))();
24+
>(() => use(++i))() : void
25+
>(() => use(++i)) : () => void
26+
>() => use(++i) : () => void
27+
>use(++i) : void
28+
>use : (n: number) => void
29+
>++i : number
30+
>i : number
31+
}
32+
})();

0 commit comments

Comments
 (0)