Skip to content

Commit 241a615

Browse files
committed
[babel][contextvar] Patch context identifier babel logic; only use referenced
identifiers --- A few fixes for finding context identifiers: Previously, we counted every babel identifier as a reference. This is problematic because babel counts every string symbol as an identifier. ```js print(x); // x is an identifier as expected obj.x // x is.. also an identifier here {x: 2} // x is also an identifier here ``` This PR adds a check for `isReferencedIdentifier`. Note that only non-lval references pass this check ```js print(x); // isReferencedIdentifier(x) -> true obj.x // isReferencedIdentifier(x) -> false {x: 2} // isReferencedIdentifier(x) -> false x = 2 // isReferencedIdentifier(x) -> false ``` Which brings us to change #2. Previously, we counted assignments as references due to the identifier visiting + checking logic. The logic was roughly the following (from facebook#1691) ```js contextVars = intersection(reassigned, referencedByInnerFn); ``` Now that assignments (lvals) and references (rvals) are tracked separately, the equivalent logic is this. Note that assignment to a context variable does not need to be modeled as a read (`console.log(x = 5)` always will evaluates and prints 5, regardless of the previous value of x). ``` contextVars = union(reassignedByInnerFn, intersection(reassigned, referencedByInnerFn)) ``` --- Note that variables that are never read do not need to be modeled as context variables, but this is unlikely to be a common pattern. ```js function fn() { let x = 2; const inner = () => { x = 3; } } ```
1 parent 8f18b82 commit 241a615

26 files changed

+656
-73
lines changed

compiler/packages/babel-plugin-react-forget/src/HIR/FindContextIdentifiers.ts

Lines changed: 65 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,28 @@
88
import type { NodePath } from "@babel/traverse";
99
import type * as t from "@babel/types";
1010
import { CompilerError } from "../CompilerError";
11-
import { Set_union } from "../Utils/utils";
11+
import { getOrInsertDefault } from "../Utils/utils";
1212
import { GeneratedSource } from "./HIR";
1313

14+
type IdentifierInfo = {
15+
reassigned: boolean;
16+
reassignedByInnerFn: boolean;
17+
referencedByInnerFn: boolean;
18+
};
19+
const DEFAULT_IDENTIFIER_INFO: IdentifierInfo = {
20+
reassigned: false,
21+
reassignedByInnerFn: false,
22+
referencedByInnerFn: false,
23+
};
24+
1425
type BabelFunction =
1526
| NodePath<t.FunctionDeclaration>
1627
| NodePath<t.FunctionExpression>
1728
| NodePath<t.ArrowFunctionExpression>
1829
| NodePath<t.ObjectMethod>;
1930
type FindContextIdentifierState = {
2031
currentFn: Array<BabelFunction>;
21-
reassigned: Set<t.Identifier>;
22-
referenced: Set<t.Identifier>;
32+
identifiers: Map<t.Identifier, IdentifierInfo>;
2333
};
2434

2535
const withFunctionScope = {
@@ -39,8 +49,7 @@ export function findContextIdentifiers(
3949
): Set<t.Identifier> {
4050
const state: FindContextIdentifierState = {
4151
currentFn: [],
42-
reassigned: new Set(),
43-
referenced: new Set(),
52+
identifiers: new Map(),
4453
};
4554

4655
func.traverse<FindContextIdentifierState>(
@@ -54,38 +63,59 @@ export function findContextIdentifiers(
5463
state: FindContextIdentifierState
5564
): void {
5665
const left = path.get("left");
57-
handleAssignment(state.reassigned, left);
66+
const currentFn = state.currentFn.at(-1) ?? null;
67+
handleAssignment(currentFn, state.identifiers, left);
5868
},
5969
Identifier(
6070
path: NodePath<t.Identifier>,
6171
state: FindContextIdentifierState
6272
): void {
63-
const currentFn = state.currentFn.at(-1);
64-
if (currentFn !== undefined)
65-
handleIdentifier(currentFn, state.referenced, path);
73+
const currentFn = state.currentFn.at(-1) ?? null;
74+
if (path.isReferencedIdentifier()) {
75+
handleIdentifier(currentFn, state.identifiers, path);
76+
}
6677
},
6778
},
6879
state
6980
);
70-
return Set_union(state.reassigned, state.referenced);
81+
82+
const result = new Set<t.Identifier>();
83+
for (const [id, info] of state.identifiers.entries()) {
84+
if (info.reassignedByInnerFn) {
85+
result.add(id);
86+
} else if (info.reassigned && info.referencedByInnerFn) {
87+
result.add(id);
88+
}
89+
}
90+
return result;
7191
}
7292

