Skip to content

Commit a654856

Browse files
committed
[wip][compiler] Infer optional dependencies
Updates PropagateScopeDeps and DeriveMinimalDeps to understand optional dependency paths (`a?.b`). There a few key pieces to this: In PropagateScopeDeps we jump through some hoops to work around the awkward structure of nested OptionalExpressions. This is much easier in HIR form, but I managed to get this pretty close and i think it will be landable with further cleanup. A good chunk of this is avoiding prematurely registering a value as a dependency - there are a bunch of indirections in the ReactiveFunction structure: ``` t0 = OptionalExpression SequenceExpression t0 = Sequence ... LoadLocal t0 ``` Where if at any point we call `visitOperand()` we'll prematurely register a dependency instead of declareProperty(). The other bit is that optionals can be optional=false for nested member expressions where not all the parts are actually optional (`foo.bar?.bar.call()`). And of course, parts of an optional chain can still be conditional even when optional=true (for example the `x` in `foo.bar?.[x]?.baz`). Not all of this is tested yet so there are likely bugs still. The other bit is DeriveMinimalDeps, which is thankfully easier. We add OptionalAccess and OptionalDep and update the merge and reducing logic for these cases. There is probably still more to update though, for things like merging subtrees. There are a lot of ternaries that assume a result can be exactly one of two states (conditional/unconditional, dependency/access) and these assumptions don't hold anymore. I'd like to refactor to dependency/access separate from conditional/optional/unconditional. Also, the reducing logic isn't quite right: once a child is optional we keep inferring all the parents as optional too, losing some precision. I need to adjust the reducing logic to let children decide whether their path token is optional or not. ghstack-source-id: c17bf59 Pull Request resolved: #30819
1 parent ac01cf7 commit a654856

14 files changed

