Skip to content

Commit d237545

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 5488fd6 commit d237545

File tree

1 file changed

+89
-36
lines changed

1 file changed

+89
-36
lines changed

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

Lines changed: 89 additions & 36 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';
@@ -41,8 +40,8 @@ type ValidationContext = {
4140
readonly errors: CompilerError;
4241
readonly derivationCache: DerivationCache;
4342
readonly effects: Set<HIRFunction>;
44-
readonly setStateCache: Map<string | undefined | null, Array<Place>>;
45-
readonly effectSetStateCache: Map<string | undefined | null, Array<Place>>;
43+
readonly setStateLoads: Map<IdentifierId, IdentifierId | null>;
44+
readonly setStateUsages: Map<IdentifierId, Set<SourceLocation>>;
4645
};
4746

4847
class DerivationCache {
@@ -188,13 +187,16 @@ export function validateNoDerivedComputationsInEffects_exp(
188187
Array<Place>
189188
> = new Map();
190189

190+
const setStateLoads: Map<IdentifierId, IdentifierId> = new Map();
191+
const setStateUsages: Map<IdentifierId, Set<SourceLocation>> = new Map();
192+
191193
const context: ValidationContext = {
192194
functions,
193195
errors,
194196
derivationCache,
195197
effects,
196-
setStateCache,
197-
effectSetStateCache,
198+
setStateLoads,
199+
setStateUsages,
198200
};
199201

200202
if (fn.fnType === 'Hook') {
@@ -284,11 +286,60 @@ function joinValue(
284286
return 'fromPropsAndState';
285287
}
286288

289+
function getRootSetState(
290+
key: IdentifierId,
291+
loads: Map<IdentifierId, IdentifierId | null>,
292+
visited: Set<IdentifierId> = new Set(),
293+
): IdentifierId | null {
294+
if (visited.has(key)) {
295+
return null;
296+
}
297+
visited.add(key);
298+
299+
const parentId = loads.get(key);
300+
301+
if (parentId === undefined) {
302+
return null;
303+
}
304+
305+
if (parentId === null) {
306+
return key;
307+
}
308+
309+
return getRootSetState(parentId, loads, visited);
310+
}
311+
312+
function maybeRecordSetState(
313+
instr: Instruction,
314+
loads: Map<IdentifierId, IdentifierId | null>,
315+
usages: Map<IdentifierId, Set<SourceLocation>>,
316+
) {
317+
for (const operand of eachInstructionLValue(instr)) {
318+
if (isSetStateType(operand.identifier)) {
319+
if (instr.value.kind === 'LoadLocal') {
320+
loads.set(operand.identifier.id, instr.value.place.identifier.id);
321+
} else {
322+
// this is a root setState
323+
loads.set(operand.identifier.id, null);
324+
}
325+
326+
const rootSetState = getRootSetState(operand.identifier.id, loads);
327+
if (rootSetState !== null && usages.get(rootSetState) === undefined) {
328+
usages.set(rootSetState, new Set([operand.loc]));
329+
}
330+
}
331+
}
332+
}
333+
287334
function recordInstructionDerivations(
288335
instr: Instruction,
289336
context: ValidationContext,
290337
isFirstPass: boolean,
291338
): void {
339+
if (isFirstPass) {
340+
maybeRecordSetState(instr, context.setStateLoads, context.setStateUsages);
341+
}
342+
292343
let typeOfValue: TypeOfValue = 'ignored';
293344
const sources: Set<IdentifierId> = new Set();
294345
const {lvalue, value} = instr;
@@ -323,15 +374,13 @@ function recordInstructionDerivations(
323374
}
324375

325376
for (const operand of eachInstructionOperand(instr)) {
326-
if (
327-
isSetStateType(operand.identifier) &&
328-
operand.loc !== GeneratedSource &&
329-
isFirstPass
330-
) {
331-
if (context.setStateCache.has(operand.loc.identifierName)) {
332-
context.setStateCache.get(operand.loc.identifierName)!.push(operand);
333-
} else {
334-
context.setStateCache.set(operand.loc.identifierName, [operand]);
377+
if (isSetStateType(operand.identifier) && isFirstPass) {
378+
const rootSetStateId = getRootSetState(
379+
operand.identifier.id,
380+
context.setStateLoads,
381+
);
382+
if (rootSetStateId !== null) {
383+
context.setStateUsages.get(rootSetStateId)?.add(operand.loc);
335384
}
336385
}
337386

@@ -469,11 +518,16 @@ function validateEffect(
469518

470519
const effectDerivedSetStateCalls: Array<{
471520
value: CallExpression;
472-
loc: SourceLocation;
521+
id: IdentifierId;
473522
sourceIds: Set<IdentifierId>;
474523
typeOfValue: TypeOfValue;
475524
}> = [];
476525

526+
const effectSetStateUsages: Map<
527+
IdentifierId,
528+
Set<SourceLocation>
529+
> = new Map();
530+
477531
const globals: Set<IdentifierId> = new Set();
478532
for (const block of effectFunction.body.blocks.values()) {
479533
for (const pred of block.preds) {
@@ -489,19 +543,16 @@ function validateEffect(
489543
return;
490544
}
491545

546+
maybeRecordSetState(instr, context.setStateLoads, effectSetStateUsages);
547+
492548
for (const operand of eachInstructionOperand(instr)) {
493-
if (
494-
isSetStateType(operand.identifier) &&
495-
operand.loc !== GeneratedSource
496-
) {
497-
if (context.effectSetStateCache.has(operand.loc.identifierName)) {
498-
context.effectSetStateCache
499-
.get(operand.loc.identifierName)!
500-
.push(operand);
501-
} else {
502-
context.effectSetStateCache.set(operand.loc.identifierName, [
503-
operand,
504-
]);
549+
if (isSetStateType(operand.identifier)) {
550+
const rootSetStateId = getRootSetState(
551+
operand.identifier.id,
552+
context.setStateLoads,
553+
);
554+
if (rootSetStateId !== null) {
555+
effectSetStateUsages.get(rootSetStateId)?.add(operand.loc);
505556
}
506557
}
507558
}
@@ -519,7 +570,7 @@ function validateEffect(
519570
if (argMetadata !== undefined) {
520571
effectDerivedSetStateCalls.push({
521572
value: instr.value,
522-
loc: instr.value.callee.loc,
573+
id: instr.value.callee.identifier.id,
523574
sourceIds: argMetadata.sourcesIds,
524575
typeOfValue: argMetadata.typeOfValue,
525576
});
@@ -553,15 +604,17 @@ function validateEffect(
553604
}
554605

555606
for (const derivedSetStateCall of effectDerivedSetStateCalls) {
607+
const rootSetStateCall = getRootSetState(
608+
derivedSetStateCall.id,
609+
context.setStateLoads,
610+
);
611+
556612
if (
557-
derivedSetStateCall.loc !== GeneratedSource &&
558-
context.effectSetStateCache.has(derivedSetStateCall.loc.identifierName) &&
559-
context.setStateCache.has(derivedSetStateCall.loc.identifierName) &&
560-
context.effectSetStateCache.get(derivedSetStateCall.loc.identifierName)!
561-
.length ===
562-
context.setStateCache.get(derivedSetStateCall.loc.identifierName)!
563-
.length -
564-
1
613+
rootSetStateCall !== null &&
614+
effectSetStateUsages.has(rootSetStateCall) &&
615+
context.setStateUsages.has(rootSetStateCall) &&
616+
effectSetStateUsages.get(rootSetStateCall)!.size ===
617+
context.setStateUsages.get(rootSetStateCall)!.size - 1
565618
) {
566619
const allSourceIds = Array.from(derivedSetStateCall.sourceIds);
567620
const trees = allSourceIds

0 commit comments

Comments
 (0)