Skip to content

Commit fbaba90

Browse files
vladimavladima
committed
Merge pull request #5747 from Microsoft/thisWithLetConst
correctly capture 'this' when converting loops into functions
2 parents c94dcbd + 988a512 commit fbaba90

9 files changed

+874
-0
lines changed

‎src/compiler/emitter.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,11 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
402402
*/
403403
argumentsName?: string;
404404

405+
/*
406+
* alias for 'this' from the calling code stack frame in case if this was used inside the converted loop
407+
*/
408+
thisName?: string;
409+
405410
/*
406411
* list of non-block scoped variable declarations that appear inside converted loop
407412
* such variable declarations should be moved outside the loop body
@@ -1992,6 +1997,9 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
19921997
if (resolver.getNodeCheckFlags(node) & NodeCheckFlags.LexicalThis) {
19931998
write("_this");
19941999
}
2000+
else if (convertedLoopState) {
2001+
write(convertedLoopState.thisName || (convertedLoopState.thisName = makeUniqueName("this")));
2002+
}
19952003
else {
19962004
write("this");
19972005
}
@@ -3322,6 +3330,12 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
33223330
convertedLoopState.argumentsName = convertedOuterLoopState.argumentsName;
33233331
}
33243332

3333+
if (convertedOuterLoopState.thisName) {
3334+
// outer loop has already used 'this' so we've already have some name to alias it
3335+
// use the same name in all nested loops
3336+
convertedLoopState.thisName = convertedOuterLoopState.thisName;
3337+
}
3338+
33253339
if (convertedOuterLoopState.hoistedLocalVariables) {
33263340
// we've already collected some non-block scoped variable declarations in enclosing loop
33273341
// use the same storage in nested loop
@@ -3351,6 +3365,21 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
33513365
writeLine();
33523366
}
33533367
}
3368+
if (convertedLoopState.thisName) {
3369+
// if alias for this is set
3370+
if (convertedOuterLoopState) {
3371+
// pass it to outer converted loop
3372+
convertedOuterLoopState.thisName = convertedLoopState.thisName;
3373+
}
3374+
else {
3375+
// this is top level converted loop so we need to create an alias for 'this' here
3376+
// NOTE:
3377+
// if converted loops were all nested in arrow function then we'll always emit '_this' so convertedLoopState.thisName will not be set.
3378+
// If it is set this means that all nested loops are not nested in arrow function and it is safe to capture 'this'.
3379+
write(`var ${convertedLoopState.thisName} = this;`);
3380+
writeLine();
3381+
}
3382+
}
33543383

33553384
if (convertedLoopState.hoistedLocalVariables) {
33563385
// if hoistedLocalVariables !== undefined this means that we've possibly collected some variable declarations to be hoisted later

‎tests/baselines/reference/capturedLetConstInLoop10.js

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
//// [capturedLetConstInLoop10.ts]
2+
class A {
3+
foo() {
4+
for (let x of [0]) {
5+
let f = function() { return x; };
6+
this.bar(f());
7+
}
8+
}
9+
bar(a: number) {
10+
}
11+
12+
baz() {
13+
for (let x of [1]) {
14+
let a = function() { return x; };
15+
for (let y of [1]) {
16+
let b = function() { return y; };
17+
this.bar(b());
18+
}
19+
this.bar(a());
20+
}
21+
}
22+
baz2() {
23+
for (let x of [1]) {
24+
let a = function() { return x; };
25+
this.bar(a());
26+
for (let y of [1]) {
27+
let b = function() { return y; };
28+
this.bar(b());
29+
}
30+
}
31+
}
32+
}
33+
34+
class B {
35+
foo() {
36+
let a =
37+
() => {
38+
for (let x of [0]) {
39+
let f = () => x;
40+
this.bar(f());
41+
}
42+
}
43+
}
44+
bar(a: number) {
45+
}
46+
}
47+
48+
//// [capturedLetConstInLoop10.js]
49+
var A = (function () {
50+
function A() {
51+
}
52+
A.prototype.foo = function () {
53+
var _loop_1 = function(x) {
54+
var f = function () { return x; };
55+
this_1.bar(f());
56+
};
57+
var this_1 = this;
58+
for (var _i = 0, _a = [0]; _i < _a.length; _i++) {
59+
var x = _a[_i];
60+
_loop_1(x);
61+
}
62+
};
63+
A.prototype.bar = function (a) {
64+
};
65+
A.prototype.baz = function () {
66+
var _loop_2 = function(x) {
67+
var a = function () { return x; };
68+
var _loop_3 = function(y) {
69+
var b = function () { return y; };
70+
this_2.bar(b());
71+
};
72+
for (var _i = 0, _a = [1]; _i < _a.length; _i++) {
73+
var y = _a[_i];
74+
_loop_3(y);
75+
}
76+
this_2.bar(a());
77+
};
78+
var this_2 = this;
79+
for (var _b = 0, _c = [1]; _b < _c.length; _b++) {
80+
var x = _c[_b];
81+
_loop_2(x);
82+
}
83+
};
84+
A.prototype.baz2 = function () {
85+
var _loop_4 = function(x) {
86+
var a = function () { return x; };
87+
this_3.bar(a());
88+
var _loop_5 = function(y) {
89+
var b = function () { return y; };
90+
this_3.bar(b());
91+
};
92+
for (var _i = 0, _a = [1]; _i < _a.length; _i++) {
93+
var y = _a[_i];
94+
_loop_5(y);
95+
}
96+
};
97+
var this_3 = this;
98+
for (var _b = 0, _c = [1]; _b < _c.length; _b++) {
99+
var x = _c[_b];
100+
_loop_4(x);
101+
}
102+
};
103+
return A;
104+
})();
105+
var B = (function () {
106+
function B() {
107+
}
108+
B.prototype.foo = function () {
109+
var _this = this;
110+
var a = function () {
111+
var _loop_6 = function(x) {
112+
var f = function () { return x; };
113+
_this.bar(f());
114+
};
115+
for (var _i = 0, _a = [0]; _i < _a.length; _i++) {
116+
var x = _a[_i];
117+
_loop_6(x);
118+
}
119+
};
120+
};
121+
B.prototype.bar = function (a) {
122+
};
123+
return B;
124+
})();

‎tests/baselines/reference/capturedLetConstInLoop10.symbols

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
=== tests/cases/compiler/capturedLetConstInLoop10.ts ===
2+
class A {
3+
>A : Symbol(A, Decl(capturedLetConstInLoop10.ts, 0, 0))
4+
5+
foo() {
6+
>foo : Symbol(foo, Decl(capturedLetConstInLoop10.ts, 0, 9))
7+
8+
for (let x of [0]) {
9+
>x : Symbol(x, Decl(capturedLetConstInLoop10.ts, 2, 16))
10+
11+
let f = function() { return x; };
12+
>f : Symbol(f, Decl(capturedLetConstInLoop10.ts, 3, 15))
13+
>x : Symbol(x, Decl(capturedLetConstInLoop10.ts, 2, 16))
14+
15+
this.bar(f());
16+
>this.bar : Symbol(bar, Decl(capturedLetConstInLoop10.ts, 6, 5))
17+
>this : Symbol(A, Decl(capturedLetConstInLoop10.ts, 0, 0))
18+
>bar : Symbol(bar, Decl(capturedLetConstInLoop10.ts, 6, 5))
19+
>f : Symbol(f, Decl(capturedLetConstInLoop10.ts, 3, 15))
20+
}
21+
}
22+
bar(a: number) {
23+
>bar : Symbol(bar, Decl(capturedLetConstInLoop10.ts, 6, 5))
24+
>a : Symbol(a, Decl(capturedLetConstInLoop10.ts, 7, 8))
25+
}
26+
27+
baz() {
28+
>baz : Symbol(baz, Decl(capturedLetConstInLoop10.ts, 8, 5))
29+
30+
for (let x of [1]) {
31+
>x : Symbol(x, Decl(capturedLetConstInLoop10.ts, 11, 16))
32+
33+
let a = function() { return x; };
34+
>a : Symbol(a, Decl(capturedLetConstInLoop10.ts, 12, 15))
35+
>x : Symbol(x, Decl(capturedLetConstInLoop10.ts, 11, 16))
36+
37+
for (let y of [1]) {
38+
>y : Symbol(y, Decl(capturedLetConstInLoop10.ts, 13, 20))
39+
40+
let b = function() { return y; };
41+
>b : Symbol(b, Decl(capturedLetConstInLoop10.ts, 14, 19))
42+
>y : Symbol(y, Decl(capturedLetConstInLoop10.ts, 13, 20))
43+
44+
this.bar(b());
45+
>this.bar : Symbol(bar, Decl(capturedLetConstInLoop10.ts, 6, 5))
46+
>this : Symbol(A, Decl(capturedLetConstInLoop10.ts, 0, 0))
47+
>bar : Symbol(bar, Decl(capturedLetConstInLoop10.ts, 6, 5))
48+
>b : Symbol(b, Decl(capturedLetConstInLoop10.ts, 14, 19))
49+
}
50+
this.bar(a());
51+
>this.bar : Symbol(bar, Decl(capturedLetConstInLoop10.ts, 6, 5))
52+
>this : Symbol(A, Decl(capturedLetConstInLoop10.ts, 0, 0))
53+
>bar : Symbol(bar, Decl(capturedLetConstInLoop10.ts, 6, 5))
54+
>a : Symbol(a, Decl(capturedLetConstInLoop10.ts, 12, 15))
55+
}
56+
}
57+
baz2() {
58+
>baz2 : Symbol(baz2, Decl(capturedLetConstInLoop10.ts, 19, 5))
59+
60+
for (let x of [1]) {
61+
>x : Symbol(x, Decl(capturedLetConstInLoop10.ts, 21, 16))
62+
63+
let a = function() { return x; };
64+
>a : Symbol(a, Decl(capturedLetConstInLoop10.ts, 22, 15))
65+
>x : Symbol(x, Decl(capturedLetConstInLoop10.ts, 21, 16))
66+
67+
this.bar(a());
68+
>this.bar : Symbol(bar, Decl(capturedLetConstInLoop10.ts, 6, 5))
69+
>this : Symbol(A, Decl(capturedLetConstInLoop10.ts, 0, 0))
70+
>bar : Symbol(bar, Decl(capturedLetConstInLoop10.ts, 6, 5))
71+
>a : Symbol(a, Decl(capturedLetConstInLoop10.ts, 22, 15))
72+
73+
for (let y of [1]) {
74+
>y : Symbol(y, Decl(capturedLetConstInLoop10.ts, 24, 20))
75+
76+
let b = function() { return y; };
77+
>b : Symbol(b, Decl(capturedLetConstInLoop10.ts, 25, 19))
78+
>y : Symbol(y, Decl(capturedLetConstInLoop10.ts, 24, 20))
79+
80+
this.bar(b());
81+
>this.bar : Symbol(bar, Decl(capturedLetConstInLoop10.ts, 6, 5))
82+
>this : Symbol(A, Decl(capturedLetConstInLoop10.ts, 0, 0))
83+
>bar : Symbol(bar, Decl(capturedLetConstInLoop10.ts, 6, 5))
84+
>b : Symbol(b, Decl(capturedLetConstInLoop10.ts, 25, 19))
85+
}
86+
}
87+
}
88+
}
89+
90+
class B {
91+
>B : Symbol(B, Decl(capturedLetConstInLoop10.ts, 30, 1))
92+
93+
foo() {
94+
>foo : Symbol(foo, Decl(capturedLetConstInLoop10.ts, 32, 9))
95+
96+
let a =
97+
>a : Symbol(a, Decl(capturedLetConstInLoop10.ts, 34, 11))
98+
99+
() => {
100+
for (let x of [0]) {
101+
>x : Symbol(x, Decl(capturedLetConstInLoop10.ts, 36, 24))
102+
103+
let f = () => x;
104+
>f : Symbol(f, Decl(capturedLetConstInLoop10.ts, 37, 23))
105+
>x : Symbol(x, Decl(capturedLetConstInLoop10.ts, 36, 24))
106+
107+
this.bar(f());
108+
>this.bar : Symbol(bar, Decl(capturedLetConstInLoop10.ts, 41, 5))
109+
>this : Symbol(B, Decl(capturedLetConstInLoop10.ts, 30, 1))
110+
>bar : Symbol(bar, Decl(capturedLetConstInLoop10.ts, 41, 5))
111+
>f : Symbol(f, Decl(capturedLetConstInLoop10.ts, 37, 23))
112+
}
113+
}
114+
}
115+
bar(a: number) {
116+
>bar : Symbol(bar, Decl(capturedLetConstInLoop10.ts, 41, 5))
117+
>a : Symbol(a, Decl(capturedLetConstInLoop10.ts, 42, 8))
118+
}
119+
}

0 commit comments

Comments
 (0)