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

Add PropertyCall/ComputedCall instructions

Adds `PropertyCall` and `ComputedCall`, which are a combination of CallExpression and PropertyLoad/ComputedLoad, respectively. The goal is to ensure that we correctly model the receiver of a call where the callee is a member expression, and also accurately record scope dependencies in for both the computed and non-computed (property) cases. An alternative that I tried first was to add a `receiver: Place | null` to CallExpression. That works well for HIR construction, but it's then very difficult at codegen time to correctly reconstruct the original call: if the receiver and callee share part of their structure then we can transform back to a non-computed member expression, otherwise it has to be computed. Eg we have to distinguish `a.b.c[d.foo]()` from `a.b.c[a.b.c.foo]()`. Given that our target is high-level code, it seems reasonable to have a higher-level representation for these cases. I'm open to feedback but this feels pretty reasonable in terms of complexity / precision of modeling.

Joe Savona committed Jan 17, 2023 at 09:40 UTC edeaaaf38aea54152f93a6136fdef15c49a2fc82
16 files changed +431 -77
compiler/forget/src/HIR/BuildHIR.ts
+77 -39
@@ -902,21 +902,51 @@ function lowerExpression(
902 calleePath.isExpression(),
903 "Call expressions only support callees that are expressions (v8 intrinsics not supported)"
904 );
905 - const callee = lowerExpressionToPlace(builder, calleePath);
906 - const argPaths = expr.get("arguments");
907 - const args = argPaths.map((arg) => {
908 - todoInvariant(
909 - arg.isExpression(),
910 - "todo: support non-expression call arguments"
905 + if (calleePath.isMemberExpression()) {
906 + const { object, property, value } = lowerMemberExpression(
907 + builder,
908 + calleePath
909 );
912 - return lowerExpressionToPlace(builder, arg);
913 - });
914 - return {
915 - kind: "CallExpression",
916 - callee,
917 - args,
918 - loc: exprLoc,
919 - };
910 + const args = expr.get("arguments").map((arg) => {
911 + todoInvariant(
912 + arg.isExpression(),
913 + "todo: support non-expression call arguments"
914 + );
915 + return lowerExpressionToPlace(builder, arg);
916 + });
917 + if (typeof property === "string") {
918 + return {
919 + kind: "PropertyCall",
920 + receiver: object,
921 + property,
922 + args,
923 + loc: exprLoc,
924 + };
925 + } else {
926 + return {
927 + kind: "ComputedCall",
928 + receiver: object,
929 + property,
930 + args,
931 + loc: exprLoc,
932 + };
933 + }
934 + } else {
935 + const callee = lowerExpressionToPlace(builder, calleePath);
936 + const args = expr.get("arguments").map((arg) => {
937 + todoInvariant(
938 + arg.isExpression(),
939 + "todo: support non-expression call arguments"
940 + );
941 + return lowerExpressionToPlace(builder, arg);
942 + });
943 + return {
944 + kind: "CallExpression",
945 + callee,
946 + args,
947 + loc: exprLoc,
948 + };
949 + }
950 }
951 case "BinaryExpression": {
952 const expr = exprPath as NodePath<t.BinaryExpression>;
@@ -1141,31 +1171,7 @@ function lowerExpression(
1171 }
1172 case "MemberExpression": {
1173 const expr = exprPath as NodePath<t.MemberExpression>;
1144 - const object = lowerExpressionToPlace(builder, expr.get("object"));
1145 - invariant(object.kind === "Identifier", "scope cannot appear here");
1146 - const property = expr.get("property");
1147 - let value: InstructionValue;
1148 - if (!expr.node.computed) {
1149 - todoInvariant(property.isIdentifier(), "Support private names");
1150 - value = {
1151 - kind: "PropertyLoad",
1152 - object,
1153 - property: property.node.name,
1154 - loc: exprLoc,
1155 - };
1156 - } else {
1157 - invariant(
1158 - property.isExpression(),
1159 - "Expected private names to be non-computed"
1160 - );
1161 - const propertyPlace = lowerExpressionToPlace(builder, property);
1162 - value = {
1163 - kind: "ComputedLoad",
1164 - object,
1165 - property: propertyPlace,
1166 - loc: exprLoc,
1167 - };
1168 - }
1174 + const { value } = lowerMemberExpression(builder, expr);
1175 const place: Place = buildTemporaryPlace(builder, exprLoc);
1176 builder.push({
1177 id: makeInstructionId(0),
@@ -1258,6 +1264,38 @@ function lowerExpression(
1264 }
1265 }
1266
1267 +function lowerMemberExpression(
1268 + builder: HIRBuilder,
1269 + expr: NodePath<t.MemberExpression>
1270 +): { object: Place; property: Place | string; value: InstructionValue } {
1271 + const exprLoc = expr.node.loc ?? GeneratedSource;
1272 + const object = lowerExpressionToPlace(builder, expr.get("object"));
1273 + const property = expr.get("property");
1274 + if (!expr.node.computed) {
1275 + todoInvariant(property.isIdentifier(), "Support private names");
1276 + const value: InstructionValue = {
1277 + kind: "PropertyLoad",
1278 + object: { ...object },
1279 + property: property.node.name,
1280 + loc: exprLoc,
1281 + };
1282 + return { object, property: property.node.name, value };
1283 + } else {
1284 + invariant(
1285 + property.isExpression(),
1286 + "Expected private names to be non-computed"
1287 + );
1288 + const propertyPlace = lowerExpressionToPlace(builder, property);
1289 + const value: InstructionValue = {
1290 + kind: "ComputedLoad",
1291 + object: { ...object },
1292 + property: { ...propertyPlace },
1293 + loc: exprLoc,
1294 + };
1295 + return { object, property: propertyPlace, value };
1296 + }
1297 +}
1298 +
1299 function lowerConditional(
1300 builder: HIRBuilder,
1301 test: Place,
compiler/forget/src/HIR/HIR.ts
+17 -1
@@ -355,7 +355,23 @@ export type InstructionData =
355 right: Place;
356 }
357 | { kind: "NewExpression"; callee: Place; args: Array<Place> }
358 - | { kind: "CallExpression"; callee: Place; args: Array<Place> }
358 + | {
359 + kind: "CallExpression";
360 + callee: Place;
361 + args: Array<Place>;
362 + }
363 + | {
364 + kind: "PropertyCall";
365 + receiver: Place;
366 + property: string;
367 + args: Array<Place>;
368 + }
369 + | {
370 + kind: "ComputedCall";
371 + receiver: Place;
372 + property: Place;
373 + args: Array<Place>;
374 + }
375 | { kind: "UnaryExpression"; operator: string; value: Place }
376 | {
377 kind: "JsxExpression";
compiler/forget/src/HIR/PrintHIR.ts
+12
@@ -229,6 +229,18 @@ export function printInstructionValue(instrValue: InstructionValue): string {
229 .join(", ")})`;
230 break;
231 }
232 + case "PropertyCall": {
233 + value = `PropertyCall ${printPlace(instrValue.receiver)}.${
234 + instrValue.property
235 + }(${instrValue.args.map((arg) => printPlace(arg)).join(", ")})`;
236 + break;
237 + }
238 + case "ComputedCall": {
239 + value = `ComputedCall ${printPlace(instrValue.receiver)}[${printPlace(
240 + instrValue.property
241 + )}](${instrValue.args.map((arg) => printPlace(arg)).join(", ")})`;
242 + break;
243 + }
244 case "JSXText":
245 case "Primitive": {
246 value = JSON.stringify(instrValue.value);
compiler/forget/src/HIR/visitors.ts
+22
@@ -37,6 +37,17 @@ export function* eachInstructionValueOperand(
37 yield instrValue.right;
38 break;
39 }
40 + case "PropertyCall": {
41 + yield instrValue.receiver;
42 + yield* instrValue.args;
43 + break;
44 + }
45 + case "ComputedCall": {
46 + yield instrValue.receiver;
47 + yield instrValue.property;
48 + yield* instrValue.args;
49 + break;
50 + }
51 case "Identifier": {
52 yield instrValue;
53 break;
@@ -146,6 +157,17 @@ export function mapInstructionOperands(
157 instrValue.args = instrValue.args.map((arg) => fn(arg));
158 break;
159 }
160 + case "PropertyCall": {
161 + instrValue.receiver = fn(instrValue.receiver);
162 + instrValue.args = instrValue.args.map((arg) => fn(arg));
163 + break;
164 + }
165 + case "ComputedCall": {
166 + instrValue.receiver = fn(instrValue.receiver);
167 + instrValue.property = fn(instrValue.property);
168 + instrValue.args = instrValue.args.map((arg) => fn(arg));
169 + break;
170 + }
171 case "UnaryExpression": {
172 instrValue.value = fn(instrValue.value);
173 break;
compiler/forget/src/Inference/InferMutableLifetimes.ts
+2 -2
@@ -6,7 +6,6 @@
6 */
7
8 import invariant from "invariant";
9 -import { assertExhaustive } from "../Utils/utils";
9 import {
10 Effect,
11 HIRFunction,
@@ -16,6 +15,7 @@ import {
15 } from "../HIR/HIR";
16 import { printInstruction, printPlace } from "../HIR/PrintHIR";
17 import { eachInstructionOperand } from "../HIR/visitors";
18 +import { assertExhaustive } from "../Utils/utils";
19
20 /**
21 * For each usage of a value in the given function, determines if the usage
@@ -72,7 +72,7 @@ function inferPlace(
72 switch (place.effect) {
73 case Effect.Unknown: {
74 throw new Error(
75 - `Found an unkown place ${printPlace(place)} at ${printInstruction(
75 + `Found an unknown place ${printPlace(place)} at ${printInstruction(
76 instr
77 )}!`
78 );
compiler/forget/src/Inference/InferReferenceEffects.ts
+43
@@ -583,6 +583,49 @@ function inferBlock(env: Environment, block: BasicBlock) {
583 valueKind = ValueKind.Immutable;
584 break;
585 }
586 + case "PropertyCall": {
587 + if (!env.isDefined(instrValue.receiver)) {
588 + // TODO @josephsavona: improve handling of globals
589 + const value: InstructionValue = {
590 + kind: "Primitive",
591 + loc: instrValue.loc,
592 + value: undefined,
593 + };
594 + env.initialize(value, ValueKind.Frozen);
595 + env.define(instrValue.receiver, value);
596 + }
597 +
598 + env.reference(instrValue.receiver, Effect.Mutate);
599 + for (const arg of instrValue.args) {
600 + env.reference(arg, Effect.Mutate);
601 + }
602 + env.initialize(instrValue, ValueKind.Mutable);
603 + env.define(instr.lvalue.place, instrValue);
604 + instr.lvalue.place.effect = Effect.Mutate;
605 + continue;
606 + }
607 + case "ComputedCall": {
608 + if (!env.isDefined(instrValue.receiver)) {
609 + // TODO @josephsavona: improve handling of globals
610 + const value: InstructionValue = {
611 + kind: "Primitive",
612 + loc: instrValue.loc,
613 + value: undefined,
614 + };
615 + env.initialize(value, ValueKind.Frozen);
616 + env.define(instrValue.receiver, value);
617 + }
618 +
619 + env.reference(instrValue.receiver, Effect.Mutate);
620 + env.reference(instrValue.property, Effect.Read);
621 + for (const arg of instrValue.args) {
622 + env.reference(arg, Effect.Mutate);
623 + }
624 + env.initialize(instrValue, ValueKind.Mutable);
625 + env.define(instr.lvalue.place, instrValue);
626 + instr.lvalue.place.effect = Effect.Mutate;
627 + continue;
628 + }
629 case "PropertyStore": {
630 const effect = isObjectType(instrValue.object.identifier)
631 ? Effect.Store
compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+18
@@ -518,6 +518,24 @@ function codegenInstructionValue(
518 value = createCallExpression(instrValue.loc, callee, args);
519 break;
520 }
521 + case "PropertyCall": {
522 + const receiver = codegenPlace(temp, instrValue.receiver);
523 + const callee = t.memberExpression(
524 + receiver,
525 + t.identifier(instrValue.property)
526 + );
527 + const args = instrValue.args.map((arg) => codegenPlace(temp, arg));
528 + value = createCallExpression(instrValue.loc, callee, args);
529 + break;
530 + }
531 + case "ComputedCall": {
532 + const receiver = codegenPlace(temp, instrValue.receiver);
533 + const property = codegenPlace(temp, instrValue.property);
534 + const callee = t.memberExpression(receiver, property, true);
535 + const args = instrValue.args.map((arg) => codegenPlace(temp, arg));
536 + value = createCallExpression(instrValue.loc, callee, args);
537 + break;
538 + }
539 case "NewExpression": {
540 const callee = codegenPlace(temp, instrValue.callee);
541 const args = instrValue.args.map((arg) => codegenPlace(temp, arg));
compiler/forget/src/ReactiveScopes/InferReactiveScopeVariables.ts
+2
@@ -175,6 +175,8 @@ function mayAllocate(value: InstructionValue): boolean {
175 case "Primitive": {
176 return false;
177 }
178 + case "PropertyCall":
179 + case "ComputedCall":
180 case "PropertyStore":
181 case "ComputedStore":
182 case "ArrayExpression":
compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+4 -5
@@ -271,11 +271,10 @@ function visitInstructionValue(
271 value: InstructionValue,
272 lvalue: LValue | null
273 ): void {
274 - for (const operand of eachInstructionValueOperand(value)) {
275 - // check for method invocation, we want to depend on the callee, not the method
276 - if (value.kind === "PropertyLoad" && lvalue !== null) {
277 - context.declareProperty(lvalue.place, value.object, value.property);
278 - } else {
274 + if (value.kind === "PropertyLoad" && lvalue !== null) {
275 + context.declareProperty(lvalue.place, value.object, value.property);
276 + } else {
277 + for (const operand of eachInstructionValueOperand(value)) {
278 context.visitOperand(operand);
279 }
280 }
compiler/forget/src/__tests__/fixtures/hir/component.expect.md
+28 -30
@@ -39,21 +39,20 @@ function Component(props) {
39 const items = props.items;
40 const maxItems = props.maxItems;
41 const c_0 = $[0] !== maxItems;
42 - const c_1 = $[1] !== items.length;
43 - const c_2 = $[2] !== items.at;
42 + const c_1 = $[1] !== items;
43 let renderedItems;
45 - if (c_0 || c_1 || c_2) {
44 + if (c_0 || c_1) {
45 renderedItems = [];
46 const seen = new Set();
48 - const c_4 = $[4] !== maxItems;
47 + const c_3 = $[3] !== maxItems;
48 let max;
49
51 - if (c_4) {
50 + if (c_3) {
51 max = Math.max(0, maxItems);
53 - $[4] = maxItems;
54 - $[5] = max;
52 + $[3] = maxItems;
53 + $[4] = max;
54 } else {
56 - max = $[5];
55 + max = $[4];
56 }
57
58 for (let i = 0; i < items.length; i = i + 1, i) {
@@ -76,44 +75,43 @@ function Component(props) {
75 }
76
77 $[0] = maxItems;
79 - $[1] = items.length;
80 - $[2] = items.at;
81 - $[3] = renderedItems;
78 + $[1] = items;
79 + $[2] = renderedItems;
80 } else {
83 - renderedItems = $[3];
81 + renderedItems = $[2];
82 }
83
84 const count = renderedItems.length;
87 - const c_6 = $[6] !== count;
88 - let t7;
85 + const c_5 = $[5] !== count;
86 + let t6;
87
90 - if (c_6) {
91 - t7 = <h1>{count} Items</h1>;
92 - $[6] = count;
93 - $[7] = t7;
88 + if (c_5) {
89 + t6 = <h1>{count} Items</h1>;
90 + $[5] = count;
91 + $[6] = t6;
92 } else {
95 - t7 = $[7];
93 + t6 = $[6];
94 }
95
98 - const c_8 = $[8] !== t7;
99 - const c_9 = $[9] !== renderedItems;
100 - let t10;
96 + const c_7 = $[7] !== t6;
97 + const c_8 = $[8] !== renderedItems;
98 + let t9;
99
102 - if (c_8 || c_9) {
103 - t10 = (
100 + if (c_7 || c_8) {
101 + t9 = (
102 <div>
105 - {t7}
103 + {t6}
104 {renderedItems}
105 </div>
106 );
109 - $[8] = t7;
110 - $[9] = renderedItems;
111 - $[10] = t10;
107 + $[7] = t6;
108 + $[8] = renderedItems;
109 + $[9] = t9;
110 } else {
113 - t10 = $[10];
111 + t9 = $[9];
112 }
113
116 - return t10;
114 + return t9;
115 }
116
117 ```
compiler/forget/src/__tests__/fixtures/hir/method-call-computed.expect.md new
+70
@@ -0,0 +1,70 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function foo(a, b, c) {
6 + // Construct and freeze x, y
7 + const x = makeObject(a);
8 + const y = makeObject(a);
9 + <div>
10 + {x}
11 + {y}
12 + </div>;
13 +
14 + // z should depend on `x`, `y.method`, and `b`
15 + const z = x[y.method](b);
16 + return z;
17 +}
18 +
19 +```
20 +
21 +## Code
22 +
23 +```javascript
24 +function foo(a, b, c) {
25 + const $ = React.useMemoCache();
26 + const c_0 = $[0] !== a;
27 + let x;
28 + if (c_0) {
29 + x = makeObject(a);
30 + $[0] = a;
31 + $[1] = x;
32 + } else {
33 + x = $[1];
34 + }
35 +
36 + const c_2 = $[2] !== a;
37 + let y;
38 +
39 + if (c_2) {
40 + y = makeObject(a);
41 + $[2] = a;
42 + $[3] = y;
43 + } else {
44 + y = $[3];
45 + }
46 +
47 + <div>
48 + {x}
49 + {y}
50 + </div>;
51 + const c_4 = $[4] !== x;
52 + const c_5 = $[5] !== y.method;
53 + const c_6 = $[6] !== b;
54 + let z;
55 +
56 + if (c_4 || c_5 || c_6) {
57 + z = x[y.method](b);
58 + $[4] = x;
59 + $[5] = y.method;
60 + $[6] = b;
61 + $[7] = z;
62 + } else {
63 + z = $[7];
64 + }
65 +
66 + return z;
67 +}
68 +
69 +```
70 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/method-call-computed.js new
+13
@@ -0,0 +1,13 @@
1 +function foo(a, b, c) {
2 + // Construct and freeze x, y
3 + const x = makeObject(a);
4 + const y = makeObject(a);
5 + <div>
6 + {x}
7 + {y}
8 + </div>;
9 +
10 + // z should depend on `x`, `y.method`, and `b`
11 + const z = x[y.method](b);
12 + return z;
13 +}
compiler/forget/src/__tests__/fixtures/hir/method-call-fn-call.expect.md new
+54
@@ -0,0 +1,54 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function foo(a, b, c) {
6 + // Construct and freeze x
7 + const x = makeObject(a);
8 + <div>{x}</div>;
9 +
10 + // y should depend on `x` and `b`
11 + const method = x.method;
12 + const y = method.call(x, b);
13 + return y;
14 +}
15 +
16 +```
17 +
18 +## Code
19 +
20 +```javascript
21 +function foo(a, b, c) {
22 + const $ = React.useMemoCache();
23 + const c_0 = $[0] !== a;
24 + let x;
25 + if (c_0) {
26 + x = makeObject(a);
27 + $[0] = a;
28 + $[1] = x;
29 + } else {
30 + x = $[1];
31 + }
32 +
33 + <div>{x}</div>;
34 + const method = x.method;
35 + const c_2 = $[2] !== method;
36 + const c_3 = $[3] !== x;
37 + const c_4 = $[4] !== b;
38 + let y;
39 +
40 + if (c_2 || c_3 || c_4) {
41 + y = method.call(x, b);
42 + $[2] = method;
43 + $[3] = x;
44 + $[4] = b;
45 + $[5] = y;
46 + } else {
47 + y = $[5];
48 + }
49 +
50 + return y;
51 +}
52 +
53 +```
54 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/method-call-fn-call.js new
+10
@@ -0,0 +1,10 @@
1 +function foo(a, b, c) {
2 + // Construct and freeze x
3 + const x = makeObject(a);
4 + <div>{x}</div>;
5 +
6 + // y should depend on `x` and `b`
7 + const method = x.method;
8 + const y = method.call(x, b);
9 + return y;
10 +}
compiler/forget/src/__tests__/fixtures/hir/method-call.expect.md new
+50
@@ -0,0 +1,50 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function foo(a, b, c) {
6 + // Construct and freeze x
7 + const x = makeObject(a);
8 + <div>{x}</div>;
9 +
10 + // y should depend on `x` and `b`
11 + const y = x.foo(b);
12 + return y;
13 +}
14 +
15 +```
16 +
17 +## Code
18 +
19 +```javascript
20 +function foo(a, b, c) {
21 + const $ = React.useMemoCache();
22 + const c_0 = $[0] !== a;
23 + let x;
24 + if (c_0) {
25 + x = makeObject(a);
26 + $[0] = a;
27 + $[1] = x;
28 + } else {
29 + x = $[1];
30 + }
31 +
32 + <div>{x}</div>;
33 + const c_2 = $[2] !== x;
34 + const c_3 = $[3] !== b;
35 + let y;
36 +
37 + if (c_2 || c_3) {
38 + y = x.foo(b);
39 + $[2] = x;
40 + $[3] = b;
41 + $[4] = y;
42 + } else {
43 + y = $[4];
44 + }
45 +
46 + return y;
47 +}
48 +
49 +```
50 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/method-call.js new
+9
@@ -0,0 +1,9 @@
1 +function foo(a, b, c) {
2 + // Construct and freeze x
3 + const x = makeObject(a);
4 + <div>{x}</div>;
5 +
6 + // y should depend on `x` and `b`
7 + const y = x.foo(b);
8 + return y;
9 +}