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

[λ] Run analyseFunctions on inner func exprs

Previously we would skip out on calling analyseFunctions on inner function exprs so we missed out on catching aliased mutation in a different scope.

Sathya Gunasekaran committed Feb 2, 2023 at 14:05 UTC aa2403e06883f023c5031666d8970bef2fe0b013
6 files changed +156 -48
compiler/forget/src/HIR/HIR.ts
+12 -11
@@ -440,17 +440,7 @@ export type InstructionData =
440 | { kind: "ComputedStore"; object: Place; property: Place; value: Place }
441 // load `object[index]` - like PropertyLoad but with a dynamic property
442 | { kind: "ComputedLoad"; object: Place; property: Place }
443 - | {
444 - kind: "FunctionExpression";
445 - name: string | null;
446 - params: Array<string>;
447 - dependencies: Array<Place>;
448 - // TODO(gsn): Remove this mutatedDeps array and use dependencies as single
449 - // source of truth.
450 - mutatedDeps: Array<Place>;
451 - loweredFunc: HIRFunction;
452 - expr: t.ArrowFunctionExpression | t.FunctionExpression;
453 - }
443 + | FunctionExpression
444 | {
445 kind: "TaggedTemplateExpression";
446 tag: Place;
@@ -471,6 +461,17 @@ export type JsxAttribute =
461 | { kind: "JsxSpreadAttribute"; argument: Place }
462 | { kind: "JsxAttribute"; name: string; place: Place };
463
464 +export type FunctionExpression = {
465 + kind: "FunctionExpression";
466 + name: string | null;
467 + params: Array<string>;
468 + dependencies: Array<Place>;
469 + // TODO(gsn): Remove this mutatedDeps array and use dependencies as single
470 + // source of truth.
471 + mutatedDeps: Array<Place>;
472 + loweredFunc: HIRFunction;
473 + expr: t.ArrowFunctionExpression | t.FunctionExpression;
474 +};
475 /**
476 * A place where data may be read from / written to:
477 * - a variable (identifier)
compiler/forget/src/Inference/AnalyseFunctions.ts
+41 -37
@@ -1,4 +1,10 @@
1 -import { HIRFunction, Identifier, mergeConsecutiveBlocks, Place } from "../HIR";
1 +import {
2 + HIRFunction,
3 + FunctionExpression,
4 + Identifier,
5 + mergeConsecutiveBlocks,
6 + Place,
7 +} from "../HIR";
8 import { constantPropagation } from "../Optimization";
9 import { eliminateRedundantPhi, enterSSA } from "../SSA";
10 import { inferTypes } from "../TypeInference";
@@ -30,18 +36,15 @@ function declareProperty(
36 properties.set(lvalue.identifier, nextDependency);
37 }
38
33 -export default function (func: HIRFunction) {
39 +export default function analyseFunctions(func: HIRFunction) {
40 const properties: Map<Identifier, Dependency> = new Map();
41
42 for (const [_, block] of func.body.blocks) {
43 for (const instr of block.instructions) {
44 switch (instr.value.kind) {
45 case "FunctionExpression": {
40 - instr.value.mutatedDeps = buildMutatedDeps(
41 - analyzeMutatedPlaces(instr.value.loweredFunc),
42 - instr.value.dependencies,
43 - properties
44 - );
46 + lower(instr.value.loweredFunc);
47 + infer(instr.value, properties);
48 break;
49 }
50 case "PropertyLoad": {
@@ -57,6 +60,37 @@ export default function (func: HIRFunction) {
60 }
61 }
62
63 +function lower(func: HIRFunction) {
64 + mergeConsecutiveBlocks(func);
65 + enterSSA(func);
66 + eliminateRedundantPhi(func);
67 + constantPropagation(func);
68 + inferTypes(func);
69 + analyseFunctions(func);
70 + inferReferenceEffects(func);
71 + inferMutableRanges(func);
72 + logHIRFunction("AnalyseFunction (inner)", func);
73 +}
74 +
75 +function infer(
76 + value: FunctionExpression,
77 + properties: Map<Identifier, Dependency>
78 +) {
79 + const func = value.loweredFunc;
80 + const mutations: Array<Place> = func.context.filter((dep) =>
81 + isMutated(dep.identifier)
82 + );
83 + value.mutatedDeps = buildMutatedDeps(
84 + mutations,
85 + value.dependencies,
86 + properties
87 + );
88 +}
89 +
90 +function isMutated(id: Identifier) {
91 + return id.mutableRange.end - id.mutableRange.start > 1;
92 +}
93 +
94 function buildMutatedDeps(
95 mutations: Place[],
96 capturedDeps: Place[],
@@ -89,33 +123,3 @@ function buildMutatedDeps(
123
124 return mutatedDeps;
125 }
92 -
93 -function analyzeMutatedPlaces(func: HIRFunction): Array<Place> {
94 - mergeConsecutiveBlocks(func);
95 - enterSSA(func);
96 - eliminateRedundantPhi(func);
97 - constantPropagation(func);
98 - inferTypes(func);
99 - inferReferenceEffects(func);
100 - inferMutableRanges(func);
101 - logHIRFunction("AnalyseFunction (inner)", func);
102 -
103 - const mutations: Array<Place> = [];
104 - for (const [_, block] of func.body.blocks) {
105 - for (const instr of block.instructions) {
106 - if (
107 - instr.value.kind === "FunctionExpression" &&
108 - instr.value.loweredFunc !== null
109 - ) {
110 - mutations.push(...analyzeMutatedPlaces(instr.value.loweredFunc));
111 - }
112 - }
113 - }
114 -
115 - mutations.push(...func.context.filter((dep) => isMutated(dep.identifier)));
116 - return mutations;
117 -}
118 -
119 -function isMutated(id: Identifier) {
120 - return id.mutableRange.end - id.mutableRange.start > 1;
121 -}
compiler/forget/src/__tests__/fixtures/hir/_bug_capture-mutate-across-fns.expect.md new
+40
@@ -0,0 +1,40 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function component(a) {
6 + let z = { a };
7 + (function () {
8 + (function () {
9 + z.b = 1;
10 + })();
11 + })();
12 + return z;
13 +}
14 +
15 +```
16 +
17 +## Code
18 +
19 +```javascript
20 +function component(a) {
21 + const $ = React.useMemoCache();
22 + const c_0 = $[0] !== a;
23 + let z;
24 + if (c_0) {
25 + z = { a: a };
26 + $[0] = a;
27 + $[1] = z;
28 + } else {
29 + z = $[1];
30 + }
31 + (function () {
32 + (function () {
33 + z.b = 1;
34 + })();
35 + })();
36 + return z;
37 +}
38 +
39 +```
40 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/_bug_capture-mutate-across-fns.js new
+9
@@ -0,0 +1,9 @@
1 +function component(a) {
2 + let z = { a };
3 + (function () {
4 + (function () {
5 + z.b = 1;
6 + })();
7 + })();
8 + return z;
9 +}
compiler/forget/src/__tests__/fixtures/hir/capture-indirect-mutate-alias.expect.md new
+43
@@ -0,0 +1,43 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function component(a) {
6 + let x = { a };
7 + (function () {
8 + let q = x;
9 + (function () {
10 + q.b = 1;
11 + })();
12 + })();
13 +
14 + return x;
15 +}
16 +
17 +```
18 +
19 +## Code
20 +
21 +```javascript
22 +function component(a) {
23 + const $ = React.useMemoCache();
24 + const c_0 = $[0] !== a;
25 + let x;
26 + if (c_0) {
27 + x = { a: a };
28 + (function () {
29 + let q = x;
30 + (function () {
31 + q.b = 1;
32 + })();
33 + })();
34 + $[0] = a;
35 + $[1] = x;
36 + } else {
37 + x = $[1];
38 + }
39 + return x;
40 +}
41 +
42 +```
43 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/capture-indirect-mutate-alias.js new
+11
@@ -0,0 +1,11 @@
1 +function component(a) {
2 + let x = { a };
3 + (function () {
4 + let q = x;
5 + (function () {
6 + q.b = 1;
7 + })();
8 + })();
9 +
10 + return x;
11 +}