Support reordering call expressions
Supports call expressions if the callee and args are themselves reorderable. As part of this I realized that we currently allow identifier references to be reordered. To be safe, this PR updates the logic to continue consider identifiers as reorderable, but considers an arrow function to be not reorderable if it contains a local variable reference. We can likely relax that rule, but this quickly unblocks the next experiment.
Joe Savona committed
Sep 11, 2023 at 12:49 UTC
1ec445d8f93de0f0577a55707a12b86f0b9d6d05
6 files changed
+156
-13
compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts
+60
-11
@@ -2214,7 +2214,7 @@ function lowerReorderableExpression(
2214
builder: HIRBuilder,
2215
expr: NodePath<t.Expression>
2216
): Place {
2217
- if (!isReorderableExpression(builder, expr)) {
2217
+ if (!isReorderableExpression(builder, expr, true)) {
2218
builder.errors.push({
2219
reason: `(BuildHIR::node.lowerReorderableExpression) Expression type '${expr.type}' cannot be safely reordered`,
2220
severity: ErrorSeverity.Todo,
@@ -2227,10 +2227,21 @@ function lowerReorderableExpression(
2227
2228
function isReorderableExpression(
2229
builder: HIRBuilder,
2230
- expr: NodePath<t.Expression>
2230
+ expr: NodePath<t.Expression>,
2231
+ allowLocalIdentifiers: boolean
2232
): boolean {
2233
switch (expr.node.type) {
2233
- case "Identifier":
2234
+ case "Identifier": {
2235
+ const identifier = builder.resolveIdentifier(
2236
+ expr as NodePath<t.Identifier>
2237
+ );
2238
+ if (identifier === null) {
2239
+ // global, definitely safe
2240
+ return true;
2241
+ } else {
2242
+ return allowLocalIdentifiers;
2243
+ }
2244
+ }
2245
case "RegExpLiteral":
2246
case "StringLiteral":
2247
case "NumericLiteral":
@@ -2245,7 +2256,11 @@ function isReorderableExpression(
2256
case "!":
2257
case "+":
2258
case "-": {
2248
- return isReorderableExpression(builder, unary.get("argument"));
2259
+ return isReorderableExpression(
2260
+ builder,
2261
+ unary.get("argument"),
2262
+ allowLocalIdentifiers
2263
+ );
2264
}
2265
default: {
2266
return false;
@@ -2255,15 +2270,28 @@ function isReorderableExpression(
2270
case "TypeCastExpression": {
2271
return isReorderableExpression(
2272
builder,
2258
- (expr as NodePath<t.TypeCastExpression>).get("expression")
2273
+ (expr as NodePath<t.TypeCastExpression>).get("expression"),
2274
+ allowLocalIdentifiers
2275
);
2276
}
2277
case "ConditionalExpression": {
2278
const conditional = expr as NodePath<t.ConditionalExpression>;
2279
return (
2264
- isReorderableExpression(builder, conditional.get("test")) &&
2265
- isReorderableExpression(builder, conditional.get("consequent")) &&
2266
- isReorderableExpression(builder, conditional.get("alternate"))
2280
+ isReorderableExpression(
2281
+ builder,
2282
+ conditional.get("test"),
2283
+ allowLocalIdentifiers
2284
+ ) &&
2285
+ isReorderableExpression(
2286
+ builder,
2287
+ conditional.get("consequent"),
2288
+ allowLocalIdentifiers
2289
+ ) &&
2290
+ isReorderableExpression(
2291
+ builder,
2292
+ conditional.get("alternate"),
2293
+ allowLocalIdentifiers
2294
+ )
2295
);
2296
}
2297
case "ArrayExpression": {
@@ -2271,7 +2299,8 @@ function isReorderableExpression(
2299
.get("elements")
2300
.every(
2301
(element) =>
2274
- element.isExpression() && isReorderableExpression(builder, element)
2302
+ element.isExpression() &&
2303
+ isReorderableExpression(builder, element, allowLocalIdentifiers)
2304
);
2305
}
2306
case "ObjectExpression": {
@@ -2283,7 +2312,8 @@ function isReorderableExpression(
2312
}
2313
const value = property.get("value");
2314
return (
2286
- value.isExpression() && isReorderableExpression(builder, value)
2315
+ value.isExpression() &&
2316
+ isReorderableExpression(builder, value, allowLocalIdentifiers)
2317
);
2318
});
2319
}
@@ -2315,9 +2345,28 @@ function isReorderableExpression(
2345
} else {
2346
// For TypeScript
2347
invariant(body.isExpression(), "Expected an expression");
2318
- return isReorderableExpression(builder, body);
2348
+ return isReorderableExpression(
2349
+ builder,
2350
+ body,
2351
+ /* disallow local identifiers in the body */ false
2352
+ );
2353
}
2354
}
2355
+ case "CallExpression": {
2356
+ const call = expr as NodePath<t.CallExpression>;
2357
+ const callee = call.get("callee");
2358
+ return (
2359
+ callee.isExpression() &&
2360
+ isReorderableExpression(builder, callee, allowLocalIdentifiers) &&
2361
+ call
2362
+ .get("arguments")
2363
+ .every(
2364
+ (arg) =>
2365
+ arg.isExpression() &&
2366
+ isReorderableExpression(builder, arg, allowLocalIdentifiers)
2367
+ )
2368
+ );
2369
+ }
2370
default: {
2371
return false;
2372
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/default-param-calls-global-function.expect.md
new
+45
@@ -0,0 +1,45 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import { identity } from "shared-runtime";
6
+
7
+function Component(x = identity([() => {}, true, 42, "hello"])) {
8
+ return x;
9
+}
10
+
11
+export const FIXTURE_ENTRYPOINT = {
12
+ fn: Component,
13
+ params: [],
14
+};
15
+
16
+```
17
+
18
+## Code
19
+
20
+```javascript
21
+import { unstable_useMemoCache as useMemoCache } from "react";
22
+import { identity } from "shared-runtime";
23
+
24
+function Component(t0) {
25
+ const $ = useMemoCache(2);
26
+ const c_0 = $[0] !== t0;
27
+ let t1;
28
+ if (c_0) {
29
+ t1 = t0 === undefined ? identity([() => {}, true, 42, "hello"]) : t0;
30
+ $[0] = t0;
31
+ $[1] = t1;
32
+ } else {
33
+ t1 = $[1];
34
+ }
35
+ const x = t1;
36
+ return x;
37
+}
38
+
39
+export const FIXTURE_ENTRYPOINT = {
40
+ fn: Component,
41
+ params: [],
42
+};
43
+
44
+```
45
+
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/default-param-calls-global-function.js
new
+10
@@ -0,0 +1,10 @@
1
+import { identity } from "shared-runtime";
2
+
3
+function Component(x = identity([() => {}, true, 42, "hello"])) {
4
+ return x;
5
+}
6
+
7
+export const FIXTURE_ENTRYPOINT = {
8
+ fn: Component,
9
+ params: [],
10
+};
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.default-param-accesses-local.expect.md
new
+28
@@ -0,0 +1,28 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(
6
+ x,
7
+ y = () => {
8
+ return x;
9
+ }
10
+) {
11
+ return y();
12
+}
13
+
14
+export const FIXTURE_ENTRYPOINT = {
15
+ fn: Component,
16
+ params: [],
17
+};
18
+
19
+```
20
+
21
+
22
+## Error
23
+
24
+```
25
+[ReactForget] Todo: (BuildHIR::node.lowerReorderableExpression) Expression type 'ArrowFunctionExpression' cannot be safely reordered (3:5)
26
+```
27
+
28
+
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.default-param-accesses-local.js
new
+13
@@ -0,0 +1,13 @@
1
+function Component(
2
+ x,
3
+ y = () => {
4
+ return x;
5
+ }
6
+) {
7
+ return y();
8
+}
9
+
10
+export const FIXTURE_ENTRYPOINT = {
11
+ fn: Component,
12
+ params: [],
13
+};
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-kitchensink.expect.md
-2
@@ -115,8 +115,6 @@ let moduleLocal = false;
115
116
[ReactForget] Todo: (BuildHIR::node.lowerReorderableExpression) Expression type 'MemberExpression' cannot be safely reordered (57:57)
117
118
-[ReactForget] Todo: (BuildHIR::node.lowerReorderableExpression) Expression type 'CallExpression' cannot be safely reordered (55:55)
119
-
118
[ReactForget] Todo: (BuildHIR::node.lowerReorderableExpression) Expression type 'BinaryExpression' cannot be safely reordered (53:53)
119
```
120