Skip to content

Commit a3149fb

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 6a2c00d commit a3149fb

File tree

1 file changed

+88
-41
lines changed

1 file changed

+88
-41
lines changed

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

Lines changed: 88 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';
@@ -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 {
@@ -182,19 +181,16 @@ export function validateNoDerivedComputationsInEffects_exp(
182181
const errors = new CompilerError();
183182
const effects: Set<HIRFunction> = new Set();
184183

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

191187
const context: ValidationContext = {
192188
functions,
193189
errors,
194190
derivationCache,
195191
effects,
196-
setStateCache,
197-
effectSetStateCache,
192+
setStateLoads,
193+
setStateUsages,
198194
};
199195

200196
if (fn.fnType === 'Hook') {
@@ -284,11 +280,60 @@ function joinValue(
284280
return 'fromPropsAndState';
285281
}
286282

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

325370
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]);
371+
if (isSetStateType(operand.identifier) && isFirstPass) {
372+
const rootSetStateId = getRootSetState(
373+
operand.identifier.id,
374+
context.setStateLoads,
375+
);
376+
if (rootSetStateId !== null) {
377+
context.setStateUsages.get(rootSetStateId)?.add(operand.loc);
335378
}
336379
}
337380

@@ -482,11 +525,16 @@ function validateEffect(
482525

483526
const effectDerivedSetStateCalls: Array<{
484527
value: CallExpression;
485-
loc: SourceLocation;
528+
id: IdentifierId;
486529
sourceIds: Set<IdentifierId>;
487530
typeOfValue: TypeOfValue;
488531
}> = [];
489532

533+
const effectSetStateUsages: Map<
534+
IdentifierId,
535+
Set<SourceLocation>
536+
> = new Map();
537+
490538
const globals: Set<IdentifierId> = new Set();
491539
for (const block of effectFunction.body.blocks.values()) {
492540
for (const pred of block.preds) {
@@ -502,19 +550,16 @@ function validateEffect(
502550
return;
503551
}
504552

553+
maybeRecordSetState(instr, context.setStateLoads, effectSetStateUsages);
554+
505555
for (const operand of eachInstructionOperand(instr)) {
506-
if (
507-
isSetStateType(operand.identifier) &&
508-
operand.loc !== GeneratedSource
509-
) {
510-
if (context.effectSetStateCache.has(operand.loc.identifierName)) {
511-
context.effectSetStateCache
512-
.get(operand.loc.identifierName)!
513-
.push(operand);
514-
} else {
515-
context.effectSetStateCache.set(operand.loc.identifierName, [
516-
operand,
517-
]);
556+
if (isSetStateType(operand.identifier)) {
557+
const rootSetStateId = getRootSetState(
558+
operand.identifier.id,
559+
context.setStateLoads,
560+
);
561+
if (rootSetStateId !== null) {
562+
effectSetStateUsages.get(rootSetStateId)?.add(operand.loc);
518563
}
519564
}
520565
}
@@ -532,7 +577,7 @@ function validateEffect(
532577
if (argMetadata !== undefined) {
533578
effectDerivedSetStateCalls.push({
534579
value: instr.value,
535-
loc: instr.value.callee.loc,
580+
id: instr.value.callee.identifier.id,
536581
sourceIds: argMetadata.sourcesIds,
537582
typeOfValue: argMetadata.typeOfValue,
538583
});
@@ -566,15 +611,17 @@ function validateEffect(
566611
}
567612

568613
for (const derivedSetStateCall of effectDerivedSetStateCalls) {
614+
const rootSetStateCall = getRootSetState(
615+
derivedSetStateCall.id,
616+
context.setStateLoads,
617+
);
618+
569619
if (
570-
derivedSetStateCall.loc !== GeneratedSource &&
571-
context.effectSetStateCache.has(derivedSetStateCall.loc.identifierName) &&
572-
context.setStateCache.has(derivedSetStateCall.loc.identifierName) &&
573-
context.effectSetStateCache.get(derivedSetStateCall.loc.identifierName)!
574-
.length ===
575-
context.setStateCache.get(derivedSetStateCall.loc.identifierName)!
576-
.length -
577-
1
620+
rootSetStateCall !== null &&
621+
effectSetStateUsages.has(rootSetStateCall) &&
622+
context.setStateUsages.has(rootSetStateCall) &&
623+
effectSetStateUsages.get(rootSetStateCall)!.size ===
624+
context.setStateUsages.get(rootSetStateCall)!.size - 1
578625
) {
579626
const allSourceIds = Array.from(derivedSetStateCall.sourceIds);
580627
const propsSet = new Set<string>();

0 commit comments

Comments
 (0)