@samitouri / QOS-React-2 / commits / af91a7ab86

Ensure member path assignments memoize independently

Assignment expressions to a member path are a special case because they're the only place where a value isn't assigned to a (possibly temporary) variable, which is our unit of memoization. #901 demonstrated how this can lead to values that can't be independently memoized: ```javascript const x = {a: a} x.y = [b, c]; // array recomputed w `x`, even if only `a` changed ``` This PR ensures that assignment expressions where the LHS is a member path lower the RHS to a Place. That means the above example is handled as if you wrote: ```javascript const x = {a: a}; const tmp1 = [b, c]; x.y = tmp1; ``` And we independently memoize the temporary.

Joe Savona committed Dec 20, 2022 at 11:06 UTC af91a7ab863c1990be28b7b0db8bc1b0227c82b9
6 files changed +109 -157
compiler/forget/src/HIR/BuildHIR.ts
+4 -1
@@ -946,7 +946,10 @@ function lowerExpression(
946 const operator = expr.node.operator;
947
948 if (operator === "=") {
949 - const right = lowerExpression(builder, expr.get("right"));
949 + const right =
950 + left.memberPath === null
951 + ? lowerExpression(builder, expr.get("right"))
952 + : lowerExpressionToPlace(builder, expr.get("right"));
953 builder.push({
954 id: makeInstructionId(0),
955 lvalue: { place: left, kind: InstructionKind.Reassign },
compiler/forget/src/__tests__/fixtures/hir/_bug_independently-memoize-object-property.expect.md deleted
-131
@@ -1,131 +0,0 @@
1 -
2 -## Input
3 -
4 -```javascript
5 -function foo(a, b, c) {
6 - const x = { a: a };
7 - // TODO @josephsavona: this array *should* be memoized independently from `x`,
8 - // similar to the behavior if we extract into a variable as with `z` below:
9 - x.y = [b, c];
10 -
11 - const y = { a: a };
12 - // this array correctly memoizes independently
13 - const z = [b, c];
14 - y.y = z;
15 -
16 - return [x, y];
17 -}
18 -
19 -```
20 -
21 -## HIR
22 -
23 -```
24 -bb0:
25 - [1] Const mutate x$11_@0:TObject[1:3] = Object { a: read a$8 }
26 - [2] Reassign mutate x$11_@0.y[1:3] = Array [read b$9, read c$10]
27 - [3] Const mutate y$12_@1:TObject[3:6] = Object { a: read a$8 }
28 - [4] Const mutate z$13_@2 = Array [read b$9, read c$10]
29 - [5] Reassign mutate y$12_@1.y[3:6] = read z$13_@2
30 - [6] Const mutate t13$14_@3 = Array [read x$11_@0:TObject, read y$12_@1:TObject]
31 - [7] Return freeze t13$14_@3
32 -```
33 -
34 -## Reactive Scopes
35 -
36 -```
37 -function foo(
38 - a,
39 - b,
40 - c,
41 -) {
42 - scope @0 [1:3] deps=[read a$8, read b$9, read c$10] out=[x$11_@0] {
43 - [1] Const mutate x$11_@0:TObject[1:3] = Object { a: read a$8 }
44 - [2] Reassign mutate x$11_@0.y[1:3] = Array [read b$9, read c$10]
45 - }
46 - scope @1 [3:6] deps=[read a$8, read b$9, read c$10] out=[y$12_@1] {
47 - [3] Const mutate y$12_@1:TObject[3:6] = Object { a: read a$8 }
48 - scope @2 [4:5] deps=[read b$9, read c$10] out=[z$13_@2] {
49 - [4] Const mutate z$13_@2 = Array [read b$9, read c$10]
50 - }
51 - [5] Reassign mutate y$12_@1.y[3:6] = read z$13_@2
52 - }
53 - scope @3 [6:7] deps=[read x$11_@0:TObject, read y$12_@1:TObject] out=[$14_@3] {
54 - [6] Const mutate $14_@3 = Array [read x$11_@0:TObject, read y$12_@1:TObject]
55 - }
56 - return freeze $14_@3
57 -}
58 -
59 -```
60 -
61 -## Code
62 -
63 -```javascript
64 -function foo$0(a$8, b$9, c$10) {
65 - const $ = React.useMemoCache();
66 - const c_0 = $[0] !== a$8;
67 - const c_1 = $[1] !== b$9;
68 - const c_2 = $[2] !== c$10;
69 - let x$11;
70 - if (c_0 || c_1 || c_2) {
71 - x$11 = {
72 - a: a$8,
73 - };
74 - x$11.y = [b$9, c$10];
75 - $[0] = a$8;
76 - $[1] = b$9;
77 - $[2] = c$10;
78 - $[3] = x$11;
79 - } else {
80 - x$11 = $[3];
81 - }
82 -
83 - const c_4 = $[4] !== a$8;
84 - const c_5 = $[5] !== b$9;
85 - const c_6 = $[6] !== c$10;
86 - let y$12;
87 -
88 - if (c_4 || c_5 || c_6) {
89 - y$12 = {
90 - a: a$8,
91 - };
92 - const c_8 = $[8] !== b$9;
93 - const c_9 = $[9] !== c$10;
94 - let z$13;
95 -
96 - if (c_8 || c_9) {
97 - z$13 = [b$9, c$10];
98 - $[8] = b$9;
99 - $[9] = c$10;
100 - $[10] = z$13;
101 - } else {
102 - z$13 = $[10];
103 - }
104 -
105 - y$12.y = z$13;
106 - $[4] = a$8;
107 - $[5] = b$9;
108 - $[6] = c$10;
109 - $[7] = y$12;
110 - } else {
111 - y$12 = $[7];
112 - }
113 -
114 - const c_11 = $[11] !== x$11;
115 - const c_12 = $[12] !== y$12;
116 - let t13$14;
117 -
118 - if (c_11 || c_12) {
119 - t13$14 = [x$11, y$12];
120 - $[11] = x$11;
121 - $[12] = y$12;
122 - $[13] = t13$14;
123 - } else {
124 - t13$14 = $[13];
125 - }
126 -
127 - return t13$14;
128 -}
129 -
130 -```
131 -
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/_bug_independently-memoize-object-property.js deleted
-13
@@ -1,13 +0,0 @@
1 -function foo(a, b, c) {
2 - const x = { a: a };
3 - // TODO @josephsavona: this array *should* be memoized independently from `x`,
4 - // similar to the behavior if we extract into a variable as with `z` below:
5 - x.y = [b, c];
6 -
7 - const y = { a: a };
8 - // this array correctly memoizes independently
9 - const z = [b, c];
10 - y.y = z;
11 -
12 - return [x, y];
13 -}
compiler/forget/src/__tests__/fixtures/hir/assignment-variations.expect.md
+14 -12
@@ -62,11 +62,12 @@ function f$0() {
62
63 ```
64 bb0:
65 - [1] Const mutate $5:TPrimitive = 1
66 - [2] Reassign mutate a$4_@0.b.c[0:5] = Binary read a$4_@0.b.c + read $5:TPrimitive
67 - [3] Const mutate $6:TPrimitive = 2
68 - [4] Reassign mutate a$4_@0.b.c[0:5] = Binary read a$4_@0.b.c * read $6:TPrimitive
69 - [5] Return
65 + [1] Const mutate $6:TPrimitive = 1
66 + [2] Const mutate $7:TPrimitive = Binary read a$5_@0.b.c + read $6:TPrimitive
67 + [3] Reassign read a$5_@0.b.c[0:6] = read $7:TPrimitive
68 + [4] Const mutate $8:TPrimitive = 2
69 + [5] Reassign mutate a$5_@0.b.c[0:6] = Binary read a$5_@0.b.c * read $8:TPrimitive
70 + [6] Return
71 ```
72
73 ## Reactive Scopes
@@ -75,10 +76,11 @@ bb0:
76 function g(
77 a,
78 ) {
78 - [1] Const mutate $5:TPrimitive = 1
79 - [2] Reassign mutate a$4_@0.b.c[0:5] = Binary read a$4_@0.b.c + read $5:TPrimitive
80 - [3] Const mutate $6:TPrimitive = 2
81 - [4] Reassign mutate a$4_@0.b.c[0:5] = Binary read a$4_@0.b.c * read $6:TPrimitive
79 + [1] Const mutate $6:TPrimitive = 1
80 + [2] Const mutate $7:TPrimitive = Binary read a$5_@0.b.c + read $6:TPrimitive
81 + [3] Reassign read a$5_@0.b.c[0:6] = read $7:TPrimitive
82 + [4] Const mutate $8:TPrimitive = 2
83 + [5] Reassign mutate a$5_@0.b.c[0:6] = Binary read a$5_@0.b.c * read $8:TPrimitive
84 return
85 }
86
@@ -87,9 +89,9 @@ function g(
89 ## Code
90
91 ```javascript
90 -function g$0(a$4) {
91 - a$4.c.b = a$4.b.c + 1;
92 - a$4.c.b = a$4.b.c * 2;
92 +function g$0(a$5) {
93 + a$5.c.b = a$5.b.c + 1;
94 + a$5.c.b = a$5.b.c * 2;
95 }
96
97 ```
compiler/forget/src/__tests__/fixtures/hir/independently-memoize-object-property.expect.md new
+84
@@ -0,0 +1,84 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function foo(a, b, c) {
6 + const x = { a: a };
7 + // NOTE: this array should memoize independently from x, w only b,c as deps
8 + x.y = [b, c];
9 +
10 + return x;
11 +}
12 +
13 +```
14 +
15 +## HIR
16 +
17 +```
18 +bb0:
19 + [1] Const mutate x$9_@0:TObject[1:4] = Object { a: read a$6 }
20 + [2] Const mutate t6$10_@1 = Array [read b$7, read c$8]
21 + [3] Reassign mutate x$9_@0.y[1:4] = read t6$10_@1
22 + [4] Return freeze x$9_@0:TObject
23 +```
24 +
25 +## Reactive Scopes
26 +
27 +```
28 +function foo(
29 + a,
30 + b,
31 + c,
32 +) {
33 + scope @0 [1:4] deps=[read a$6, read b$7, read c$8] out=[x$9_@0] {
34 + [1] Const mutate x$9_@0:TObject[1:4] = Object { a: read a$6 }
35 + scope @1 [2:3] deps=[read b$7, read c$8] out=[$10_@1] {
36 + [2] Const mutate $10_@1 = Array [read b$7, read c$8]
37 + }
38 + [3] Reassign mutate x$9_@0.y[1:4] = read $10_@1
39 + }
40 + return freeze x$9_@0:TObject
41 +}
42 +
43 +```
44 +
45 +## Code
46 +
47 +```javascript
48 +function foo$0(a$6, b$7, c$8) {
49 + const $ = React.useMemoCache();
50 + const c_0 = $[0] !== a$6;
51 + const c_1 = $[1] !== b$7;
52 + const c_2 = $[2] !== c$8;
53 + let x$9;
54 + if (c_0 || c_1 || c_2) {
55 + x$9 = {
56 + a: a$6,
57 + };
58 + const c_4 = $[4] !== b$7;
59 + const c_5 = $[5] !== c$8;
60 + let t6$10;
61 +
62 + if (c_4 || c_5) {
63 + t6$10 = [b$7, c$8];
64 + $[4] = b$7;
65 + $[5] = c$8;
66 + $[6] = t6$10;
67 + } else {
68 + t6$10 = $[6];
69 + }
70 +
71 + x$9.y = t6$10;
72 + $[0] = a$6;
73 + $[1] = b$7;
74 + $[2] = c$8;
75 + $[3] = x$9;
76 + } else {
77 + x$9 = $[3];
78 + }
79 +
80 + return x$9;
81 +}
82 +
83 +```
84 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/independently-memoize-object-property.js new
+7
@@ -0,0 +1,7 @@
1 +function foo(a, b, c) {
2 + const x = { a: a };
3 + // NOTE: this array should memoize independently from x, w only b,c as deps
4 + x.y = [b, c];
5 +
6 + return x;
7 +}