Skip to content

Commit 00d44be

Browse files
committed
Update on "[compiler] Add fallthrough to branch terminal"
Branch terminals didn't have a fallthrough because they correspond to an outer terminal (optional, logical, etc) that has the "real" fallthrough. But understanding how branch terminals correspond to these outer terminals requires knowing the branch fallthrough. For example, `foo?.bar?.baz` creates terminals along the lines of: ``` bb0: optional fallthrough=bb4 bb1: optional fallthrough=bb3 bb2: ... branch ... (fallthrough=bb3) ... bb3: ... branch ... (fallthrough=bb4) ... bb4: ... ``` Without a fallthrough on `branch` terminals, it's unclear that the optional from bb0 has its branch node in bb3. With the fallthroughs, we can see look for a branch with the same fallthrough as the outer optional terminal to match them up. [ghstack-poisoned]
2 parents fed48b4 + 8241424 commit 00d44be

File tree

3 files changed

+53
-5
lines changed

3 files changed

+53
-5
lines changed

compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ type IdentifierSidemap = {
4242
react: Set<IdentifierId>;
4343
maybeDepsLists: Map<IdentifierId, Array<Place>>;
4444
maybeDeps: Map<IdentifierId, ManualMemoDependency>;
45+
optionals: Set<IdentifierId>;
4546
};
4647

4748
/**
@@ -52,6 +53,7 @@ type IdentifierSidemap = {
5253
export function collectMaybeMemoDependencies(
5354
value: InstructionValue,
5455
maybeDeps: Map<IdentifierId, ManualMemoDependency>,
56+
optional: boolean,
5557
): ManualMemoDependency | null {
5658
switch (value.kind) {
5759
case 'LoadGlobal': {
@@ -69,7 +71,7 @@ export function collectMaybeMemoDependencies(
6971
return {
7072
root: object.root,
7173
// TODO: determine if the access is optional
72-
path: [...object.path, {property: value.property, optional: false}],
74+
path: [...object.path, {property: value.property, optional}],
7375
};
7476
}
7577
break;
@@ -162,7 +164,11 @@ function collectTemporaries(
162164
break;
163165
}
164166
}
165-
const maybeDep = collectMaybeMemoDependencies(value, sidemap.maybeDeps);
167+
const maybeDep = collectMaybeMemoDependencies(
168+
value,
169+
sidemap.maybeDeps,
170+
sidemap.optionals.has(lvalue.identifier.id),
171+
);
166172
// We don't expect named lvalues during this pass (unlike ValidatePreservingManualMemo)
167173
if (maybeDep != null) {
168174
sidemap.maybeDeps.set(lvalue.identifier.id, maybeDep);
@@ -338,12 +344,14 @@ export function dropManualMemoization(func: HIRFunction): void {
338344
func.env.config.validatePreserveExistingMemoizationGuarantees ||
339345
func.env.config.validateNoSetStateInRender ||
340346
func.env.config.enablePreserveExistingMemoizationGuarantees;
347+
const optionals = findOptionalPlaces(func);
341348
const sidemap: IdentifierSidemap = {
342349
functions: new Map(),
343350
manualMemos: new Map(),
344351
react: new Set(),
345352
maybeDeps: new Map(),
346353
maybeDepsLists: new Map(),
354+
optionals,
347355
};
348356
let nextManualMemoId = 0;
349357

@@ -481,6 +489,40 @@ function findOptionalPlaces(fn: HIRFunction): Set<IdentifierId> {
481489
const optionals = new Set<IdentifierId>();
482490
for (const [, block] of fn.body.blocks) {
483491
if (block.terminal.kind === 'optional') {
492+
const optionalTerminal = block.terminal;
493+
let testBlock = fn.body.blocks.get(block.terminal.test)!;
494+
loop: while (true) {
495+
const terminal = testBlock.terminal;
496+
switch (terminal.kind) {
497+
case 'branch': {
498+
if (terminal.fallthrough === optionalTerminal.fallthrough) {
499+
// found it
500+
const consequent = fn.body.blocks.get(terminal.consequent)!;
501+
const last = consequent.instructions.at(-1);
502+
if (last !== undefined && last.value.kind === 'StoreLocal') {
503+
optionals.add(last.value.value.identifier.id);
504+
}
505+
break loop;
506+
} else {
507+
testBlock = fn.body.blocks.get(terminal.fallthrough)!;
508+
}
509+
break;
510+
}
511+
case 'optional':
512+
case 'logical':
513+
case 'sequence':
514+
case 'ternary': {
515+
testBlock = fn.body.blocks.get(terminal.fallthrough)!;
516+
break;
517+
}
518+
default: {
519+
CompilerError.invariant(false, {
520+
reason: `Unexpected terminal in optional`,
521+
loc: terminal.loc,
522+
});
523+
}
524+
}
525+
}
484526
}
485527
}
486528
return optionals;

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import {CompilerError} from '..';
99
import {
1010
DeclarationId,
11-
DependencyPath,
1211
InstructionId,
1312
InstructionKind,
1413
Place,

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,10 @@ function compareDeps(
167167

168168
let isSubpath = true;
169169
for (let i = 0; i < Math.min(inferred.path.length, source.path.length); i++) {
170-
if (inferred.path[i].property !== source.path[i].property) {
170+
if (
171+
inferred.path[i].property !== source.path[i].property ||
172+
inferred.path[i].optional !== source.path[i].optional
173+
) {
171174
isSubpath = false;
172175
break;
173176
}
@@ -339,7 +342,11 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
339342
return null;
340343
}
341344
default: {
342-
const dep = collectMaybeMemoDependencies(value, this.temporaries);
345+
const dep = collectMaybeMemoDependencies(
346+
value,
347+
this.temporaries,
348+
false,
349+
);
343350
if (value.kind === 'StoreLocal' || value.kind === 'StoreContext') {
344351
const storeTarget = value.lvalue.place;
345352
state.manualMemoState?.decls.add(

0 commit comments

Comments
 (0)