Promote and rename within nested functions
Another title for this PR could be "Yet another reason for HIR-everywhere" ReactiveFunctionVisitor doesn't traverse into HIRFunctions from FunctionExpression and ObjectMethod values. This means that PromoteUsedTemporaries and RenameVariables also weren't traversing into such functions, and those values weren't getting promoted and renamed correctly. This PR updates ReactiveFunctionVisitor with a method that can optionally be invoked to traverse an HIRFunction and call the appropriate visitor methods. PromoteUsedTemporaries and RenameVariables invoke this to ensure they visit all places, even in nested HIRFunctions.
Joe Savona committed
Mar 6, 2024 at 11:07 UTC
2ae0f36543e3306bc4b72de01a1ad8554c9176bf
8 files changed
+120
-16
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+1
-8
@@ -6,12 +6,7 @@
6
*/
7
8
import * as t from "@babel/types";
9
-import {
10
- pruneHoistedContexts,
11
- pruneUnusedLValues,
12
- pruneUnusedLabels,
13
- renameVariables,
14
-} from ".";
9
+import { pruneHoistedContexts, pruneUnusedLValues, pruneUnusedLabels } from ".";
10
import { CompilerError, ErrorSeverity } from "../CompilerError";
11
import { Environment, EnvironmentConfig, ExternalFunction } from "../HIR";
12
import {
@@ -1342,7 +1337,6 @@ function codegenInstructionValue(
1337
const reactiveFunction = buildReactiveFunction(loweredFunc.func);
1338
pruneUnusedLabels(reactiveFunction);
1339
pruneUnusedLValues(reactiveFunction);
1345
- renameVariables(reactiveFunction);
1340
const fn = codegenReactiveFunction(
1341
new Context(
1342
cx.env,
@@ -1547,7 +1541,6 @@ function codegenInstructionValue(
1541
const reactiveFunction = buildReactiveFunction(loweredFunc);
1542
pruneUnusedLabels(reactiveFunction);
1543
pruneUnusedLValues(reactiveFunction);
1550
- renameVariables(reactiveFunction);
1544
pruneHoistedContexts(reactiveFunction);
1545
const fn = codegenReactiveFunction(
1546
new Context(cx.env, reactiveFunction.id ?? "[[ anonymous ]]", cx.temp),
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PromoteUsedTemporaries.ts
+7
-6
@@ -46,6 +46,12 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
46
}
47
}
48
49
+ override visitParam(place: Place, state: VisitorState): void {
50
+ if (place.identifier.name === null) {
51
+ promoteTemporary(place.identifier, state);
52
+ }
53
+ }
54
+
55
override visitValue(
56
id: InstructionId,
57
value: ReactiveValue,
@@ -53,12 +59,7 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
59
): void {
60
this.traverseValue(id, value, state);
61
if (value.kind === "FunctionExpression" || value.kind === "ObjectMethod") {
56
- for (const operand of value.loweredFunc.func.params) {
57
- const place = operand.kind === "Identifier" ? operand : operand.place;
58
- if (place.identifier.name === null) {
59
- promoteTemporary(place.identifier, state);
60
- }
61
- }
62
+ this.visitHirFunction(value.loweredFunc.func, state);
63
}
64
}
65
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/RenameVariables.ts
+15
@@ -15,6 +15,7 @@ import {
15
ReactiveBlock,
16
ReactiveFunction,
17
ReactiveScopeBlock,
18
+ ReactiveValue,
19
isPromotedJsxTemporary,
20
isPromotedTemporary,
21
makeIdentifierName,
@@ -62,6 +63,9 @@ function renameVariablesImpl(
63
}
64
65
class Visitor extends ReactiveFunctionVisitor<Scopes> {
66
+ override visitParam(place: Place, state: Scopes): void {
67
+ state.visit(place.identifier);
68
+ }
69
override visitLValue(_id: InstructionId, lvalue: Place, state: Scopes): void {
70
state.visit(lvalue.identifier);
71
}
@@ -81,6 +85,17 @@ class Visitor extends ReactiveFunctionVisitor<Scopes> {
85
this.traverseScope(scope, state);
86
}
87
88
+ override visitValue(
89
+ id: InstructionId,
90
+ value: ReactiveValue,
91
+ state: Scopes
92
+ ): void {
93
+ this.traverseValue(id, value, state);
94
+ if (value.kind === "FunctionExpression" || value.kind === "ObjectMethod") {
95
+ this.visitHirFunction(value.loweredFunc.func, state);
96
+ }
97
+ }
98
+
99
override visitReactiveFunctionValue(
100
_id: InstructionId,
101
_dependencies: Place[],
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/visitors.ts
+24
@@ -6,6 +6,7 @@
6
*/
7
8
import {
9
+ HIRFunction,
10
InstructionId,
11
Place,
12
ReactiveBlock,
@@ -20,6 +21,7 @@ import {
21
import {
22
eachInstructionLValue,
23
eachInstructionValueOperand,
24
+ eachTerminalOperand,
25
} from "../HIR/visitors";
26
import { assertExhaustive } from "../Utils/utils";
27
@@ -33,6 +35,7 @@ export function visitReactiveFunction<TState>(
35
36
export class ReactiveFunctionVisitor<TState = void> {
37
visitID(_id: InstructionId, _state: TState): void {}
38
+ visitParam(_place: Place, _state: TState): void {}
39
visitLValue(_id: InstructionId, _lvalue: Place, _state: TState): void {}
40
visitPlace(_id: InstructionId, _place: Place, _state: TState): void {}
41
visitReactiveFunctionValue(
@@ -219,6 +222,27 @@ export class ReactiveFunctionVisitor<TState = void> {
222
}
223
}
224
}
225
+
226
+ visitHirFunction(fn: HIRFunction, state: TState): void {
227
+ for (const param of fn.params) {
228
+ const place = param.kind === "Identifier" ? param : param.place;
229
+ this.visitParam(place, state);
230
+ }
231
+ for (const [, block] of fn.body.blocks) {
232
+ for (const instr of block.instructions) {
233
+ this.visitInstruction(instr, state);
234
+ if (
235
+ instr.value.kind === "FunctionExpression" ||
236
+ instr.value.kind === "ObjectMethod"
237
+ ) {
238
+ this.visitHirFunction(instr.value.loweredFunc.func, state);
239
+ }
240
+ }
241
+ for (const operand of eachTerminalOperand(block.terminal)) {
242
+ this.visitPlace(block.terminal.id, operand, state);
243
+ }
244
+ }
245
+ }
246
}
247
248
export type TransformedValue =
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/deeply-nested-function-expressions-with-params.expect.md
new
+54
@@ -0,0 +1,54 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Foo() {
6
+ return (function t() {
7
+ let x = {};
8
+ let y = {};
9
+ return function a(x = () => {}) {
10
+ return (function b(y = []) {
11
+ return [x, y];
12
+ })();
13
+ };
14
+ })();
15
+}
16
+
17
+export const FIXTURE_ENTRYPOINT = {
18
+ fn: Foo,
19
+ params: [],
20
+};
21
+
22
+```
23
+
24
+## Code
25
+
26
+```javascript
27
+import { unstable_useMemoCache as useMemoCache } from "react";
28
+function Foo() {
29
+ const $ = useMemoCache(1);
30
+ let t0;
31
+ let t1;
32
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
33
+ t1 = function a(t2) {
34
+ const x_0 = t2 === undefined ? () => {} : t2;
35
+ return (function b(t3) {
36
+ const y_0 = t3 === undefined ? [] : t3;
37
+ return [x_0, y_0];
38
+ })();
39
+ };
40
+ $[0] = t1;
41
+ } else {
42
+ t1 = $[0];
43
+ }
44
+ t0 = t1;
45
+ return t0;
46
+}
47
+
48
+export const FIXTURE_ENTRYPOINT = {
49
+ fn: Foo,
50
+ params: [],
51
+};
52
+
53
+```
54
+
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/deeply-nested-function-expressions-with-params.js
new
+16
@@ -0,0 +1,16 @@
1
+function Foo() {
2
+ return (function t() {
3
+ let x = {};
4
+ let y = {};
5
+ return function a(x = () => {}) {
6
+ return (function b(y = []) {
7
+ return [x, y];
8
+ })();
9
+ };
10
+ })();
11
+}
12
+
13
+export const FIXTURE_ENTRYPOINT = {
14
+ fn: Foo,
15
+ params: [],
16
+};
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/nested-function-with-param-as-captured-dep.expect.md
+2
-2
@@ -28,8 +28,8 @@ function Foo() {
28
let t0;
29
let t1;
30
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
31
- t1 = function a(t0) {
32
- const x_0 = t0 === undefined ? () => {} : t0;
31
+ t1 = function a(t2) {
32
+ const x_0 = t2 === undefined ? () => {} : t2;
33
return x_0;
34
};
35
$[0] = t1;
compiler/packages/snap/src/SproutTodoFilter.ts
+1
@@ -422,6 +422,7 @@ const skipFilter = new Set([
422
"component-declaration-basic.flow",
423
"hook-declaration-basic.flow",
424
"nested-function-with-param-as-captured-dep",
425
+ "deeply-nested-function-expressions-with-params",
426
"readonly-object-method-calls",
427
"readonly-object-method-calls-mutable-lambda",
428