[hir] todo tests for lambda capture
--- (I'm not sure if these are already known issues. I found them while playing around with lambda captures. They are also reproducible on main / stable) I have some limited understanding of lambda captures after reading Sathya's posts -- please correct if/where this is incorrect ``` function Component() { // instr1 // instr2 const func3 = function(...) { // func3instr1 } } ``` We currently determine effects of captured references in `AnalyzeFunctions`, before InferReferenceEffects. - i.e. for some function 1. dependencies of all functions (func3.deps) 2. prefix traversal of all instructions (e.g. instr1, instr2, func3.deps, func3instr1, ...) - is this just an implementation decision? i.e. what is stopping us from postfix traversal in InferReferenceEffects (e.g. instr1, instr2, func3instr1, func3.deps) As such, for each captured reference, `AnalyzeFunctions` needs to assign a reference effect. We currently check `MutableRange`, which seems to miss a few cases - We do not model assignments to primitives correctly, since primitives do not have a mutable range. - We're not able to model captured (but not mutated) values correctly. Would it be possible to consolidate `AnalyzeFunctions` into InferReferenceEffects, using some post-order traversal (iterating over a function's instructions to collect its dependencies + associated capture effects)? I definitely don't understand lambdas completely, so please tell me what I'm missing
Mofei Zhang committed
Mar 22, 2023 at 15:58 UTC
8ef22a2a90206eed7c67b5b78be39b4e542b3fff
8 files changed
+253
compiler/forget/src/__tests__/fixtures/compiler/_bug.lambda-capture-returned-alias.expect.md
new
+77
@@ -0,0 +1,77 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// Here, element should not be memoized independently of aliasedElement, since
6
+// it is captured by fn.
7
+// AnalyzeFunctions currently does not find captured objects.
8
+// - mutated context refs are declared as `Capture` effect in `FunctionExpression.deps`
9
+// - all other context refs are left as Unknown. InferReferenceEffects currently demotes
10
+// them to reads
11
+function CaptureNotMutate(props) {
12
+ const idx = foo(props.x);
13
+ const element = bar(props.el);
14
+
15
+ const fn = function () {
16
+ const arr = { element };
17
+ return arr[idx];
18
+ };
19
+ const aliasedElement = fn();
20
+ mutate(aliasedElement);
21
+ return aliasedElement;
22
+}
23
+
24
+```
25
+
26
+## Code
27
+
28
+```javascript
29
+// Here, element should not be memoized independently of aliasedElement, since
30
+// it is captured by fn.
31
+// AnalyzeFunctions currently does not find captured objects.
32
+// - mutated context refs are declared as `Capture` effect in `FunctionExpression.deps`
33
+// - all other context refs are left as Unknown. InferReferenceEffects currently demotes
34
+// them to reads
35
+function CaptureNotMutate(props) {
36
+ const $ = React.unstable_useMemoCache(7);
37
+ const c_0 = $[0] !== props.x;
38
+ let t0;
39
+ if (c_0) {
40
+ t0 = foo(props.x);
41
+ $[0] = props.x;
42
+ $[1] = t0;
43
+ } else {
44
+ t0 = $[1];
45
+ }
46
+ const idx = t0;
47
+ const c_2 = $[2] !== props.el;
48
+ let t1;
49
+ if (c_2) {
50
+ t1 = bar(props.el);
51
+ $[2] = props.el;
52
+ $[3] = t1;
53
+ } else {
54
+ t1 = $[3];
55
+ }
56
+ const element = t1;
57
+ const c_4 = $[4] !== element;
58
+ const c_5 = $[5] !== idx;
59
+ let aliasedElement;
60
+ if (c_4 || c_5) {
61
+ const fn = function () {
62
+ const arr = { element };
63
+ return arr[idx];
64
+ };
65
+ aliasedElement = fn();
66
+ mutate(aliasedElement);
67
+ $[4] = element;
68
+ $[5] = idx;
69
+ $[6] = aliasedElement;
70
+ } else {
71
+ aliasedElement = $[6];
72
+ }
73
+ return aliasedElement;
74
+}
75
+
76
+```
77
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/_bug.lambda-capture-returned-alias.js
new
+18
@@ -0,0 +1,18 @@
1
+// Here, element should not be memoized independently of aliasedElement, since
2
+// it is captured by fn.
3
+// AnalyzeFunctions currently does not find captured objects.
4
+// - mutated context refs are declared as `Capture` effect in `FunctionExpression.deps`
5
+// - all other context refs are left as Unknown. InferReferenceEffects currently demotes
6
+// them to reads
7
+function CaptureNotMutate(props) {
8
+ const idx = foo(props.x);
9
+ const element = bar(props.el);
10
+
11
+ const fn = function () {
12
+ const arr = { element };
13
+ return arr[idx];
14
+ };
15
+ const aliasedElement = fn();
16
+ mutate(aliasedElement);
17
+ return aliasedElement;
18
+}
compiler/forget/src/__tests__/fixtures/compiler/_bug.lambda-mutate-shadowed-object.expect.md
new
+42
@@ -0,0 +1,42 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component() {
6
+ const x = {};
7
+ {
8
+ const x = [];
9
+ const fn = function () {
10
+ mutate(x);
11
+ };
12
+ fn();
13
+ }
14
+ return x; // should return {}
15
+}
16
+
17
+```
18
+
19
+## Code
20
+
21
+```javascript
22
+function Component() {
23
+ const $ = React.unstable_useMemoCache(1);
24
+ let t0;
25
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
26
+ t0 = {};
27
+ $[0] = t0;
28
+ } else {
29
+ t0 = $[0];
30
+ }
31
+ const x = t0;
32
+
33
+ const x_0 = [];
34
+ const fn = function () {
35
+ mutate(x);
36
+ };
37
+ fn();
38
+ return x;
39
+}
40
+
41
+```
42
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/_bug.lambda-mutate-shadowed-object.js
new
+11
@@ -0,0 +1,11 @@
1
+function Component() {
2
+ const x = {};
3
+ {
4
+ const x = [];
5
+ const fn = function () {
6
+ mutate(x);
7
+ };
8
+ fn();
9
+ }
10
+ return x; // should return {}
11
+}
compiler/forget/src/__tests__/fixtures/compiler/_bug.lambda-reassign-primitive.expect.md
new
+39
@@ -0,0 +1,39 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// writing to primitives is not a 'mutate' or 'store' to context references,
6
+// under current analysis in AnalyzeFunctions.
7
+// <unknown> $23:TFunction = Function @deps[<unknown>
8
+// $21:TPrimitive,<unknown> $22:TPrimitive]:
9
+
10
+function Component() {
11
+ let x = 40;
12
+
13
+ const fn = function () {
14
+ x = x + 1;
15
+ };
16
+ fn();
17
+ return x;
18
+}
19
+
20
+```
21
+
22
+## Code
23
+
24
+```javascript
25
+// writing to primitives is not a 'mutate' or 'store' to context references,
26
+// under current analysis in AnalyzeFunctions.
27
+// <unknown> $23:TFunction = Function @deps[<unknown>
28
+// $21:TPrimitive,<unknown> $22:TPrimitive]:
29
+
30
+function Component() {
31
+ const fn = function () {
32
+ x = x + 1;
33
+ };
34
+ fn();
35
+ return 40;
36
+}
37
+
38
+```
39
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/_bug.lambda-reassign-primitive.js
new
+14
@@ -0,0 +1,14 @@
1
+// writing to primitives is not a 'mutate' or 'store' to context references,
2
+// under current analysis in AnalyzeFunctions.
3
+// <unknown> $23:TFunction = Function @deps[<unknown>
4
+// $21:TPrimitive,<unknown> $22:TPrimitive]:
5
+
6
+function Component() {
7
+ let x = 40;
8
+
9
+ const fn = function () {
10
+ x = x + 1;
11
+ };
12
+ fn();
13
+ return x;
14
+}
compiler/forget/src/__tests__/fixtures/compiler/_bug.lambda-reassign-shadowed-primitive.expect.md
new
+41
@@ -0,0 +1,41 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component() {
6
+ const x = {};
7
+ {
8
+ let x = 56;
9
+ const fn = function () {
10
+ x = 42;
11
+ };
12
+ fn();
13
+ }
14
+ return x; // should return {}
15
+}
16
+
17
+```
18
+
19
+## Code
20
+
21
+```javascript
22
+function Component() {
23
+ const $ = React.unstable_useMemoCache(1);
24
+ let t0;
25
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
26
+ t0 = {};
27
+ $[0] = t0;
28
+ } else {
29
+ t0 = $[0];
30
+ }
31
+ const x = t0;
32
+
33
+ const fn = function () {
34
+ x = 42;
35
+ };
36
+ fn();
37
+ return x;
38
+}
39
+
40
+```
41
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/_bug.lambda-reassign-shadowed-primitive.js
new
+11
@@ -0,0 +1,11 @@
1
+function Component() {
2
+ const x = {};
3
+ {
4
+ let x = 56;
5
+ const fn = function () {
6
+ x = 42;
7
+ };
8
+ fn();
9
+ }
10
+ return x; // should return {}
11
+}