Skip to content

Commit f84fda3

Browse files
committed
[compiler] Switch to track setStates by aliasing and id instead of identifier names
Summary: This makes the setState usage logic much more robust. We no longer rely on identifierName. Now we track when a setState is loaded into a new promoted identifier variable and track this in a map `setStateLoaded` map. For other types of instructions we consider the setState to be being used. In this case we record its usage into the `setStateUsages` map. Test Plan: We expect no changes in behavior for the current tests
1 parent 8101d07 commit f84fda3

File tree

1 file changed

+89
-41
lines changed

1 file changed

+89
-41
lines changed

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

Lines changed: 89 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import {
2121
isUseStateType,
2222
BasicBlock,
2323
isUseRefType,
24-
GeneratedSource,
2524
SourceLocation,
2625
} from '../HIR';
2726
import {eachInstructionLValue, eachInstructionOperand} from '../HIR/visitors';
@@ -42,8 +41,8 @@ type ValidationContext = {
4241
readonly errors: CompilerError;
4342
readonly derivationCache: DerivationCache;
4443
readonly effects: Set<HIRFunction>;
45-
readonly setStateCache: Map<string | undefined | null, Array<Place>>;
46-
readonly effectSetStateCache: Map<string | undefined | null, Array<Place>>;
44+
readonly setStateLoads: Map<IdentifierId, IdentifierId | null>;
45+
readonly setStateUsages: Map<IdentifierId, Set<SourceLocation>>;
4746
};
4847

