[compiler] FunctionExpression context locations point to first reference (#33512)
This has always been awkward: `FunctionExpression.context` places have locations set to the declaration of the identifier, whereas other references have locations pointing to the reference itself. Here, we update context operands to have their location point to the first reference of that variable within the function. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33512). * #33571 * #33558 * #33547 * #33543 * #33533 * #33532 * #33530 * #33526 * #33522 * #33518 * #33514 * #33513 * __->__ #33512 * #33504 * #33500 * #33497 * #33496
Joseph Savona committed
Jun 18, 2025 at 13:02 UTC
e081cb344652dc3003d9194cca618292a889ff2a
3 files changed
+35
-21
compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
+26
-12
@@ -72,7 +72,7 @@ export function lower(
72
env: Environment,
73
// Bindings captured from the outer function, in case lower() is called recursively (for lambdas)
74
bindings: Bindings | null = null,
75
- capturedRefs: Array<t.Identifier> = [],
75
+ capturedRefs: Map<t.Identifier, SourceLocation> = new Map(),
76
): Result<HIRFunction, CompilerError> {
77
const builder = new HIRBuilder(env, {
78
bindings,
@@ -80,13 +80,13 @@ export function lower(
80
});
81
const context: HIRFunction['context'] = [];
82
83
- for (const ref of capturedRefs ?? []) {
83
+ for (const [ref, loc] of capturedRefs ?? []) {
84
context.push({
85
kind: 'Identifier',
86
identifier: builder.resolveBinding(ref),
87
effect: Effect.Unknown,
88
reactive: false,
89
- loc: ref.loc ?? GeneratedSource,
89
+ loc,
90
});
91
}
92
@@ -3439,10 +3439,12 @@ function lowerFunction(
3439
* This isn't a problem in practice because use Babel's scope analysis to
3440
* identify the correct references.
3441
*/
3442
- const lowering = lower(expr, builder.environment, builder.bindings, [
3443
- ...builder.context,
3444
- ...capturedContext,
3445
- ]);
3442
+ const lowering = lower(
3443
+ expr,
3444
+ builder.environment,
3445
+ builder.bindings,
3446
+ new Map([...builder.context, ...capturedContext]),
3447
+ );
3448
let loweredFunc: HIRFunction;
3449
if (lowering.isErr()) {
3450
lowering
@@ -4160,6 +4162,11 @@ function captureScopes({from, to}: {from: Scope; to: Scope}): Set<Scope> {
4162
return scopes;
4163
}
4164
4165
+/**
4166
+ * Returns a mapping of "context" identifiers — references to free variables that
4167
+ * will become part of the function expression's `context` array — along with the
4168
+ * source location of their first reference within the function.
4169
+ */
4170
function gatherCapturedContext(
4171
fn: NodePath<
4172
| t.FunctionExpression
@@ -4168,8 +4175,8 @@ function gatherCapturedContext(
4175
| t.ObjectMethod
4176
>,
4177
componentScope: Scope,
4171
-): Array<t.Identifier> {
4172
- const capturedIds = new Set<t.Identifier>();
4178
+): Map<t.Identifier, SourceLocation> {
4179
+ const capturedIds = new Map<t.Identifier, SourceLocation>();
4180
4181
/*
4182
* Capture all the scopes from the parent of this function up to and including
@@ -4212,8 +4219,15 @@ function gatherCapturedContext(
4219
4220
// Add the base identifier binding as a dependency.
4221
const binding = baseIdentifier.scope.getBinding(baseIdentifier.node.name);
4215
- if (binding !== undefined && pureScopes.has(binding.scope)) {
4216
- capturedIds.add(binding.identifier);
4222
+ if (
4223
+ binding !== undefined &&
4224
+ pureScopes.has(binding.scope) &&
4225
+ !capturedIds.has(binding.identifier)
4226
+ ) {
4227
+ capturedIds.set(
4228
+ binding.identifier,
4229
+ path.node.loc ?? binding.identifier.loc ?? GeneratedSource,
4230
+ );
4231
}
4232
}
4233
@@ -4250,7 +4264,7 @@ function gatherCapturedContext(
4264
},
4265
});
4266
4253
- return [...capturedIds.keys()];
4267
+ return capturedIds;
4268
}
4269
4270
function notNull<T>(value: T | null): value is T {
compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts
+4
-4
@@ -106,7 +106,7 @@ export default class HIRBuilder {
106
#current: WipBlock;
107
#entry: BlockId;
108
#scopes: Array<Scope> = [];
109
- #context: Array<t.Identifier>;
109
+ #context: Map<t.Identifier, SourceLocation>;
110
#bindings: Bindings;
111
#env: Environment;
112
#exceptionHandlerStack: Array<BlockId> = [];
@@ -121,7 +121,7 @@ export default class HIRBuilder {
121
return this.#env.nextIdentifierId;
122
}
123
124
- get context(): Array<t.Identifier> {
124
+ get context(): Map<t.Identifier, SourceLocation> {
125
return this.#context;
126
}
127
@@ -137,13 +137,13 @@ export default class HIRBuilder {
137
env: Environment,
138
options?: {
139
bindings?: Bindings | null;
140
- context?: Array<t.Identifier>;
140
+ context?: Map<t.Identifier, SourceLocation>;
141
entryBlockKind?: BlockKind;
142
},
143
) {
144
this.#env = env;
145
this.#bindings = options?.bindings ?? new Map();
146
- this.#context = options?.context ?? [];
146
+ this.#context = options?.context ?? new Map();
147
this.#entry = makeBlockId(env.nextBlockId);
148
this.#current = newBlock(this.#entry, options?.entryBlockKind ?? 'block');
149
}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-valid-functiondecl-hoisting.expect.md
+5
-5
@@ -34,13 +34,13 @@ export const FIXTURE_ENTRYPOINT = {
34
## Error
35
36
```
37
- 13 | return bar();
37
+ 11 |
38
+ 12 | function foo() {
39
+> 13 | return bar();
40
+ | ^^^ Todo: [PruneHoistedContexts] Rewrite hoisted function references (13:13)
41
14 | }
39
-> 15 | function bar() {
40
- | ^^^ Todo: [PruneHoistedContexts] Rewrite hoisted function references (15:15)
42
+ 15 | function bar() {
43
16 | return 42;
42
- 17 | }
43
- 18 |
44
```
45
46
\ No newline at end of file