[compiler] Delete PropagatePhiTypes (#34107)
We moved this logic into InferTypes a long time ago and the PRs to clean it up keep getting lost in the shuffle.
Joseph Savona committed
Aug 4, 2025 at 15:15 UTC
7deda941f7f77e82de0311fc3e0cf94d8a863069
2 files changed
-118
compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts
-8
@@ -92,7 +92,6 @@ import {
92
} from '../Validation';
93
import {validateLocalsNotReassignedAfterRender} from '../Validation/ValidateLocalsNotReassignedAfterRender';
94
import {outlineFunctions} from '../Optimization/OutlineFunctions';
95
-import {propagatePhiTypes} from '../TypeInference/PropagatePhiTypes';
95
import {lowerContextAccess} from '../Optimization/LowerContextAccess';
96
import {validateNoSetStateInEffects} from '../Validation/ValidateNoSetStateInEffects';
97
import {validateNoJSXInTryStatement} from '../Validation/ValidateNoJSXInTryStatement';
@@ -327,13 +326,6 @@ function runWithEnvironment(
326
value: hir,
327
});
328
330
- propagatePhiTypes(hir);
331
- log({
332
- kind: 'hir',
333
- name: 'PropagatePhiTypes',
334
- value: hir,
335
- });
336
-
329
if (env.isInferredMemoEnabled) {
330
if (env.config.validateStaticComponents) {
331
env.logErrors(validateStaticComponents(hir));
compiler/packages/babel-plugin-react-compiler/src/TypeInference/PropagatePhiTypes.ts
deleted
-110
@@ -1,110 +0,0 @@
1
-/**
2
- * Copyright (c) Meta Platforms, Inc. and affiliates.
3
- *
4
- * This source code is licensed under the MIT license found in the
5
- * LICENSE file in the root directory of this source tree.
6
- */
7
-
8
-import {HIRFunction, IdentifierId, Type, typeEquals} from '../HIR';
9
-
10
-/**
11
- * Temporary workaround for InferTypes not propagating the types of phis.
12
- * Previously, LeaveSSA would replace all the identifiers for each phi (operands and
13
- * the phi itself) with a single "canonical" identifier, generally chosen as the first
14
- * operand to flow into the phi. In case of a phi whose operand was a phi, this could
15
- * sometimes be an operand from the earlier phi.
16
- *
17
- * As a result, even though InferTypes did not propagate types for phis, LeaveSSA
18
- * could end up replacing the phi Identifier with another identifer from an operand,
19
- * which _did_ have a type inferred.
20
- *
21
- * This didn't affect the initial construction of mutable ranges because InferMutableRanges
22
- * runs before LeaveSSA - thus, the types propagated by LeaveSSA only affected later optimizations,
23
- * notably MergeScopesThatInvalidateTogether which uses type to determine if a scope's output
24
- * will always invalidate with its input.
25
- *
26
- * The long-term correct approach is to update InferTypes to infer the types of phis,
27
- * but this is complicated because InferMutableRanges inadvertently depends on phis
28
- * never having a known type, such that a Store effect cannot occur on a phi value.
29
- * Once we fix InferTypes to infer phi types, then we'll also have to update InferMutableRanges
30
- * to handle this case.
31
- *
32
- * As a temporary workaround, this pass propagates the type of phis and can be called
33
- * safely *after* InferMutableRanges. Unlike LeaveSSA, this pass only propagates the
34
- * type if all operands have the same type, it's its more correct.
35
- */
36
-export function propagatePhiTypes(fn: HIRFunction): void {
37
- /**
38
- * We track which SSA ids have had their types propagated to handle nested ternaries,
39
- * see the StoreLocal handling below
40
- */
41
- const propagated = new Set<IdentifierId>();
42
- for (const [, block] of fn.body.blocks) {
43
- for (const phi of block.phis) {
44
- /*
45
- * We replicate the previous LeaveSSA behavior and only propagate types for
46
- * unnamed variables. LeaveSSA would have chosen one of the operands as the
47
- * canonical id and taken its type as the type of all identifiers. We're
48
- * more conservative and only propagate if the types are the same and the
49
- * phi didn't have a type inferred.
50
- *
51
- * Note that this can change output slightly in cases such as
52
- * `cond ? <div /> : null`.
53
- *
54
- * Previously the first operand's type (BuiltInJsx) would have been propagated,
55
- * and this expression may have been merged with subsequent reactive scopes
56
- * since it appears (based on that type) to always invalidate.
57
- *
58
- * But the correct type is `BuiltInJsx | null`, which we can't express and
59
- * so leave as a generic `Type`, which does not always invalidate and therefore
60
- * does not merge with subsequent scopes.
61
- *
62
- * We also don't propagate scopes for named variables, to preserve compatibility
63
- * with previous LeaveSSA behavior.
64
- */
65
- if (
66
- phi.place.identifier.type.kind !== 'Type' ||
67
- phi.place.identifier.name !== null
68
- ) {
69
- continue;
70
- }
71
- let type: Type | null = null;
72
- for (const [, operand] of phi.operands) {
73
- if (type === null) {
74
- type = operand.identifier.type;
75
- } else if (!typeEquals(type, operand.identifier.type)) {
76
- type = null;
77
- break;
78
- }
79
- }
80
- if (type !== null) {
81
- phi.place.identifier.type = type;
82
- propagated.add(phi.place.identifier.id);
83
- }
84
- }
85
- for (const instr of block.instructions) {
86
- const {value} = instr;
87
- switch (value.kind) {
88
- case 'StoreLocal': {
89
- /**
90
- * Nested ternaries can lower to a form with an intermediate StoreLocal where
91
- * the value.lvalue is the temporary of the outer ternary, and the value.value
92
- * is the result of the inner ternary.
93
- *
94
- * This is a common pattern in practice and easy enough to support. Again, the
95
- * long-term approach is to update InferTypes and InferMutableRanges.
96
- */
97
- const lvalue = value.lvalue.place;
98
- if (
99
- propagated.has(value.value.identifier.id) &&
100
- lvalue.identifier.type.kind === 'Type' &&
101
- lvalue.identifier.name === null
102
- ) {
103
- lvalue.identifier.type = value.value.identifier.type;
104
- propagated.add(lvalue.identifier.id);
105
- }
106
- }
107
- }
108
- }
109
- }
110
-}