Use single name resolver for nested functions
This is a prerequisite to inlining `useMemo()` lambdas so that we can better optimize them. Nested functions are evaluated with a fresh HIRBuilder, which means that they currently have their own `bindings` object for mapping identifier instances to IdentifierIds. This means that identifier ids in a closure are _always_ different that those outside the closure, even when they refer to the same identifier: ``` function Component(props) { props; // becomes e.g. props$1 const onClick = () => { props // becomes e.g. props$2 }; } ``` For useMemo inlining this is problematic because we've lost the association that these identifiers actually refer to the same thing. This PR changes that, sharing the name resolution data structure between the top-level function and any nested function expressions.
Joe Savona committed
Apr 6, 2023 at 15:25 UTC
5622c0ee91b3f011c735e4addc4693ff92ee8370
10 files changed
+91
-16
compiler/forget/src/CompilerPipeline.ts
+2
-1
@@ -56,7 +56,8 @@ export function* run(
56
func: NodePath<t.FunctionDeclaration>,
57
config?: EnvironmentConfig | null
58
): Generator<CompilerPipelineValue, t.FunctionDeclaration> {
59
- const hir = lower(func, new Environment(config ?? null)).unwrap();
59
+ const env = new Environment(config ?? null);
60
+ const hir = lower(func, env).unwrap();
61
yield log({ kind: "hir", name: "HIR", value: hir });
62
63
mergeConsecutiveBlocks(hir);
compiler/forget/src/HIR/BuildHIR.ts
+4
-2
@@ -36,7 +36,7 @@ import {
36
SpreadPattern,
37
ThrowTerminal,
38
} from "./HIR";
39
-import HIRBuilder from "./HIRBuilder";
39
+import HIRBuilder, { Bindings } from "./HIRBuilder";
40
41
// *******************************************************************************************
42
// *******************************************************************************************
@@ -58,11 +58,12 @@ import HIRBuilder from "./HIRBuilder";
58
export function lower(
59
func: NodePath<t.Function>,
60
env: Environment,
61
+ bindings: Bindings | null = null,
62
capturedRefs: t.Identifier[] = [],
63
// the outermost function being compiled, in case lower() is called recursively (for lambdas)
64
parent: NodePath<t.Function> | null = null
65
): Result<HIRFunction, CompilerError> {
65
- const builder = new HIRBuilder(env, parent ?? func, capturedRefs);
66
+ const builder = new HIRBuilder(env, parent ?? func, bindings, capturedRefs);
67
const context: Place[] = [];
68
69
for (const ref of capturedRefs ?? []) {
@@ -2134,6 +2135,7 @@ function lowerFunctionExpression(
2135
const lowering = lower(
2136
expr,
2137
builder.environment,
2138
+ builder.bindings,
2139
[...builder.context, ...captured.identifiers],
2140
builder.parentFunction
2141
);
compiler/forget/src/HIR/HIRBuilder.ts
+14
-4
@@ -71,6 +71,11 @@ function newBlock(id: BlockId, kind: BlockKind): WipBlock {
71
return { id, kind, instructions: [] };
72
}
73
74
+export type Bindings = Map<
75
+ string,
76
+ { node: t.Identifier; identifier: Identifier }
77
+>;
78
+
79
/**
80
* Helper class for constructing a CFG
81
*/
@@ -80,8 +85,7 @@ export default class HIRBuilder {
85
#entry: BlockId;
86
#scopes: Array<Scope> = [];
87
#context: t.Identifier[];
83
- #bindings: Map<string, { node: t.Identifier; identifier: Identifier }> =
84
- new Map();
88
+ #bindings: Bindings;
89
#env: Environment;
90
parentFunction: NodePath<t.Function>;
91
errors: CompilerError = new CompilerError();
@@ -94,6 +98,10 @@ export default class HIRBuilder {
98
return this.#context;
99
}
100
101
+ get bindings(): Bindings {
102
+ return this.#bindings;
103
+ }
104
+
105
get environment(): Environment {
106
return this.#env;
107
}
@@ -101,11 +109,13 @@ export default class HIRBuilder {
109
constructor(
110
env: Environment,
111
parentFunction: NodePath<t.Function>, // the outermost function being compiled
104
- context: t.Identifier[]
112
+ bindings: Bindings | null = null,
113
+ context: t.Identifier[] | null = null
114
) {
115
this.#env = env;
116
+ this.#bindings = bindings ?? new Map();
117
this.parentFunction = parentFunction;
108
- this.#context = context;
118
+ this.#context = context ?? [];
119
this.#entry = makeBlockId(env.nextBlockId);
120
this.#current = newBlock(this.#entry, "block");
121
}
compiler/forget/src/__tests__/fixtures/compiler/capturing-func-alias-receiver-computed-mutate.expect.md
+2
-2
@@ -26,8 +26,8 @@ function component(a) {
26
const x = { a };
27
y = {};
28
(function () {
29
- let a = y;
30
- a["x"] = x;
29
+ let a_0 = y;
30
+ a_0["x"] = x;
31
})();
32
mutate(y);
33
$[0] = a;
compiler/forget/src/__tests__/fixtures/compiler/capturing-func-alias-receiver-mutate.expect.md
+2
-2
@@ -26,8 +26,8 @@ function component(a) {
26
const x = { a };
27
y = {};
28
(function () {
29
- let a = y;
30
- a.x = x;
29
+ let a_0 = y;
30
+ a_0.x = x;
31
})();
32
mutate(y);
33
$[0] = a;
compiler/forget/src/__tests__/fixtures/compiler/capturing-function-member-expr-call.expect.md
+2
-2
@@ -19,9 +19,9 @@ function component({ mutator }) {
19
## Code
20
21
```javascript
22
-function component(t26) {
22
+function component(t24) {
23
const $ = React.unstable_useMemoCache(7);
24
- const { mutator } = t26;
24
+ const { mutator } = t24;
25
const c_0 = $[0] !== mutator;
26
let t0;
27
if (c_0) {
compiler/forget/src/__tests__/fixtures/compiler/capturing-function-shadow-captured.expect.md
+2
-2
@@ -21,8 +21,8 @@ function component(a) {
21
let t0;
22
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
23
t0 = function () {
24
- let z;
25
- mutate(z);
24
+ let z_0;
25
+ mutate(z_0);
26
};
27
$[0] = t0;
28
} else {
compiler/forget/src/__tests__/fixtures/compiler/inadvertent-mutability-readonly-lambda.expect.md
+1
-1
@@ -27,7 +27,7 @@ function Component(props) {
27
const c_0 = $[0] !== setValue;
28
let t0;
29
if (c_0) {
30
- t0 = (e) => setValue((value) => value + e.target.value);
30
+ t0 = (e) => setValue((value_0) => value_0 + e.target.value);
31
$[0] = setValue;
32
$[1] = t0;
33
} else {
compiler/forget/src/__tests__/fixtures/compiler/nested-function-shadowed-identifiers.expect.md
new
+52
@@ -0,0 +1,52 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ const [x, setX] = useState(null);
7
+
8
+ const onChange = (e) => {
9
+ let x = null; // intentionally shadow the original x
10
+ setX((currentX) => currentX + x); // intentionally refer to shadowed x
11
+ };
12
+
13
+ return <input value={x} onChange={onChange} />;
14
+}
15
+
16
+```
17
+
18
+## Code
19
+
20
+```javascript
21
+function Component(props) {
22
+ const $ = React.unstable_useMemoCache(5);
23
+ const [x, setX] = useState(null);
24
+ const c_0 = $[0] !== setX;
25
+ let t0;
26
+ if (c_0) {
27
+ t0 = (e) => {
28
+ let x_0 = null; // intentionally shadow the original x
29
+ setX((currentX) => currentX + x_0); // intentionally refer to shadowed x
30
+ };
31
+ $[0] = setX;
32
+ $[1] = t0;
33
+ } else {
34
+ t0 = $[1];
35
+ }
36
+ const onChange = t0;
37
+ const c_2 = $[2] !== x;
38
+ const c_3 = $[3] !== onChange;
39
+ let t1;
40
+ if (c_2 || c_3) {
41
+ t1 = <input value={x} onChange={onChange} />;
42
+ $[2] = x;
43
+ $[3] = onChange;
44
+ $[4] = t1;
45
+ } else {
46
+ t1 = $[4];
47
+ }
48
+ return t1;
49
+}
50
+
51
+```
52
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/nested-function-shadowed-identifiers.js
new
+10
@@ -0,0 +1,10 @@
1
+function Component(props) {
2
+ const [x, setX] = useState(null);
3
+
4
+ const onChange = (e) => {
5
+ let x = null; // intentionally shadow the original x
6
+ setX((currentX) => currentX + x); // intentionally refer to shadowed x
7
+ };
8
+
9
+ return <input value={x} onChange={onChange} />;
10
+}