4948
class DerivationCache {
@@ -180,19 +179,16 @@ export function validateNoDerivedComputationsInEffects_exp(
180179
const errors = new CompilerError();
181180
const effects: Set<HIRFunction> = new Set();
182181

183-
const setStateCache: Map<string | undefined | null, Array<Place>> = new Map();
184-
const effectSetStateCache: Map<
185-
string | undefined | null,
186-
Array<Place>
187-
> = new Map();
182+
const setStateLoads: Map<IdentifierId, IdentifierId> = new Map();
183+
const setStateUsages: Map<IdentifierId, Set<SourceLocation>> = new Map();
188184

189185
const context: ValidationContext = {
190186
functions,
191187
errors,
192188
derivationCache,
193189
effects,
194-
setStateCache,
195-
effectSetStateCache,
190+
setStateLoads,
191+
setStateUsages,
196192
};
197193

198194
if (fn.fnType === 'Hook') {
@@ -281,11 +277,61 @@ function joinValue(
281277
return 'fromPropsAndState';
282278
}
283279

280+
function getRootSetState(
281+
key: IdentifierId,
282+
loads: Map<IdentifierId, IdentifierId | null>,
283+
visited: Set<IdentifierId> = new Set(),
284+
): IdentifierId | null {
285+
if (visited.has(key)) {
286+
return null;
287+
}
288+
visited.add(key);
289+
290+
const parentId = loads.get(key);
291+
292+
if (parentId === undefined) {
293+
return null;
294+
}
295+
296+
if (parentId === null) {
297+
return key;
298+
}
299+
300+
return getRootSetState(parentId, loads, visited);
301+
}
302+
303+
function maybeRecordSetState(
304+
instr: Instruction,
305+
loads: Map<IdentifierId, IdentifierId | null>,
306+
usages: Map<IdentifierId, Set<SourceLocation>>,
307+
): void {
308+
for (const operand of eachInstructionLValue(instr)) {
309+
if (
310+
instr.value.kind === 'LoadLocal' &&
311+
loads.has(instr.value.place.identifier.id)
312+
) {
313+
loads.set(operand.identifier.id, instr.value.place.identifier.id);
314+
} else {
315+
if (isSetStateType(operand.identifier)) {
316+
// this is a root setState
317+
loads.set(operand.identifier.id, null);
318+
}
319+
}
320+
321+
const rootSetState = getRootSetState(operand.identifier.id, loads);
322+
if (rootSetState !== null && usages.get(rootSetState) === undefined) {
323+
usages.set(rootSetState, new Set([operand.loc]));
324+
}
325+
}
326+
}
327+
284328
function recordInstructionDerivations(
285329
instr: Instruction,
286330
context: ValidationContext,
287331
isFirstPass: boolean,
288332
): void {
333+
maybeRecordSetState(instr, context.setStateLoads, context.setStateUsages);
334+
289335
let typeOfValue: TypeOfValue = 'ignored';
290336
let isSource: boolean = false;
291337
const sources: Set<IdentifierId> = new Set();
@@ -318,15 +364,13 @@ function recordInstructionDerivations(
318364
}
319365

320366
for (const operand of eachInstructionOperand(instr)) {
321-
if (
322-
isSetStateType(operand.identifier) &&
323-
operand.loc !== GeneratedSource &&
324-
isFirstPass
325-
) {
326-
if (context.setStateCache.has(operand.loc.identifierName)) {
327-
context.setStateCache.get(operand.loc.identifierName)!.push(operand);
328-
} else {
329-
context.setStateCache.set(operand.loc.identifierName, [operand]);
367+
if (context.setStateLoads.has(operand.identifier.id)) {
368+
const rootSetStateId = getRootSetState(
369+
operand.identifier.id,
370+
context.setStateLoads,
371+
);
372+
if (rootSetStateId !== null) {
373+
context.setStateUsages.get(rootSetStateId)?.add(operand.loc);
330374
}
331375
}
332376

@@ -532,11 +576,16 @@ function validateEffect(
532576

533577
const effectDerivedSetStateCalls: Array<{
534578
value: CallExpression;
535-
loc: SourceLocation;
579+
id: IdentifierId;
536580
sourceIds: Set<IdentifierId>;
537581
typeOfValue: TypeOfValue;
538582
}> = [];
539583

584+
const effectSetStateUsages: Map<
585+
IdentifierId,
586+
Set<SourceLocation>
587+
> = new Map();
588+
540589
const globals: Set<IdentifierId> = new Set();
541590
for (const block of effectFunction.body.blocks.values()) {
542591
for (const pred of block.preds) {
@@ -552,19 +601,16 @@ function validateEffect(
552601
return;
553602
}
554603

604+
maybeRecordSetState(instr, context.setStateLoads, effectSetStateUsages);
605+
555606
for (const operand of eachInstructionOperand(instr)) {
556-
if (
557-
isSetStateType(operand.identifier) &&
558-
operand.loc !== GeneratedSource
559-
) {
560-
if (context.effectSetStateCache.has(operand.loc.identifierName)) {
561-
context.effectSetStateCache
562-
.get(operand.loc.identifierName)!
563-
.push(operand);
564-
} else {
565-
context.effectSetStateCache.set(operand.loc.identifierName, [
566-
operand,
567-
]);
607+
if (context.setStateLoads.has(operand.identifier.id)) {
608+
const rootSetStateId = getRootSetState(
609+
operand.identifier.id,
610+
context.setStateLoads,
611+
);
612+
if (rootSetStateId !== null) {
613+
effectSetStateUsages.get(rootSetStateId)?.add(operand.loc);
568614
}
569615
}
570616
}
@@ -582,7 +628,7 @@ function validateEffect(
582628
if (argMetadata !== undefined) {
583629
effectDerivedSetStateCalls.push({
584630
value: instr.value,
585-
loc: instr.value.callee.loc,
631+
id: instr.value.callee.identifier.id,
586632
sourceIds: argMetadata.sourcesIds,
587633
typeOfValue: argMetadata.typeOfValue,
588634
});
@@ -616,15 +662,17 @@ function validateEffect(
616662
}
617663

618664
for (const derivedSetStateCall of effectDerivedSetStateCalls) {
665+
const rootSetStateCall = getRootSetState(
666+
derivedSetStateCall.id,
667+
context.setStateLoads,
668+
);
669+
619670
if (
620-
derivedSetStateCall.loc !== GeneratedSource &&
621-
context.effectSetStateCache.has(derivedSetStateCall.loc.identifierName) &&
622-
context.setStateCache.has(derivedSetStateCall.loc.identifierName) &&
623-
context.effectSetStateCache.get(derivedSetStateCall.loc.identifierName)!
624-
.length ===
625-
context.setStateCache.get(derivedSetStateCall.loc.identifierName)!
626-
.length -
627-
1
671+
rootSetStateCall !== null &&
672+
effectSetStateUsages.has(rootSetStateCall) &&
673+
context.setStateUsages.has(rootSetStateCall) &&
674+
effectSetStateUsages.get(rootSetStateCall)!.size ===
675+
context.setStateUsages.get(rootSetStateCall)!.size - 1
628676
) {
629677
const propsSet = new Set<string>();
630678
const stateSet = new Set<string>();

0 commit comments

Comments
 (0)