+348
-126
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ export function printTerminal(terminal: Terminal): Array<string> | string {
191191
case 'branch': {
192192
value = `[${terminal.id}] Branch (${printPlace(terminal.test)}) then:bb${
193193
terminal.consequent
194-
} else:bb${terminal.alternate}`;
194+
} else:bb${terminal.alternate} fallthrough:bb${terminal.fallthrough}`;
195195
break;
196196
}
197197
case 'logical': {

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1446,9 +1446,19 @@ function codegenDependency(
14461446
dependency: ReactiveScopeDependency,
14471447
): t.Expression {
14481448
let object: t.Expression = convertIdentifier(dependency.identifier);
1449-
if (dependency.path !== null) {
1449+
if (dependency.path.length !== 0) {
1450+
const hasOptional = dependency.path.some(path => path.optional);
14501451
for (const path of dependency.path) {
1451-
object = t.memberExpression(object, t.identifier(path.property));
1452+
if (hasOptional) {
1453+
object = t.optionalMemberExpression(
1454+
object,
1455+
t.identifier(path.property),
1456+
false,
1457+
path.optional,
1458+
);
1459+
} else {
1460+
object = t.memberExpression(object, t.identifier(path.property));
1461+
}
14521462
}
14531463
}
14541464
return object;

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/DeriveMinimalDependencies.ts

Lines changed: 132 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {CompilerError} from '../CompilerError';
99
import {DependencyPath, Identifier, ReactiveScopeDependency} from '../HIR';
1010
import {printIdentifier} from '../HIR/PrintHIR';
1111
import {assertExhaustive} from '../Utils/utils';
12+
import {printDependency} from './PrintReactiveFunction';
1213

1314
/*
1415
* We need to understand optional member expressions only when determining
@@ -60,13 +61,14 @@ export class ReactiveScopeDependencyTree {
6061
const {path} = dep;
6162
let currNode = this.#getOrCreateRoot(dep.identifier);
6263

63-
const accessType = inConditional
64-
? PropertyAccessType.ConditionalAccess
65-
: PropertyAccessType.UnconditionalAccess;
66-
6764
for (const item of path) {
6865
// all properties read 'on the way' to a dependency are marked as 'access'
6966
let currChild = getOrMakeProperty(currNode, item.property);
67+
const accessType = inConditional
68+
? PropertyAccessType.ConditionalAccess
69+
: item.optional
70+
? PropertyAccessType.OptionalAccess
71+
: PropertyAccessType.UnconditionalAccess;
7072
currChild.accessType = merge(currChild.accessType, accessType);
7173
currNode = currChild;
7274
}
@@ -77,18 +79,22 @@ export class ReactiveScopeDependencyTree {
7779
*/
7880
const depType = inConditional
7981
? PropertyAccessType.ConditionalDependency
80-
: PropertyAccessType.UnconditionalDependency;
82+
: isOptional(currNode.accessType)
83+
? PropertyAccessType.OptionalDependency
84+
: PropertyAccessType.UnconditionalDependency;
8185

8286
currNode.accessType = merge(currNode.accessType, depType);
8387
}
8488

8589
deriveMinimalDependencies(): Set<ReactiveScopeDependency> {
8690
const results = new Set<ReactiveScopeDependency>();
8791
for (const [rootId, rootNode] of this.#roots.entries()) {
88-
const deps = deriveMinimalDependenciesInSubtree(rootNode);
92+
const deps = deriveMinimalDependenciesInSubtree(rootNode, null);
8993
CompilerError.invariant(
9094
deps.every(
91-
dep => dep.accessType === PropertyAccessType.UnconditionalDependency,
95+
dep =>
96+
dep.accessType === PropertyAccessType.UnconditionalDependency ||
97+
dep.accessType == PropertyAccessType.OptionalDependency,
9298
),
9399
{
94100
reason:
@@ -173,6 +179,27 @@ export class ReactiveScopeDependencyTree {
173179
}
174180
return res.flat().join('\n');
175181
}
182+
183+
debug(): string {
184+
const buf: Array<string> = [`tree() [`];
185+
for (const [rootId, rootNode] of this.#roots) {
186+
buf.push(`${printIdentifier(rootId)} (${rootNode.accessType}):`);
187+
this.#debugImpl(buf, rootNode, 1);
188+
}
189+
buf.push(']');
190+
return buf.length > 2 ? buf.join('\n') : buf.join('');
191+
}
192+
193+
#debugImpl(
194+
buf: Array<string>,
195+
node: DependencyNode,
196+
depth: number = 0,
197+
): void {
198+
for (const [property, childNode] of node.properties) {
199+
buf.push(`${' '.repeat(depth)}.${property} (${childNode.accessType}):`);
200+
this.#debugImpl(buf, childNode, depth + 1);
201+
}
202+
}
176203
}
177204

178205
/*
@@ -196,8 +223,10 @@ export class ReactiveScopeDependencyTree {
196223
*/
197224
enum PropertyAccessType {
198225
ConditionalAccess = 'ConditionalAccess',
226+
OptionalAccess = 'OptionalAccess',
199227
UnconditionalAccess = 'UnconditionalAccess',
200228
ConditionalDependency = 'ConditionalDependency',
229+
OptionalDependency = 'OptionalDependency',
201230
UnconditionalDependency = 'UnconditionalDependency',
202231
}
203232

@@ -211,9 +240,16 @@ function isUnconditional(access: PropertyAccessType): boolean {
211240
function isDependency(access: PropertyAccessType): boolean {
212241
return (
213242
access === PropertyAccessType.ConditionalDependency ||
243+
access === PropertyAccessType.OptionalDependency ||
214244
access === PropertyAccessType.UnconditionalDependency
215245
);
216246
}
247+
function isOptional(access: PropertyAccessType): boolean {
248+
return (
249+
access === PropertyAccessType.OptionalAccess ||
250+
access === PropertyAccessType.OptionalDependency
251+
);
252+
}
217253

218254
function merge(
219255
access1: PropertyAccessType,
@@ -222,6 +258,7 @@ function merge(
222258
const resultIsUnconditional =
223259
isUnconditional(access1) || isUnconditional(access2);
224260
const resultIsDependency = isDependency(access1) || isDependency(access2);
261+
const resultIsOptional = isOptional(access1) || isOptional(access2);
225262

226263
/*
227264
* Straightforward merge.
@@ -237,6 +274,12 @@ function merge(
237274
} else {
238275
return PropertyAccessType.UnconditionalAccess;
239276
}
277+
} else if (resultIsOptional) {
278+
if (resultIsDependency) {
279+
return PropertyAccessType.OptionalDependency;
280+
} else {
281+
return PropertyAccessType.OptionalAccess;
282+
}
240283
} else {
241284
if (resultIsDependency) {
242285
return PropertyAccessType.ConditionalDependency;
@@ -256,19 +299,34 @@ type ReduceResultNode = {
256299
accessType: PropertyAccessType;
257300
};
258301

259-
const promoteUncondResult = [
260-
{
302+
function promoteResult(
303+
accessType: PropertyAccessType,
304+
path: {property: string; optional: boolean} | null,
305+
): Array<ReduceResultNode> {
306+
const result: ReduceResultNode = {
261307
relativePath: [],
262-
accessType: PropertyAccessType.UnconditionalDependency,
263-
},
264-
];
308+
accessType,
309+
};
310+
if (path !== null) {
311+
result.relativePath.push(path);
312+
}
313+
return [result];
314+
}
265315

266-
const promoteCondResult = [
267-
{
268-
relativePath: [],
269-
accessType: PropertyAccessType.ConditionalDependency,
270-
},
271-
];
316+
function prependPath(
317+
results: Array<ReduceResultNode>,
318+
path: {property: string; optional: boolean} | null,
319+
): Array<ReduceResultNode> {
320+
if (path === null) {
321+
return results;
322+
}
323+
return results.map(result => {
324+
return {
325+
accessType: result.accessType,
326+
relativePath: [path, ...result.relativePath],
327+
};
328+
});
329+
}
272330

273331
/*
274332
* Recursively calculates minimal dependencies in a subtree.
@@ -277,42 +335,76 @@ const promoteCondResult = [
277335
*/
278336
function deriveMinimalDependenciesInSubtree(
279337
dep: DependencyNode,
338+
property: string | null,
280339
): Array<ReduceResultNode> {
281340
const results: Array<ReduceResultNode> = [];
282341
for (const [childName, childNode] of dep.properties) {
283-
const childResult = deriveMinimalDependenciesInSubtree(childNode).map(
284-
({relativePath, accessType}) => {
285-
return {
286-
relativePath: [
287-
{property: childName, optional: false},
288-
...relativePath,
289-
],
290-
accessType,
291-
};
292-
},
342+
const childResult = deriveMinimalDependenciesInSubtree(
343+
childNode,
344+
childName,
293345
);
294346
results.push(...childResult);
295347
}
296348

297349
switch (dep.accessType) {
298350
case PropertyAccessType.UnconditionalDependency: {
299-
return promoteUncondResult;
351+
return promoteResult(
352+
PropertyAccessType.UnconditionalDependency,
353+
property !== null ? {property, optional: false} : null,
354+
);
300355
}
301356
case PropertyAccessType.UnconditionalAccess: {
302357
if (
303358
results.every(
304359
({accessType}) =>
305-
accessType === PropertyAccessType.UnconditionalDependency,
360+
accessType === PropertyAccessType.UnconditionalDependency ||
361+
accessType === PropertyAccessType.OptionalDependency,
306362
)
307363
) {
308364
// all children are unconditional dependencies, return them to preserve granularity
309-
return results;
365+
return prependPath(
366+
results,
367+
property !== null ? {property, optional: false} : null,
368+
);
310369
} else {
311370
/*
312371
* at least one child is accessed conditionally, so this node needs to be promoted to
313372
* unconditional dependency
314373
*/
315-
return promoteUncondResult;
374+
return promoteResult(
375+
PropertyAccessType.UnconditionalDependency,
376+
property !== null ? {property, optional: false} : null,
377+
);
378+
}
379+
}
380+
case PropertyAccessType.OptionalDependency: {
381+
return promoteResult(
382+
PropertyAccessType.OptionalDependency,
383+
property !== null ? {property, optional: true} : null,
384+
);
385+
}
386+
case PropertyAccessType.OptionalAccess: {
387+
if (
388+
results.every(
389+
({accessType}) =>
390+
accessType === PropertyAccessType.UnconditionalDependency ||
391+
accessType === PropertyAccessType.OptionalDependency,
392+
)
393+
) {
394+
// all children are unconditional dependencies, return them to preserve granularity
395+
return prependPath(
396+
results,
397+
property !== null ? {property, optional: true} : null,
398+
);
399+
} else {
400+
/*
401+
* at least one child is accessed conditionally, so this node needs to be promoted to
402+
* unconditional dependency
403+
*/
404+
return promoteResult(
405+
PropertyAccessType.OptionalDependency,
406+
property !== null ? {property, optional: true} : null,
407+
);
316408
}
317409
}
318410
case PropertyAccessType.ConditionalAccess:
@@ -328,13 +420,19 @@ function deriveMinimalDependenciesInSubtree(
328420
* unconditional access.
329421
* Truncate results of child nodes here, since we shouldn't access them anyways
330422
*/
331-
return promoteCondResult;
423+
return promoteResult(
424+
PropertyAccessType.ConditionalDependency,
425+
property !== null ? {property, optional: true} : null,
426+
);
332427
} else {
333428
/*
334429
* at least one child is accessed unconditionally, so this node can be promoted to
335430
* unconditional dependency
336431
*/
337-
return promoteUncondResult;
432+
return promoteResult(
433+
PropertyAccessType.UnconditionalDependency,
434+
property !== null ? {property, optional: true} : null,
435+
);
338436
}
339437
}
340438
default: {

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ export function printDependency(dependency: ReactiveScopeDependency): string {
113113
const identifier =
114114
printIdentifier(dependency.identifier) +
115115
printType(dependency.identifier.type);
116-
return `${identifier}${dependency.path.map(token => `.${token.property}`).join('')}`;
116+
return `${identifier}${dependency.path.map(token => `${token.optional ? '?.' : '.'}${token.property}`).join('')}`;
117117
}
118118

119119
export function printReactiveInstructions(

0 commit comments

Comments
 (0)