Skip to content

Commit ee51a5d

Browse files
committed
[compiler] FunctionExpression context locations point to first reference
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.
1 parent 5ace0f5 commit ee51a5d

File tree

3 files changed

+35
-21
lines changed

3 files changed

+35
-21
lines changed

compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -72,21 +72,21 @@ export function lower(
7272
env: Environment,
7373
// Bindings captured from the outer function, in case lower() is called recursively (for lambdas)
7474
bindings: Bindings | null = null,
75-
capturedRefs: Array<t.Identifier> = [],
75+
capturedRefs: Map<t.Identifier, SourceLocation> = new Map(),
7676
): Result<HIRFunction, CompilerError> {
7777
const builder = new HIRBuilder(env, {
7878
bindings,
7979
context: capturedRefs,
8080
});
8181
const context: HIRFunction['context'] = [];
8282

83-
for (const ref of capturedRefs ?? []) {
83+
for (const [ref, loc] of capturedRefs ?? []) {
8484
context.push({
8585
kind: 'Identifier',
8686
identifier: builder.resolveBinding(ref),
8787
effect: Effect.Unknown,
8888
reactive: false,
89-
loc: ref.loc ?? GeneratedSource,
89+
loc,
9090
});
9191
}
9292

@@ -3439,10 +3439,12 @@ function lowerFunction(
34393439
* This isn't a problem in practice because use Babel's scope analysis to
34403440
* identify the correct references.
34413441
*/
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+
);
34463448
let loweredFunc: HIRFunction;
34473449
if (lowering.isErr()) {
34483450
lowering
@@ -4160,6 +4162,11 @@ function captureScopes({from, to}: {from: Scope; to: Scope}): Set<Scope> {
41604162
return scopes;
41614163
}
41624164

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+
*/
41634170
function gatherCapturedContext(
41644171
fn: NodePath<
41654172
| t.FunctionExpression
@@ -4168,8 +4175,8 @@ function gatherCapturedContext(
41684175
| t.ObjectMethod
41694176
>,
41704177
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>();
41734180

41744181
/*
41754182
* Capture all the scopes from the parent of this function up to and including
@@ -4212,8 +4219,15 @@ function gatherCapturedContext(
42124219

42134220
// Add the base identifier binding as a dependency.
42144221
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+
);
42174231
}
42184232
}
42194233

@@ -4250,7 +4264,7 @@ function gatherCapturedContext(
42504264
},
42514265
});
42524266

4253-
return [...capturedIds.keys()];
4267+
return capturedIds;
42544268
}
42554269

42564270
function notNull<T>(value: T | null): value is T {

compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ export default class HIRBuilder {
106106
#current: WipBlock;
107107
#entry: BlockId;
108108
#scopes: Array<Scope> = [];
109-
#context: Array<t.Identifier>;
109+
#context: Map<t.Identifier, SourceLocation>;
110110
#bindings: Bindings;
111111
#env: Environment;
112112
#exceptionHandlerStack: Array<BlockId> = [];
@@ -121,7 +121,7 @@ export default class HIRBuilder {
121121
return this.#env.nextIdentifierId;
122122
}
123123

124-
get context(): Array<t.Identifier> {
124+
get context(): Map<t.Identifier, SourceLocation> {
125125
return this.#context;
126126
}
127127

@@ -137,13 +137,13 @@ export default class HIRBuilder {
137137
env: Environment,
138138
options?: {
139139
bindings?: Bindings | null;
140-
context?: Array<t.Identifier>;
140+
context?: Map<t.Identifier, SourceLocation>;
141141
entryBlockKind?: BlockKind;
142142
},
143143
) {
144144
this.#env = env;
145145
this.#bindings = options?.bindings ?? new Map();
146-
this.#context = options?.context ?? [];
146+
this.#context = options?.context ?? new Map();
147147
this.#entry = makeBlockId(env.nextBlockId);
148148
this.#current = newBlock(this.#entry, options?.entryBlockKind ?? 'block');
149149
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-valid-functiondecl-hoisting.expect.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,13 @@ export const FIXTURE_ENTRYPOINT = {
3434
## Error
3535

3636
```
37-
13 | return bar();
37+
11 |
38+
12 | function foo() {
39+
> 13 | return bar();
40+
| ^^^ Todo: [PruneHoistedContexts] Rewrite hoisted function references (13:13)
3841
14 | }
39-
> 15 | function bar() {
40-
| ^^^ Todo: [PruneHoistedContexts] Rewrite hoisted function references (15:15)
42+
15 | function bar() {
4143
16 | return 42;
42-
17 | }
43-
18 |
4444
```
4545
4646

0 commit comments

Comments
 (0)