Skip to content

Commit 8f18b82

Browse files
committed
[patch][contextvars] Patch for variables reassigned after objectmethod
definitions
1 parent f0ef0b7 commit 8f18b82

File tree

5 files changed

+183
-66
lines changed

5 files changed

+183
-66
lines changed

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

Lines changed: 29 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -11,91 +11,58 @@ import { CompilerError } from "../CompilerError";
1111
import { Set_union } from "../Utils/utils";
1212
import { GeneratedSource } from "./HIR";
1313

14+
type BabelFunction =
15+
| NodePath<t.FunctionDeclaration>
16+
| NodePath<t.FunctionExpression>
17+
| NodePath<t.ArrowFunctionExpression>
18+
| NodePath<t.ObjectMethod>;
1419
type FindContextIdentifierState = {
15-
currentLambda: Array<
16-
| NodePath<t.FunctionDeclaration>
17-
| NodePath<t.FunctionExpression>
18-
| NodePath<t.ArrowFunctionExpression>
19-
| NodePath<t.ObjectMethod>
20-
>;
20+
currentFn: Array<BabelFunction>;
2121
reassigned: Set<t.Identifier>;
2222
referenced: Set<t.Identifier>;
2323
};
2424

25+
const withFunctionScope = {
26+
enter: function (
27+
path: BabelFunction,
28+
state: FindContextIdentifierState
29+
): void {
30+
state.currentFn.push(path);
31+
},
32+
exit: function (_: BabelFunction, state: FindContextIdentifierState): void {
33+
state.currentFn.pop();
34+
},
35+
};
36+
2537
export function findContextIdentifiers(
2638
func: NodePath<t.Function>
2739
): Set<t.Identifier> {
2840
const state: FindContextIdentifierState = {
29-
currentLambda: [],
41+
currentFn: [],
3042
reassigned: new Set(),
3143
referenced: new Set(),
3244
};
3345

3446
func.traverse<FindContextIdentifierState>(
3547
{
36-
FunctionDeclaration: {
37-
enter(
38-
fn: NodePath<t.FunctionDeclaration>,
39-
state: FindContextIdentifierState
40-
): void {
41-
state.currentLambda.push(fn);
42-
},
43-
exit(
44-
fn: NodePath<t.FunctionDeclaration>,
45-
state: FindContextIdentifierState
46-
): void {
47-
state.currentLambda.pop();
48-
},
49-
},
50-
FunctionExpression: {
51-
enter(
52-
fn: NodePath<t.FunctionExpression>,
53-
state: FindContextIdentifierState
54-
): void {
55-
state.currentLambda.push(fn);
56-
},
57-
exit(
58-
_fn: NodePath<t.FunctionExpression>,
59-
state: FindContextIdentifierState
60-
): void {
61-
state.currentLambda.pop();
62-
},
63-
},
64-
65-
ArrowFunctionExpression: {
66-
enter(
67-
fn: NodePath<t.ArrowFunctionExpression>,
68-
state: FindContextIdentifierState
69-
): void {
70-
state.currentLambda.push(fn);
71-
},
72-
exit(
73-
_fn: NodePath<t.ArrowFunctionExpression>,
74-
state: FindContextIdentifierState
75-
): void {
76-
state.currentLambda.pop();
77-
},
78-
},
48+
FunctionDeclaration: withFunctionScope,
49+
FunctionExpression: withFunctionScope,
50+
ArrowFunctionExpression: withFunctionScope,
51+
ObjectMethod: withFunctionScope,
7952
AssignmentExpression(
8053
path: NodePath<t.AssignmentExpression>,
8154
state: FindContextIdentifierState
8255
): void {
8356
const left = path.get("left");
8457
handleAssignment(state.reassigned, left);
8558
},
86-
ObjectMethod(
87-
fn: NodePath<t.ObjectMethod>,
88-
state: FindContextIdentifierState
89-
): void {
90-
state.currentLambda.push(fn);
91-
},
9259
Identifier(
9360
path: NodePath<t.Identifier>,
9461
state: FindContextIdentifierState
9562
): void {
96-
const currentLambda = state.currentLambda.at(-1);
97-
if (currentLambda !== undefined)
98-
handleIdentifier(currentLambda, state.referenced, path);
63+
const currentFn = state.currentFn.at(-1);
64+
if (currentFn !== undefined)
65+
handleIdentifier(currentFn, state.referenced, path);
9966
},
10067
},
10168
state
@@ -104,17 +71,13 @@ export function findContextIdentifiers(
10471
}
10572

10673
function handleIdentifier(
107-
currentLambda:
108-
| NodePath<t.FunctionDeclaration>
109-
| NodePath<t.FunctionExpression>
110-
| NodePath<t.ArrowFunctionExpression>
111-
| NodePath<t.ObjectMethod>,
74+
currentFn: BabelFunction,
11275
referenced: Set<t.Identifier>,
11376
path: NodePath<t.Identifier>
11477
): void {
11578
const name = path.node.name;
11679
const binding = path.scope.getBinding(name);
117-
const bindingAboveLambdaScope = currentLambda.scope.parent.getBinding(name);
80+
const bindingAboveLambdaScope = currentFn.scope.parent.getBinding(name);
11881

11982
if (binding != null && binding === bindingAboveLambdaScope) {
12083
referenced.add(binding.identifier);
@@ -126,7 +89,7 @@ function handleAssignment(
12689
lvalPath: NodePath<t.LVal>
12790
): void {
12891
/*
129-
* Find all reassignments to identifiers declared outside of currentLambda
92+
* Find all reassignments to identifiers declared outside of currentFn
13093
* This closely follows destructuring assignment assumptions and logic in BuildHIR
13194
*/
13295
const lvalNode = lvalPath.node;
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
2+
## Input
3+
4+
```javascript
5+
import { identity } from "shared-runtime";
6+
7+
// repro for context identifier scoping bug, in which x was
8+
// inferred as a context variable.
9+
10+
function Component() {
11+
let x = 2;
12+
const obj = {
13+
method() {},
14+
};
15+
x = 4;
16+
identity(obj);
17+
// constant propagation should return 4 here
18+
return x;
19+
}
20+
21+
export const FIXTURE_ENTRYPOINT = {
22+
fn: Component,
23+
params: [{}],
24+
};
25+
26+
```
27+
28+
## Code
29+
30+
```javascript
31+
import { identity } from "shared-runtime";
32+
33+
// repro for context identifier scoping bug, in which x was
34+
// inferred as a context variable.
35+
36+
function Component() {
37+
const obj = { method() {} };
38+
39+
identity(obj);
40+
return 4;
41+
}
42+
43+
export const FIXTURE_ENTRYPOINT = {
44+
fn: Component,
45+
params: [{}],
46+
};
47+
48+
```
49+
50+
### Eval output
51+
(kind: ok) 4
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { identity } from "shared-runtime";
2+
3+
// repro for context identifier scoping bug, in which x was
4+
// inferred as a context variable.
5+
6+
function Component() {
7+
let x = 2;
8+
const obj = {
9+
method() {},
10+
};
11+
x = 4;
12+
identity(obj);
13+
// constant propagation should return 4 here
14+
return x;
15+
}
16+
17+
export const FIXTURE_ENTRYPOINT = {
18+
fn: Component,
19+
params: [{}],
20+
};
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
2+
## Input
3+
4+
```javascript
5+
import { invoke } from "shared-runtime";
6+
7+
function Component({ cond }) {
8+
let x = 2;
9+
const obj = {
10+
method(cond) {
11+
if (cond) {
12+
x = 4;
13+
}
14+
},
15+
};
16+
invoke(obj.method, cond);
17+
return x;
18+
}
19+
20+
export const FIXTURE_ENTRYPOINT = {
21+
fn: Component,
22+
params: [{ cond: true }],
23+
};
24+
25+
```
26+
27+
## Code
28+
29+
```javascript
30+
import { unstable_useMemoCache as useMemoCache } from "react";
31+
import { invoke } from "shared-runtime";
32+
33+
function Component(t24) {
34+
const $ = useMemoCache(2);
35+
const { cond } = t24;
36+
let x;
37+
if ($[0] !== cond) {
38+
x = 2;
39+
const obj = {
40+
method(cond_0) {
41+
if (cond_0) {
42+
x = 4;
43+
}
44+
},
45+
};
46+
47+
invoke(obj.method, cond);
48+
$[0] = cond;
49+
$[1] = x;
50+
} else {
51+
x = $[1];
52+
}
53+
return x;
54+
}
55+
56+
export const FIXTURE_ENTRYPOINT = {
57+
fn: Component,
58+
params: [{ cond: true }],
59+
};
60+
61+
```
62+
63+
### Eval output
64+
(kind: ok) 4
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import { invoke } from "shared-runtime";
2+
3+
function Component({ cond }) {
4+
let x = 2;
5+
const obj = {
6+
method(cond) {
7+
if (cond) {
8+
x = 4;
9+
}
10+
},
11+
};
12+
invoke(obj.method, cond);
13+
return x;
14+
}
15+
16+
export const FIXTURE_ENTRYPOINT = {
17+
fn: Component,
18+
params: [{ cond: true }],
19+
};

0 commit comments

Comments
 (0)