[compiler][ez] Clean up duplicate code in propagateScopeDeps (#31581)
Clean up duplicate checks for when to skip processing values as dependencies / hoistable temporaries. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31581). * #31583 * #31582 * __->__ #31581
mofeiZ committed
Dec 16, 2024 at 15:11 UTC
ac172706526a840100302f92ae90dfa4ad804c56
1 file changed
+58
-24
compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts
+58
-24
@@ -358,6 +358,7 @@ class Context {
358
359
#temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>;
360
#temporariesUsedOutsideScope: ReadonlySet<DeclarationId>;
361
+ #processedInstrsInOptional: ReadonlySet<Instruction | Terminal>;
362
363
/**
364
* Tracks the traversal state. See Context.declare for explanation of why this
@@ -368,9 +369,11 @@ class Context {
369
constructor(
370
temporariesUsedOutsideScope: ReadonlySet<DeclarationId>,
371
temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>,
372
+ processedInstrsInOptional: ReadonlySet<Instruction | Terminal>,
373
) {
374
this.#temporariesUsedOutsideScope = temporariesUsedOutsideScope;
375
this.#temporaries = temporaries;
376
+ this.#processedInstrsInOptional = processedInstrsInOptional;
377
}
378
379
enterScope(scope: ReactiveScope): void {
@@ -574,22 +577,49 @@ class Context {
577
currentScope.reassignments.add(place.identifier);
578
}
579
}
580
+ enterInnerFn<T>(cb: () => T): T {
581
+ const wasInInnerFn = this.inInnerFn;
582
+ this.inInnerFn = true;
583
+ const result = cb();
584
+ this.inInnerFn = wasInInnerFn;
585
+ return result;
586
+ }
587
+
588
+ /**
589
+ * Skip dependencies that are subexpressions of other dependencies. e.g. if a
590
+ * dependency is tracked in the temporaries sidemap, it can be added at
591
+ * site-of-use
592
+ */
593
+ isDeferredDependency(
594
+ instr:
595
+ | {kind: HIRValue.Instruction; value: Instruction}
596
+ | {kind: HIRValue.Terminal; value: Terminal},
597
+ ): boolean {
598
+ return (
599
+ this.#processedInstrsInOptional.has(instr.value) ||
600
+ (instr.kind === HIRValue.Instruction &&
601
+ this.#temporaries.has(instr.value.lvalue.identifier.id))
602
+ );
603
+ }
604
+}
605
+enum HIRValue {
606
+ Instruction = 1,
607
+ Terminal,
608
}
609
610
function handleInstruction(instr: Instruction, context: Context): void {
611
const {id, value, lvalue} = instr;
581
- if (value.kind === 'LoadLocal') {
582
- if (
583
- value.place.identifier.name === null ||
584
- lvalue.identifier.name !== null ||
585
- context.isUsedOutsideDeclaringScope(lvalue)
586
- ) {
587
- context.visitOperand(value.place);
588
- }
589
- } else if (value.kind === 'PropertyLoad') {
590
- if (context.isUsedOutsideDeclaringScope(lvalue)) {
591
- context.visitProperty(value.object, value.property, false);
592
- }
612
+ context.declare(lvalue.identifier, {
613
+ id,
614
+ scope: context.currentScope,
615
+ });
616
+ if (
617
+ context.isDeferredDependency({kind: HIRValue.Instruction, value: instr})
618
+ ) {
619
+ return;
620
+ }
621
+ if (value.kind === 'PropertyLoad') {
622
+ context.visitProperty(value.object, value.property, false);
623
} else if (value.kind === 'StoreLocal') {
624
context.visitOperand(value.value);
625
if (value.lvalue.kind === InstructionKind.Reassign) {
@@ -632,11 +662,6 @@ function handleInstruction(instr: Instruction, context: Context): void {
662
context.visitOperand(operand);
663
}
664
}
635
-
636
- context.declare(lvalue.identifier, {
637
- id,
638
- scope: context.currentScope,
639
- });
665
}
666
667
function collectDependencies(
@@ -645,7 +670,11 @@ function collectDependencies(
670
temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>,
671
processedInstrsInOptional: ReadonlySet<Instruction | Terminal>,
672
): Map<ReactiveScope, Array<ReactiveScopeDependency>> {
648
- const context = new Context(usedOutsideDeclaringScope, temporaries);
673
+ const context = new Context(
674
+ usedOutsideDeclaringScope,
675
+ temporaries,
676
+ processedInstrsInOptional,
677
+ );
678
679
for (const param of fn.params) {
680
if (param.kind === 'Identifier') {
@@ -694,16 +723,21 @@ function collectDependencies(
723
/**
724
* Recursively visit the inner function to extract dependencies there
725
*/
697
- const wasInInnerFn = context.inInnerFn;
698
- context.inInnerFn = true;
699
- handleFunction(instr.value.loweredFunc.func);
700
- context.inInnerFn = wasInInnerFn;
701
- } else if (!processedInstrsInOptional.has(instr)) {
726
+ const innerFn = instr.value.loweredFunc.func;
727
+ context.enterInnerFn(() => {
728
+ handleFunction(innerFn);
729
+ });
730
+ } else {
731
handleInstruction(instr, context);
732
}
733
}
734
706
- if (!processedInstrsInOptional.has(block.terminal)) {
735
+ if (
736
+ !context.isDeferredDependency({
737
+ kind: HIRValue.Terminal,
738
+ value: block.terminal,
739
+ })
740
+ ) {
741
for (const place of eachTerminalOperand(block.terminal)) {
742
context.visitOperand(place);
743
}