Skip to content

Commit 557b4f1

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 83a74b1 commit 557b4f1

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

@@ -522,11 +566,16 @@ function validateEffect(
522566

523567
const effectDerivedSetStateCalls: Array<{
524568
value: CallExpression;
525-
loc: SourceLocation;
569+
id: IdentifierId;
526570
sourceIds: Set<IdentifierId>;
527571
typeOfValue: TypeOfValue;
528572
}> = [];
529573

574+
const effectSetStateUsages: Map<
575+
IdentifierId,
576+
Set<SourceLocation>
577+
> = new Map();
578+
530579
const globals: Set<IdentifierId> = new Set();
531580
for (const block of effectFunction.body.blocks.values()) {
532581
for (const pred of block.preds) {
@@ -542,19 +591,16 @@ function validateEffect(
542591
return;
543592
}
544593

594+
maybeRecordSetState(instr, context.setStateLoads, effectSetStateUsages);
595+
545596
for (const operand of eachInstructionOperand(instr)) {
546-
if (
547-
isSetStateType(operand.identifier) &&
548-
operand.loc !== GeneratedSource
549-
) {
550-
if (context.effectSetStateCache.has(operand.loc.identifierName)) {
551-
context.effectSetStateCache
552-
.get(operand.loc.identifierName)!
553-
.push(operand);
554-
} else {
555-
context.effectSetStateCache.set(operand.loc.identifierName, [
556-
operand,
557-
]);
597+
if (context.setStateLoads.has(operand.identifier.id)) {
598+
const rootSetStateId = getRootSetState(
599+
operand.identifier.id,
600+
context.setStateLoads,
601+
);
602+
if (rootSetStateId !== null) {
603+
effectSetStateUsages.get(rootSetStateId)?.add(operand.loc);
558604
}
559605
}
560606
}
@@ -572,7 +618,7 @@ function validateEffect(
572618
if (argMetadata !== undefined) {
573619
effectDerivedSetStateCalls.push({
574620
value: instr.value,
575-
loc: instr.value.callee.loc,
621+
id: instr.value.callee.identifier.id,
576622
sourceIds: argMetadata.sourcesIds,
577623
typeOfValue: argMetadata.typeOfValue,
578624
});
@@ -606,15 +652,17 @@ function validateEffect(
606652
}
607653

608654
for (const derivedSetStateCall of effectDerivedSetStateCalls) {
655+
const rootSetStateCall = getRootSetState(
656+
derivedSetStateCall.id,
657+
context.setStateLoads,
658+
);
659+
609660
if (
610-
derivedSetStateCall.loc !== GeneratedSource &&
611-
context.effectSetStateCache.has(derivedSetStateCall.loc.identifierName) &&
612-
context.setStateCache.has(derivedSetStateCall.loc.identifierName) &&
613-
context.effectSetStateCache.get(derivedSetStateCall.loc.identifierName)!
614-
.length ===
615-
context.setStateCache.get(derivedSetStateCall.loc.identifierName)!
616-
.length -
617-
1
661+
rootSetStateCall !== null &&
662+
effectSetStateUsages.has(rootSetStateCall) &&
663+
context.setStateUsages.has(rootSetStateCall) &&
664+
effectSetStateUsages.get(rootSetStateCall)!.size ===
665+
context.setStateUsages.get(rootSetStateCall)!.size - 1
618666
) {
619667
const allSourceIds = Array.from(derivedSetStateCall.sourceIds);
620668
const propsSet = new Set<string>();

0 commit comments

Comments
 (0)