7393
function handleIdentifier(
74-
currentFn: BabelFunction,
75-
referenced: Set<t.Identifier>,
94+
currentFn: BabelFunction | null,
95+
identifiers: Map<t.Identifier, IdentifierInfo>,
7696
path: NodePath<t.Identifier>
7797
): void {
7898
const name = path.node.name;
7999
const binding = path.scope.getBinding(name);
80-
const bindingAboveLambdaScope = currentFn.scope.parent.getBinding(name);
100+
if (binding == null) {
101+
return;
102+
}
103+
const identifier = getOrInsertDefault(identifiers, binding.identifier, {
104+
...DEFAULT_IDENTIFIER_INFO,
105+
});
81106

82-
if (binding != null && binding === bindingAboveLambdaScope) {
83-
referenced.add(binding.identifier);
107+
if (currentFn != null) {
108+
const bindingAboveLambdaScope = currentFn.scope.parent.getBinding(name);
109+
110+
if (binding === bindingAboveLambdaScope) {
111+
identifier.referencedByInnerFn = true;
112+
}
84113
}
85114
}
86115

87116
function handleAssignment(
88-
reassigned: Set<t.Identifier>,
117+
currentFn: BabelFunction | null,
118+
identifiers: Map<t.Identifier, IdentifierInfo>,
89119
lvalPath: NodePath<t.LVal>
90120
): void {
91121
/*
@@ -98,16 +128,28 @@ function handleAssignment(
98128
const path = lvalPath as NodePath<t.Identifier>;
99129
const name = path.node.name;
100130
const binding = path.scope.getBinding(name);
101-
if (binding != null) {
102-
reassigned.add(binding.identifier);
131+
if (binding == null) {
132+
break;
133+
}
134+
const state = getOrInsertDefault(identifiers, binding.identifier, {
135+
...DEFAULT_IDENTIFIER_INFO,
136+
});
137+
state.reassigned = true;
138+
139+
if (currentFn != null) {
140+
const bindingAboveLambdaScope = currentFn.scope.parent.getBinding(name);
141+
142+
if (binding === bindingAboveLambdaScope) {
143+
state.reassignedByInnerFn = true;
144+
}
103145
}
104146
break;
105147
}
106148
case "ArrayPattern": {
107149
const path = lvalPath as NodePath<t.ArrayPattern>;
108150
for (const element of path.get("elements")) {
109151
if (nonNull(element)) {
110-
handleAssignment(reassigned, element);
152+
handleAssignment(currentFn, identifiers, element);
111153
}
112154
}
113155
break;
@@ -123,28 +165,28 @@ function handleAssignment(
123165
loc: valuePath.node.loc ?? GeneratedSource,
124166
suggestions: null,
125167
});
126-
handleAssignment(reassigned, valuePath);
168+
handleAssignment(currentFn, identifiers, valuePath);
127169
} else {
128170
CompilerError.invariant(property.isRestElement(), {
129171
reason: `[FindContextIdentifiers] Invalid assumptions for babel types.`,
130172
description: null,
131173
loc: property.node.loc ?? GeneratedSource,
132174
suggestions: null,
133175
});
134-
handleAssignment(reassigned, property);
176+
handleAssignment(currentFn, identifiers, property);
135177
}
136178
}
137179
break;
138180
}
139181
case "AssignmentPattern": {
140182
const path = lvalPath as NodePath<t.AssignmentPattern>;
141183
const left = path.get("left");
142-
handleAssignment(reassigned, left);
184+
handleAssignment(currentFn, identifiers, left);
143185
break;
144186
}
145187
case "RestElement": {
146188
const path = lvalPath as NodePath<t.RestElement>;
147-
handleAssignment(reassigned, path.get("argument"));
189+
handleAssignment(currentFn, identifiers, path.get("argument"));
148190
break;
149191
}
150192
case "MemberExpression": {

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capturing-function-alias-computed-load-3.expect.md

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@ function bar(a, b) {
1717

1818
export const FIXTURE_ENTRYPOINT = {
1919
fn: bar,
20-
params: ["TodoAdd"],
21-
isComponent: "TodoAdd",
20+
params: [
21+
[1, 2],
22+
[2, 3],
23+
],
2224
};
2325

2426
```
@@ -52,9 +54,13 @@ function bar(a, b) {
5254

5355
export const FIXTURE_ENTRYPOINT = {
5456
fn: bar,
55-
params: ["TodoAdd"],
56-
isComponent: "TodoAdd",
57+
params: [
58+
[1, 2],
59+
[2, 3],
60+
],
5761
};
5862

5963
```
60-
64+
65+
### Eval output
66+
(kind: ok) 2

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capturing-function-alias-computed-load-3.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ function bar(a, b) {
1313

1414
export const FIXTURE_ENTRYPOINT = {
1515
fn: bar,
16-
params: ["TodoAdd"],
17-
isComponent: "TodoAdd",
16+
params: [
17+
[1, 2],
18+
[2, 3],
19+
],
1820
};

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/chained-assignment-context-variable.expect.md

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,31 @@
22
## Input
33

44
```javascript
5+
import { makeArray } from "shared-runtime";
6+
57
function Component() {
68
let x,
79
y = (x = {});
810
const foo = () => {
9-
x = getObject();
11+
x = makeArray();
1012
};
1113
foo();
1214
return [y, x];
1315
}
1416

17+
export const FIXTURE_ENTRYPOINT = {
18+
fn: Component,
19+
params: [{}],
20+
};
21+
1522
```
1623

1724
## Code
1825

1926
```javascript
2027
import { unstable_useMemoCache as useMemoCache } from "react";
28+
import { makeArray } from "shared-runtime";
29+
2130
function Component() {
2231
const $ = useMemoCache(3);
2332
let x;
@@ -26,7 +35,7 @@ function Component() {
2635
y = x = {};
2736

2837
const foo = () => {
29-
x = getObject();
38+
x = makeArray();
3039
};
3140

3241
foo();
@@ -46,5 +55,12 @@ function Component() {
4655
return t0;
4756
}
4857

58+
export const FIXTURE_ENTRYPOINT = {
59+
fn: Component,
60+
params: [{}],
61+
};
62+
4963
```
50-
64+
65+
### Eval output
66+
(kind: ok) [{},[]]
Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,16 @@
1+
import { makeArray } from "shared-runtime";
2+
13
function Component() {
24
let x,
35
y = (x = {});
46
const foo = () => {
5-
x = getObject();
7+
x = makeArray();
68
};
79
foo();
810
return [y, x];
911
}
12+
13+
export const FIXTURE_ENTRYPOINT = {
14+
fn: Component,
15+
params: [{}],
16+
};
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
2+
## Input
3+
4+
```javascript
5+
import { invoke } from "shared-runtime";
6+
7+
function Component() {
8+
let x = 2;
9+
const fn = () => {
10+
return { x: "value" };
11+
};
12+
invoke(fn);
13+
x = 3;
14+
return x;
15+
}
16+
17+
export const FIXTURE_ENTRYPOINT = {
18+
fn: Component,
19+
params: [{}],
20+
};
21+
22+
```
23+
24+
## Code
25+
26+
```javascript
27+
import { invoke } from "shared-runtime";
28+
29+
function Component() {
30+
const fn = () => ({ x: "value" });
31+
32+
invoke(fn);
33+
return 3;
34+
}
35+
36+
export const FIXTURE_ENTRYPOINT = {
37+
fn: Component,
38+
params: [{}],
39+
};
40+
41+
```
42+
43+
### Eval output
44+
(kind: ok) 3
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { invoke } from "shared-runtime";
2+
3+
function Component() {
4+
let x = 2;
5+
const fn = () => {
6+
return { x: "value" };
7+
};
8+
invoke(fn);
9+
x = 3;
10+
return x;
11+
}
12+
13+
export const FIXTURE_ENTRYPOINT = {
14+
fn: Component,
15+
params: [{}],
16+
};

0 commit comments

Comments
 (0)