@samitouri / QOS-React-2 / commits / 0a6af823b3

Lowering for PropertyStore

Converts assignment expressions where the lvalue is a MemberExpression to use PropertyStore, rather than creating an lvalue with a member path. The net effect is that lvalue will always have a null member path. There are two main cases: * `x.y = <value>`. We lower <value> to a Place, lower the object of the member expression to a place (`object)` create a temporary Place for the result of the assignment, and then create a `PropertyStore <object>, "<property>", <value>`. * `x.y += <value>` (and similar update-in-place operands). We extract the object of the member expression, read the current value via a PropertyLoad, compute the updated value, and store it back with a PropertyStore (each of these goes into its own temporary).

Joe Savona committed Dec 22, 2022 at 15:04 UTC 0a6af823b338d0b5831763163134e36605b6a45d
15 files changed +253 -193
compiler/forget/src/HIR/BuildHIR.ts
+164 -33
@@ -947,18 +947,57 @@ function lowerExpression(
947
948 if (operator === "=") {
949 const left = expr.get("left");
950 - // const left = lowerLVal(builder, expr.get("left"));
951 - const right =
952 - left.node.type === "Identifier"
953 - ? lowerExpression(builder, expr.get("right"))
954 - : lowerExpressionToPlace(builder, expr.get("right"));
955 - return lowerAssignment(
956 - builder,
957 - expr.node.loc ?? GeneratedSource,
958 - InstructionKind.Reassign,
959 - left,
960 - right
961 - );
950 + const leftNode = left.node;
951 + switch (leftNode.type) {
952 + case "Identifier": {
953 + return lowerAssignment(
954 + builder,
955 + leftNode.loc ?? GeneratedSource,
956 + InstructionKind.Reassign,
957 + left,
958 + lowerExpression(builder, expr.get("right"))
959 + );
960 + }
961 + case "MemberExpression": {
962 + const leftExpr = left as NodePath<t.MemberExpression>;
963 + const object = lowerExpressionToPlace(
964 + builder,
965 + leftExpr.get("object")
966 + );
967 + const property = leftExpr.get("property");
968 + invariant(
969 + property.isIdentifier(),
970 + "Assignment expression to dynamic properties is not yet supported"
971 + );
972 + const right = lowerExpressionToPlace(builder, expr.get("right"));
973 + const place: Place = {
974 + kind: "Identifier",
975 + identifier: builder.makeTemporary(),
976 + memberPath: null,
977 + effect: Effect.Read,
978 + loc: exprLoc,
979 + };
980 + builder.push({
981 + id: makeInstructionId(0),
982 + lvalue: { place: { ...place }, kind: InstructionKind.Const },
983 + value: {
984 + kind: "PropertyStore",
985 + object,
986 + property: property.node.name,
987 + value: right,
988 + loc: leftNode.loc ?? GeneratedSource,
989 + },
990 + loc: exprLoc,
991 + });
992 + return place;
993 + }
994 + default: {
995 + todoInvariant(
996 + false,
997 + "Support lvalues other than identifier and member expression"
998 + );
999 + }
1000 + }
1001 }
1002
1003 const operators: { [key: string]: t.BinaryExpression["operator"] } = {
@@ -981,27 +1020,119 @@ function lowerExpression(
1020 `Unhandled assignment operator '${operator}'`
1021 );
1022
984 - const lvalue = lowerLVal(builder, expr.get("left"));
985 - const leftPath = expr.get("left");
986 - invariant(
987 - leftPath.isIdentifier() || leftPath.isMemberExpression(),
988 - "Expected assignment expression lvalue to be an identifier or member expression"
989 - );
990 - const left = lowerExpressionToPlace(builder, leftPath);
991 - const right = lowerExpressionToPlace(builder, expr.get("right"));
992 - builder.push({
993 - id: makeInstructionId(0),
994 - lvalue: { place: lvalue, kind: InstructionKind.Reassign },
995 - value: {
996 - kind: "BinaryExpression",
997 - operator: binaryOperator,
998 - left,
999 - right,
1000 - loc: exprLoc,
1001 - },
1002 - loc: exprLoc,
1003 - });
1004 - return lvalue;
1023 + const left = expr.get("left");
1024 + const leftNode = left.node;
1025 + switch (leftNode.type) {
1026 + case "Identifier": {
1027 + const leftExpr = left as NodePath<t.Identifier>;
1028 + const place = lowerExpressionToPlace(builder, leftExpr);
1029 + const right = lowerExpressionToPlace(builder, expr.get("right"));
1030 + builder.push({
1031 + id: makeInstructionId(0),
1032 + lvalue: { place: { ...place }, kind: InstructionKind.Reassign },
1033 + value: {
1034 + kind: "BinaryExpression",
1035 + operator: binaryOperator,
1036 + left: { ...place },
1037 + right,
1038 + loc: exprLoc,
1039 + },
1040 + loc: exprLoc,
1041 + });
1042 + return place;
1043 + }
1044 + case "MemberExpression": {
1045 + // a.b.c += <right>
1046 + const leftExpr = left as NodePath<t.MemberExpression>;
1047 + // Lower everything up to the final property to a temporary, eg `a.b`
1048 + const object = lowerExpressionToPlace(
1049 + builder,
1050 + leftExpr.get("object")
1051 + );
1052 + // Extract the final property to be read from and re-assigned, eg 'c'
1053 + const property = leftExpr.get("property");
1054 + invariant(
1055 + property.isIdentifier(),
1056 + "Assignment expression to dynamic properties is not yet supported"
1057 + );
1058 + // Store the previous value to a temporary
1059 + const previousValuePlace: Place = {
1060 + kind: "Identifier",
1061 + identifier: builder.makeTemporary(),
1062 + memberPath: null,
1063 + effect: Effect.Read,
1064 + loc: exprLoc,
1065 + };
1066 + builder.push({
1067 + id: makeInstructionId(0),
1068 + lvalue: {
1069 + place: { ...previousValuePlace },
1070 + kind: InstructionKind.Const,
1071 + },
1072 + value: {
1073 + kind: "PropertyLoad",
1074 + object: { ...object },
1075 + property: property.node.name,
1076 + loc: leftExpr.node.loc ?? GeneratedSource,
1077 + },
1078 + loc: leftExpr.node.loc ?? GeneratedSource,
1079 + });
1080 + // Store the new value to a temporary
1081 + const newValuePlace: Place = {
1082 + kind: "Identifier",
1083 + identifier: builder.makeTemporary(),
1084 + memberPath: null,
1085 + effect: Effect.Read,
1086 + loc: exprLoc,
1087 + };
1088 + builder.push({
1089 + id: makeInstructionId(0),
1090 + lvalue: {
1091 + place: { ...newValuePlace },
1092 + kind: InstructionKind.Const,
1093 + },
1094 + value: {
1095 + kind: "BinaryExpression",
1096 + operator: binaryOperator,
1097 + left: { ...previousValuePlace },
1098 + right: lowerExpressionToPlace(builder, expr.get("right")),
1099 + loc: leftExpr.node.loc ?? GeneratedSource,
1100 + },
1101 + loc: leftExpr.node.loc ?? GeneratedSource,
1102 + });
1103 +
1104 + // Save the result back to the property
1105 + const place: Place = {
1106 + kind: "Identifier",
1107 + identifier: builder.makeTemporary(),
1108 + memberPath: null,
1109 + effect: Effect.Read,
1110 + loc: exprLoc,
1111 + };
1112 + builder.push({
1113 + id: makeInstructionId(0),
1114 + lvalue: {
1115 + place: { ...place },
1116 + kind: InstructionKind.Const,
1117 + },
1118 + value: {
1119 + kind: "PropertyStore",
1120 + object: { ...object },
1121 + property: property.node.name,
1122 + value: { ...newValuePlace },
1123 + loc: leftExpr.node.loc ?? GeneratedSource,
1124 + },
1125 + loc: leftExpr.node.loc ?? GeneratedSource,
1126 + });
1127 + return place;
1128 + }
1129 + default: {
1130 + invariant(
1131 + false,
1132 + "Assignment update expressions require the lvalue to be an identifier or member expression"
1133 + );
1134 + }
1135 + }
1136 }
1137 case "MemberExpression": {
1138 const expr = exprPath as NodePath<t.MemberExpression>;
compiler/forget/src/HIR/Codegen.ts
+8 -1
@@ -302,7 +302,14 @@ export function codegenInstruction(
302 if (instr.lvalue === null) {
303 return t.expressionStatement(value);
304 }
305 - if (
305 + if (instr.value.kind === "PropertyStore") {
306 + invariant(
307 + instr.lvalue.place.identifier.name === null,
308 + "Expected property stores to be lowered to a temporary"
309 + );
310 + temp.set(instr.lvalue.place.identifier.id, value);
311 + return t.expressionStatement(value);
312 + } else if (
313 instr.lvalue.place.memberPath === null &&
314 instr.lvalue.place.identifier.name === null
315 ) {
compiler/forget/src/HIR/InferReferenceEffects.ts
+9 -5
@@ -628,6 +628,10 @@ function inferBlock(env: Environment, block: BasicBlock) {
628 env.reference(instrValue, Effect.Read);
629 const lvalue = instr.lvalue;
630 if (lvalue !== null) {
631 + invariant(
632 + lvalue.place.memberPath === null,
633 + "Expected lvalue member path to be null"
634 + );
635 lvalue.place.effect = Effect.Mutate;
636 // direct aliasing: `a = b`;
637 env.alias(lvalue.place, instrValue);
@@ -650,11 +654,11 @@ function inferBlock(env: Environment, block: BasicBlock) {
654
655 env.initialize(instrValue, valueKind);
656 if (instr.lvalue !== null) {
653 - if (instr.lvalue.place.memberPath === null) {
654 - env.define(instr.lvalue.place, instrValue);
655 - } else {
656 - env.reference(instr.lvalue.place, Effect.Mutate);
657 - }
657 + invariant(
658 + instr.lvalue.place.memberPath === null,
659 + "Expected lvalue member path to be null"
660 + );
661 + env.define(instr.lvalue.place, instrValue);
662 instr.lvalue.place.effect = lvalueEffect;
663 }
664 }
compiler/forget/src/__tests__/fixtures/hir/_bug_chained-assignment-expressions.expect.md new
+28
@@ -0,0 +1,28 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function foo() {
6 + const x = { y: 0 };
7 + const y = { z: 0 };
8 + x.y += y.z *= 1;
9 +}
10 +
11 +```
12 +
13 +## Code
14 +
15 +```javascript
16 +function foo() {
17 + const x = {
18 + y: 0,
19 + };
20 + const y = {
21 + z: 0,
22 + };
23 + y.z = y.z * 1;
24 + x.y = x.y + (y.z = y.z * 1);
25 +}
26 +
27 +```
28 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/_bug_chained-assignment-expressions.js new
+5
@@ -0,0 +1,5 @@
1 +function foo() {
2 + const x = { y: 0 };
3 + const y = { z: 0 };
4 + x.y += y.z *= 1;
5 +}
compiler/forget/src/__tests__/fixtures/hir/_bug_invalid-scope.expect.md
+2 -9
@@ -13,15 +13,8 @@ function g(a) {
13
14 ```javascript
15 function g(a) {
16 - const $ = React.useMemoCache();
17 - let a;
18 - if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
19 - a.c.b = a.b.c + 1;
20 - a.c.b = a.b.c * 2;
21 - $[0] = a;
22 - } else {
23 - a = $[0];
24 - }
16 + a.b.c = a.b.c + 1;
17 + a.b.c = a.b.c * 2;
18 }
19
20 ```
compiler/forget/src/__tests__/fixtures/hir/alias-nested-member-path-mutate.expect.md
+3 -21
@@ -17,27 +17,9 @@ function component() {
17
18 ```javascript
19 function component() {
20 - const $ = React.useMemoCache();
21 - let z;
22 - if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
23 - z = [];
24 - $[0] = z;
25 - } else {
26 - z = $[0];
27 - }
28 -
29 - const c_1 = $[1] !== z;
30 - let y;
31 -
32 - if (c_1) {
33 - y = {};
34 - y.z = z;
35 - $[1] = z;
36 - $[2] = y;
37 - } else {
38 - y = $[2];
39 - }
40 -
20 + const z = [];
21 + const y = {};
22 + y.z = z;
23 const x = {};
24 x.y = y;
25 mutate(x.y.z);
compiler/forget/src/__tests__/fixtures/hir/assignment-variations-complex-lvalue.expect.md
+2 -2
@@ -23,8 +23,8 @@ function g() {
23 z: 1,
24 },
25 };
26 - x.z.y = x.y.z + 1;
27 - x.z.y = x.y.z * 2;
26 + x.y.z = x.y.z + 1;
27 + x.y.z = x.y.z * 2;
28 $[0] = x;
29 } else {
30 x = $[0];
compiler/forget/src/__tests__/fixtures/hir/mutable-lifetime-with-aliasing.expect.md
+2 -21
@@ -45,26 +45,8 @@ function mutate(x, y) {}
45
46 ```javascript
47 function Component(props) {
48 - const $ = React.useMemoCache();
49 - let a;
50 - if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
51 - a = {};
52 - $[0] = a;
53 - } else {
54 - a = $[0];
55 - }
56 -
57 - const c_1 = $[1] !== a;
58 - let b;
59 -
60 - if (c_1) {
61 - b = [a];
62 - $[1] = a;
63 - $[2] = b;
64 - } else {
65 - b = $[2];
66 - }
67 -
48 + const a = {};
49 + const b = [a];
50 const c = {};
51 const d = {
52 c: c,
@@ -72,7 +54,6 @@ function Component(props) {
54 const x = {};
55 x.b = b;
56 const y = mutate(x, d);
75 -
57 if (a) {
58 }
59
compiler/forget/src/__tests__/fixtures/hir/property-assignment.expect.md
+20 -33
@@ -18,50 +18,37 @@ function Component(props) {
18 ```javascript
19 function Component(props) {
20 const $ = React.useMemoCache();
21 + const c_0 = $[0] !== props.p0;
22 let x;
22 - if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
23 + let child;
24 + if (c_0) {
25 x = {};
24 - let y;
25 -
26 - if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
27 - y = [];
28 - $[1] = y;
29 - } else {
30 - y = $[1];
31 - }
32 -
26 + const y = [];
27 x.y = y;
34 - $[0] = x;
35 - } else {
36 - x = $[0];
37 - }
38 -
39 - const c_2 = $[2] !== y;
40 - let child;
41 -
42 - if (c_2) {
28 child = <Component data={y}></Component>;
44 - $[2] = y;
45 - $[3] = child;
29 + x.y.push(props.p0);
30 + $[0] = props.p0;
31 + $[1] = x;
32 + $[2] = child;
33 } else {
47 - child = $[3];
34 + x = $[1];
35 + child = $[2];
36 }
37
50 - x.y.push(props.p0);
51 - const c_4 = $[4] !== x;
52 - const c_5 = $[5] !== child;
53 - let t6;
38 + const c_3 = $[3] !== x;
39 + const c_4 = $[4] !== child;
40 + let t5;
41
55 - if (c_4 || c_5) {
56 - t6 = <Component data={x}>{child}</Component>;
57 - $[4] = x;
58 - $[5] = child;
59 - $[6] = t6;
42 + if (c_3 || c_4) {
43 + t5 = <Component data={x}>{child}</Component>;
44 + $[3] = x;
45 + $[4] = child;
46 + $[5] = t5;
47 } else {
61 - t6 = $[6];
48 + t5 = $[5];
49 }
50
64 - return t6;
51 + return t5;
52 }
53
54 ```
compiler/forget/src/__tests__/fixtures/hir/ssa-property-alias-alias-mutate-if.expect.md
+2 -18
@@ -30,26 +30,10 @@ function foo(a) {
30 x = b;
31
32 if (a) {
33 - let y;
34 -
35 - if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
36 - y = {};
37 - $[2] = y;
38 - } else {
39 - y = $[2];
40 - }
41 -
33 + const y = {};
34 x.y = y;
35 } else {
44 - let z;
45 -
46 - if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
47 - z = {};
48 - $[3] = z;
49 - } else {
50 - z = $[3];
51 - }
52 -
36 + const z = {};
37 x.z = z;
38 }
39
compiler/forget/src/__tests__/fixtures/hir/ssa-property-alias-mutate-if.expect.md
+2 -18
@@ -28,26 +28,10 @@ function foo(a) {
28 x = {};
29
30 if (a) {
31 - let y;
32 -
33 - if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
34 - y = {};
35 - $[2] = y;
36 - } else {
37 - y = $[2];
38 - }
39 -
31 + const y = {};
32 x.y = y;
33 } else {
42 - let z;
43 -
44 - if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
45 - z = {};
46 - $[3] = z;
47 - } else {
48 - z = $[3];
49 - }
50 -
34 + const z = {};
35 x.z = z;
36 }
37
compiler/forget/src/__tests__/fixtures/hir/ssa-property-mutate-alias.expect.md
+1 -9
@@ -24,15 +24,7 @@ function foo() {
24 if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
25 const a = {};
26 y = a;
27 - let x;
28 -
29 - if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
30 - x = [];
31 - $[1] = x;
32 - } else {
33 - x = $[1];
34 - }
35 -
27 + const x = [];
28 y.x = x;
29 mutate(a);
30 $[0] = y;
compiler/forget/src/__tests__/fixtures/hir/ssa-property-mutate.expect.md
+4 -14
@@ -17,25 +17,15 @@ function foo() {
17 ```javascript
18 function foo() {
19 const $ = React.useMemoCache();
20 - let x;
21 - if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
22 - x = [];
23 - $[0] = x;
24 - } else {
25 - x = $[0];
26 - }
27 -
28 - const c_1 = $[1] !== x;
20 let y;
30 -
31 - if (c_1) {
21 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
22 + const x = [];
23 y = {};
24 y.x = x;
25 mutate(y);
35 - $[1] = x;
36 - $[2] = y;
26 + $[0] = y;
27 } else {
38 - y = $[2];
28 + y = $[0];
29 }
30
31 return y;
compiler/forget/src/__tests__/fixtures/hir/transitive-alias-fields.expect.md
+1 -9
@@ -21,18 +21,10 @@ function component() {
21
22 ```javascript
23 function component() {
24 - const $ = React.useMemoCache();
24 const x = {};
25 const p = {};
26 const q = {};
28 - let y;
29 - if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
30 - y = {};
31 - $[0] = y;
32 - } else {
33 - y = $[0];
34 - }
35 -
27 + const y = {};
28 x.y = y;
29 p.y = x.y;
30 q.y = p.y;