@samitouri / QOS-React-2 / commits / 79eb250187

Bug repro for unobserved aliased mutation w phi

I found this while working to ensure that we always lower all operands to temporaries. This works: ```javascript // the whole computation of x is memoized in one block, bc of the mutation after the phi let x; if (cond) { x = someObj(); } else { x = someObj(); } mutate(x); ``` However, if you alias either of the operands, we lose the mutation: ```javascript let x; if (cond) { const y = someObj(); // OOPS this gets independently memoized x = y; } else { x = someObj(); } mutate(x); ``` The core issue is that InferMutableRanges does not take into account mutation of phis. ~~My first thought is that we need an additional, outer fixpoint iteration loop to flow mutation back "up" to phi operands~~ edit: there was a much easier fix, we need to alias phi operands and phi id within the existing fixpoint iteration. See follow-up PR which fixes.

Joe Savona committed Feb 21, 2023 at 08:29 UTC 79eb25018756cff958c28c6db523356c6bf5783e
4 files changed +157
compiler/forget/src/__tests__/fixtures/hir/obj-mutated-after-if-else-with-alias.expect.md new
+53
@@ -0,0 +1,53 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function foo(a, b, c, d) {
6 + let x = someObj();
7 + if (a) {
8 + const y = someObj();
9 + const z = y;
10 + x = z;
11 + } else {
12 + x = someObj();
13 + }
14 +
15 + x.f = 1;
16 + return x;
17 +}
18 +
19 +```
20 +
21 +## Code
22 +
23 +```javascript
24 +function foo(a, b, c, d) {
25 + const $ = React.unstable_useMemoCache(3);
26 + const c_0 = $[0] !== a;
27 + let x;
28 + if (c_0) {
29 + x = someObj();
30 + if (a) {
31 + let y;
32 + if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
33 + y = someObj();
34 + $[2] = y;
35 + } else {
36 + y = $[2];
37 + }
38 + const z = y;
39 + x = z;
40 + } else {
41 + x = someObj();
42 + }
43 + x.f = 1;
44 + $[0] = a;
45 + $[1] = x;
46 + } else {
47 + x = $[1];
48 + }
49 + return x;
50 +}
51 +
52 +```
53 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/obj-mutated-after-if-else-with-alias.js new
+13
@@ -0,0 +1,13 @@
1 +function foo(a, b, c, d) {
2 + let x = someObj();
3 + if (a) {
4 + const y = someObj();
5 + const z = y;
6 + x = z;
7 + } else {
8 + x = someObj();
9 + }
10 +
11 + x.f = 1;
12 + return x;
13 +}
compiler/forget/src/__tests__/fixtures/hir/obj-mutated-after-nested-if-else-with-alias.expect.md new
+72
@@ -0,0 +1,72 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function foo(a, b, c, d) {
6 + let x = someObj();
7 + if (a) {
8 + let z;
9 + if (b) {
10 + const w = someObj();
11 + z = w;
12 + } else {
13 + z = someObj();
14 + }
15 + const y = z;
16 + x = z;
17 + } else {
18 + x = someObj();
19 + }
20 +
21 + x.f = 1;
22 + return x;
23 +}
24 +
25 +```
26 +
27 +## Code
28 +
29 +```javascript
30 +function foo(a, b, c, d) {
31 + const $ = React.unstable_useMemoCache(5);
32 + const c_0 = $[0] !== a;
33 + const c_1 = $[1] !== b;
34 + let x;
35 + if (c_0 || c_1) {
36 + x = someObj();
37 + if (a) {
38 + let z = undefined;
39 + if (b) {
40 + let w;
41 + if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
42 + w = someObj();
43 + $[3] = w;
44 + } else {
45 + w = $[3];
46 + }
47 + z = w;
48 + } else {
49 + if ($[4] === Symbol.for("react.memo_cache_sentinel")) {
50 + z = someObj();
51 + $[4] = z;
52 + } else {
53 + z = $[4];
54 + }
55 + }
56 +
57 + x = z;
58 + } else {
59 + x = someObj();
60 + }
61 + x.f = 1;
62 + $[0] = a;
63 + $[1] = b;
64 + $[2] = x;
65 + } else {
66 + x = $[2];
67 + }
68 + return x;
69 +}
70 +
71 +```
72 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/obj-mutated-after-nested-if-else-with-alias.js new
+19
@@ -0,0 +1,19 @@
1 +function foo(a, b, c, d) {
2 + let x = someObj();
3 + if (a) {
4 + let z;
5 + if (b) {
6 + const w = someObj();
7 + z = w;
8 + } else {
9 + z = someObj();
10 + }
11 + const y = z;
12 + x = z;
13 + } else {
14 + x = someObj();
15 + }
16 +
17 + x.f = 1;
18 + return x;
19 +}