Skip to content

Commit 4ac4753

Browse files
authored
[compiler] Track locations for dependencies (facebook#35794)
Tracks locations for reactive scope dependencies, both on the deps and portions of the path. The immediate need for this is a non-public experiment where we're exploring type-directed compilation, and sometimes look up the types of expressions by location. We need to preserve locations accurately for that to work, including the locations of the deps. ## Test Plan Locations for dependencies are not easy to test: i manually spot-checked the new fixture to ensure that the deps look right. This is fine as best-effort since it doesn't impact any of our core compilation logic, i may fix forward if there are issues and will think about how to test.
1 parent 47d1ad1 commit 4ac4753

File tree

11 files changed

+194
-24
lines changed

11 files changed

+194
-24
lines changed

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

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
PropertyLiteral,
3232
ReactiveScopeDependency,
3333
ScopeId,
34+
SourceLocation,
3435
TInstruction,
3536
} from './HIR';
3637

@@ -244,6 +245,7 @@ class PropertyPathRegistry {
244245
getOrCreateIdentifier(
245246
identifier: Identifier,
246247
reactive: boolean,
248+
loc: SourceLocation,
247249
): PropertyPathNode {
248250
/**
249251
* Reads from a statically scoped variable are always safe in JS,
@@ -260,6 +262,7 @@ class PropertyPathRegistry {
260262
identifier,
261263
reactive,
262264
path: [],
265+
loc,
263266
},
264267
hasOptional: false,
265268
parent: null,
@@ -290,6 +293,7 @@ class PropertyPathRegistry {
290293
identifier: parent.fullPath.identifier,
291294
reactive: parent.fullPath.reactive,
292295
path: parent.fullPath.path.concat(entry),
296+
loc: entry.loc,
293297
},
294298
hasOptional: parent.hasOptional || entry.optional,
295299
};
@@ -304,7 +308,7 @@ class PropertyPathRegistry {
304308
* so all subpaths of a PropertyLoad should already exist
305309
* (e.g. a.b is added before a.b.c),
306310
*/
307-
let currNode = this.getOrCreateIdentifier(n.identifier, n.reactive);
311+
let currNode = this.getOrCreateIdentifier(n.identifier, n.reactive, n.loc);
308312
if (n.path.length === 0) {
309313
return currNode;
310314
}
@@ -323,20 +327,21 @@ class PropertyPathRegistry {
323327
}
324328

325329
function getMaybeNonNullInInstruction(
326-
instr: InstructionValue,
330+
value: InstructionValue,
327331
context: CollectHoistablePropertyLoadsContext,
328332
): PropertyPathNode | null {
329333
let path: ReactiveScopeDependency | null = null;
330-
if (instr.kind === 'PropertyLoad') {
331-
path = context.temporaries.get(instr.object.identifier.id) ?? {
332-
identifier: instr.object.identifier,
333-
reactive: instr.object.reactive,
334+
if (value.kind === 'PropertyLoad') {
335+
path = context.temporaries.get(value.object.identifier.id) ?? {
336+
identifier: value.object.identifier,
337+
reactive: value.object.reactive,
334338
path: [],
339+
loc: value.loc,
335340
};
336-
} else if (instr.kind === 'Destructure') {
337-
path = context.temporaries.get(instr.value.identifier.id) ?? null;
338-
} else if (instr.kind === 'ComputedLoad') {
339-
path = context.temporaries.get(instr.object.identifier.id) ?? null;
341+
} else if (value.kind === 'Destructure') {
342+
path = context.temporaries.get(value.value.identifier.id) ?? null;
343+
} else if (value.kind === 'ComputedLoad') {
344+
path = context.temporaries.get(value.object.identifier.id) ?? null;
340345
}
341346
return path != null ? context.registry.getOrCreateProperty(path) : null;
342347
}
@@ -393,7 +398,11 @@ function collectNonNullsInBlocks(
393398
) {
394399
const identifier = fn.params[0].identifier;
395400
knownNonNullIdentifiers.add(
396-
context.registry.getOrCreateIdentifier(identifier, true),
401+
context.registry.getOrCreateIdentifier(
402+
identifier,
403+
true,
404+
fn.params[0].loc,
405+
),
397406
);
398407
}
399408
const nodes = new Map<
@@ -468,6 +477,7 @@ function collectNonNullsInBlocks(
468477
identifier: dep.root.value.identifier,
469478
path: dep.path.slice(0, i),
470479
reactive: dep.root.value.reactive,
480+
loc: dep.loc,
471481
});
472482
assumedNonNullObjects.add(depNode);
473483
}
@@ -654,17 +664,23 @@ function reduceMaybeOptionalChains(
654664
changed = false;
655665

656666
for (const original of optionalChainNodes) {
657-
let {identifier, path: origPath, reactive} = original.fullPath;
667+
let {
668+
identifier,
669+
path: origPath,
670+
reactive,
671+
loc: origLoc,
672+
} = original.fullPath;
658673
let currNode: PropertyPathNode = registry.getOrCreateIdentifier(
659674
identifier,
660675
reactive,
676+
origLoc,
661677
);
662678
for (let i = 0; i < origPath.length; i++) {
663679
const entry = origPath[i];
664680
// If the base is known to be non-null, replace with a non-optional load
665681
const nextEntry: DependencyPathEntry =
666682
entry.optional && nodes.has(currNode)
667-
? {property: entry.property, optional: false}
683+
? {property: entry.property, optional: false, loc: entry.loc}
668684
: entry;
669685
currNode = PropertyPathRegistry.getOrCreatePropertyEntry(
670686
currNode,

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8-
import {CompilerError} from '..';
8+
import {CompilerError, SourceLocation} from '..';
99
import {assertNonNull} from './CollectHoistablePropertyLoads';
1010
import {
1111
BlockId,
@@ -169,6 +169,7 @@ function matchOptionalTestBlock(
169169
propertyId: IdentifierId;
170170
storeLocalInstr: Instruction;
171171
consequentGoto: BlockId;
172+
propertyLoadLoc: SourceLocation;
172173
} | null {
173174
const consequentBlock = assertNonNull(blocks.get(terminal.consequent));
174175
if (
@@ -221,6 +222,7 @@ function matchOptionalTestBlock(
221222
propertyId: propertyLoad.lvalue.identifier.id,
222223
storeLocalInstr,
223224
consequentGoto: consequentBlock.terminal.block,
225+
propertyLoadLoc: propertyLoad.loc,
224226
};
225227
}
226228
return null;
@@ -275,7 +277,11 @@ function traverseOptionalBlock(
275277
instrVal.kind === 'PropertyLoad' &&
276278
instrVal.object.identifier.id === prevInstr.lvalue.identifier.id
277279
) {
278-
path.push({property: instrVal.property, optional: false});
280+
path.push({
281+
property: instrVal.property,
282+
optional: false,
283+
loc: instrVal.loc,
284+
});
279285
} else {
280286
return null;
281287
}
@@ -292,6 +298,7 @@ function traverseOptionalBlock(
292298
identifier: maybeTest.instructions[0].value.place.identifier,
293299
reactive: maybeTest.instructions[0].value.place.reactive,
294300
path,
301+
loc: maybeTest.instructions[0].value.place.loc,
295302
};
296303
test = maybeTest.terminal;
297304
} else if (maybeTest.terminal.kind === 'optional') {
@@ -390,16 +397,18 @@ function traverseOptionalBlock(
390397
loc: optional.terminal.loc,
391398
},
392399
);
393-
const load = {
400+
const load: ReactiveScopeDependency = {
394401
identifier: baseObject.identifier,
395402
reactive: baseObject.reactive,
396403
path: [
397404
...baseObject.path,
398405
{
399406
property: matchConsequentResult.property,
400407
optional: optional.terminal.optional,
408+
loc: matchConsequentResult.propertyLoadLoc,
401409
},
402410
],
411+
loc: matchConsequentResult.propertyLoadLoc,
403412
};
404413
context.processedInstrsInOptional.add(matchConsequentResult.storeLocalInstr);
405414
context.processedInstrsInOptional.add(test);

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
Identifier,
1313
PropertyLiteral,
1414
ReactiveScopeDependency,
15+
SourceLocation,
1516
} from '../HIR';
1617
import {printIdentifier} from '../HIR/PrintHIR';
1718

@@ -36,12 +37,13 @@ export class ReactiveScopeDependencyTreeHIR {
3637
* duplicates when traversing the CFG.
3738
*/
3839
constructor(hoistableObjects: Iterable<ReactiveScopeDependency>) {
39-
for (const {path, identifier, reactive} of hoistableObjects) {
40+
for (const {path, identifier, reactive, loc} of hoistableObjects) {
4041
let currNode = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot(
4142
identifier,
4243
reactive,
4344
this.#hoistableObjects,
4445
path.length > 0 && path[0].optional ? 'Optional' : 'NonNull',
46+
loc,
4547
);
4648

4749
for (let i = 0; i < path.length; i++) {
@@ -62,6 +64,7 @@ export class ReactiveScopeDependencyTreeHIR {
6264
nextNode = {
6365
properties: new Map(),
6466
accessType,
67+
loc: path[i].loc,
6568
};
6669
currNode.properties.set(path[i].property, nextNode);
6770
}
@@ -75,6 +78,7 @@ export class ReactiveScopeDependencyTreeHIR {
7578
reactive: boolean,
7679
roots: Map<Identifier, TreeNode<T> & {reactive: boolean}>,
7780
defaultAccessType: T,
81+
loc: SourceLocation,
7882
): TreeNode<T> {
7983
// roots can always be accessed unconditionally in JS
8084
let rootNode = roots.get(identifier);
@@ -84,6 +88,7 @@ export class ReactiveScopeDependencyTreeHIR {
8488
properties: new Map(),
8589
reactive,
8690
accessType: defaultAccessType,
91+
loc,
8792
};
8893
roots.set(identifier, rootNode);
8994
} else {
@@ -102,12 +107,13 @@ export class ReactiveScopeDependencyTreeHIR {
102107
* safe-to-evaluate subpath
103108
*/
104109
addDependency(dep: ReactiveScopeDependency): void {
105-
const {identifier, reactive, path} = dep;
110+
const {identifier, reactive, path, loc} = dep;
106111
let depCursor = ReactiveScopeDependencyTreeHIR.#getOrCreateRoot(
107112
identifier,
108113
reactive,
109114
this.#deps,
110115
PropertyAccessType.UnconditionalAccess,
116+
loc,
111117
);
112118
/**
113119
* hoistableCursor is null if depCursor is not an object we can hoist
@@ -153,6 +159,7 @@ export class ReactiveScopeDependencyTreeHIR {
153159
depCursor,
154160
entry.property,
155161
accessType,
162+
entry.loc,
156163
);
157164
} else if (
158165
hoistableCursor != null &&
@@ -163,6 +170,7 @@ export class ReactiveScopeDependencyTreeHIR {
163170
depCursor,
164171
entry.property,
165172
PropertyAccessType.UnconditionalAccess,
173+
entry.loc,
166174
);
167175
} else {
168176
/**
@@ -306,6 +314,7 @@ function merge(
306314
type TreeNode<T extends string> = {
307315
properties: Map<PropertyLiteral, TreeNode<T>>;
308316
accessType: T;
317+
loc: SourceLocation;
309318
};
310319
type HoistableNode = TreeNode<'Optional' | 'NonNull'>;
311320
type DependencyNode = TreeNode<PropertyAccessType>;
@@ -323,7 +332,7 @@ function collectMinimalDependenciesInSubtree(
323332
results: Set<ReactiveScopeDependency>,
324333
): void {
325334
if (isDependency(node.accessType)) {
326-
results.add({identifier: rootIdentifier, reactive, path});
335+
results.add({identifier: rootIdentifier, reactive, path, loc: node.loc});
327336
} else {
328337
for (const [childName, childNode] of node.properties) {
329338
collectMinimalDependenciesInSubtree(
@@ -335,6 +344,7 @@ function collectMinimalDependenciesInSubtree(
335344
{
336345
property: childName,
337346
optional: isOptional(childNode.accessType),
347+
loc: childNode.loc,
338348
},
339349
],
340350
results,
@@ -362,12 +372,14 @@ function makeOrMergeProperty(
362372
node: DependencyNode,
363373
property: PropertyLiteral,
364374
accessType: PropertyAccessType,
375+
loc: SourceLocation,
365376
): DependencyNode {
366377
let child = node.properties.get(property);
367378
if (child == null) {
368379
child = {
369380
properties: new Map(),
370381
accessType,
382+
loc,
371383
};
372384
node.properties.set(property, child);
373385
} else {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1639,6 +1639,7 @@ export function makePropertyLiteral(value: string | number): PropertyLiteral {
16391639
export type DependencyPathEntry = {
16401640
property: PropertyLiteral;
16411641
optional: boolean;
1642+
loc: SourceLocation;
16421643
};
16431644
export type DependencyPath = Array<DependencyPathEntry>;
16441645
export type ReactiveScopeDependency = {
@@ -1656,6 +1657,7 @@ export type ReactiveScopeDependency = {
16561657
*/
16571658
reactive: boolean;
16581659
path: DependencyPath;
1660+
loc: SourceLocation;
16591661
};
16601662

16611663
export function areEqualPaths(a: DependencyPath, b: DependencyPath): boolean {

0 commit comments

Comments
 (0)